Align crowding bounds with the displayed triplet - #121
Open
sajjad-mazaheri wants to merge 2 commits into
Open
Conversation
There was a problem hiding this comment.
Pull request overview
This PR addresses a typographic-crowding sizing/bounding mismatch by ensuring the pre-fixation “crowding bounds” measurement uses the same target+flanker triplet that will be rendered on-screen, which is especially important for contextual/joining fonts (e.g., IranNastaliq).
Changes:
- Pass the full
CharactersLetterobject into the V2 letter stimulus generation path (instead of onlytarget). - Update
restrictLevelBeforeFixationto use the already-chosenflanker1 + target + flanker2triplet when computing typographic-crowding bounds.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| components/stimulusGeneration.ts | Threads CharactersLetter through V2 stimulus generation so bounding can measure the displayed triplet. |
| components/boundingNew.js | Uses provided target/flankers for typographic-crowding pre-fixation bounds instead of sampling a proxy triplet. |
Comments suppressed due to low confidence (1)
components/stimulusGeneration.ts:288
- The afterFixation parameter check treats
proposedLevelas a truthy value; a valid level of0would incorrectly throw. Prefer an explicitproposedLevel === undefinedcheck while still rejecting missing/emptytargetCharacter.
if (stage === "afterFixation" && !(targetCharacter && proposedLevel)) {
throw new Error(
`targetCharacter (${targetCharacter}) and proposedLevel (${proposedLevel}) must be provided for afterFixation letter stimulus generation.`,
);
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
350
to
354
| } else if (quickCase === "typographicCrowding") { | ||
| [targetString, flanker1String, flanker2String] = sampleWithoutReplacement( | ||
| fontCharacterSet, | ||
| 3, | ||
| ); | ||
| targetString = letterCharacters.target; | ||
| flanker1String = letterCharacters.flanker1; | ||
| flanker2String = letterCharacters.flanker2; | ||
| characterSet = [flanker1String, targetString, flanker2String]; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What I noticed
While running Persian experiment with IranNastaliq, I saw the triplet occasionally render partly off the
bottom of the screen. In this trial the fixation cross is centred, but the
triplet has dropped below and is clipped by the screen edge:
I wanted to understand where that size was coming from, so I cloned the repo and
traced the letter-stimulus path. I might be missing context, so I'm sharing this
as something I noticed rather than a firm diagnosis.
What seems to be happening
For a typographic-crowding trial, two places pick characters, and I think they
can quietly disagree:
actually sees, and renders them as one joined string
(
flanker1 + target + flanker2).restrictLevelBeforeFixationsamples another three charactersfrom
fontCharacterSetand measures that triplet to decide how large thestimulus may be.
Because these are two independent draws, the triplet measured for the bounds can
be a different string from the one drawn on screen. For most fonts their widths
are close, so it never shows. IranNastaliq is contextual, though (letters join
and reshape depending on their neighbours) so two different triplets can have
quite different joined widths. When the measured proxy is much narrower than the
displayed triplet, the size cap comes out too generous and the real triplet can
extend past the screen edge, which is what the screenshot above shows.
To see it more concretely I built a small standalone reproduction (the hidden

measured triplet on the left, the displayed triplet with its calculated vs
actual bounds below):
Why it can matter
In Persian the letters join together, so two different triplets can have very
different shapes and widths. The code uses that width to choose the stimulus
size and to keep it on the screen. So when the size is measured on one triplet
but a different triplet is shown, the participant can see a different size and
spacing than intended, and if the width difference is large (which it easily is
in Persian), the triplet can even run off the screen, as above.
It also affects QUEST. Each trial QUEST picks a spacing and records the response
at the spacing it asked for, but the spacing actually shown depends on the
displayed triplet's width, and not the measured one. So when those widths differ a lot, the presented spacing is off by a lot, and because a new proxy is drawn for every trial, that error changes from trial to trial, so QUEST ends up adapting to spacings that weren't really the ones presented. (in my opinion)
What this PR proposes
A small change: pass the target and flankers already chosen for the trial into
the bounding step, so it measures exactly the string that will be displayed
(
flanker1 + target + flanker2). Acuity and ratio-crowding keep samplingexactly as before.