Skip to content

Ignore empty word break ranges, clarify ranges must fall on character boundaries - #720

Merged
tomcur merged 1 commit into
linebender:mainfrom
tomcur:push-wsxxstmxxysm
Jul 31, 2026
Merged

Ignore empty word break ranges, clarify ranges must fall on character boundaries#720
tomcur merged 1 commit into
linebender:mainfrom
tomcur:push-wsxxstmxxysm

Conversation

@tomcur

@tomcur tomcur commented Jul 30, 2026

Copy link
Copy Markdown
Member

Empty ranges must be ignored, as otherwise the range the iterator is pointing can get out of sync with the character.

@nicoburns nicoburns left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be better to store this as a flat Vec of indices (usize rather than Range<usize>)?

@tomcur

tomcur commented Jul 30, 2026

Copy link
Copy Markdown
Member Author

Would it be better to store this as a flat Vec of indices (usize rather than Range<usize>)?

I think probably not, because of gaps using WordBreak::Normal. (If we didn't allow gaps, we'd either have to special-case &[] or always require an allocation, even if the entire text is WordBreak::Normal.) The alternative would be foregoing storing and either monomorphizing or doing dyn dispatch.

@tomcur tomcur changed the title Ignore empty word break ranges, clarify they must fall on character boundaries Ignore empty word break ranges, clarify ranges must fall on character boundaries Jul 30, 2026
@tomcur
tomcur added this pull request to the merge queue Jul 31, 2026
Merged via the queue into linebender:main with commit 27f4c98 Jul 31, 2026
24 checks passed
@tomcur
tomcur deleted the push-wsxxstmxxysm branch July 31, 2026 04:36
tomcur added a commit to tomcur/parley that referenced this pull request Jul 31, 2026
On top of linebender#720, as otherwise
that code hits an underflow.

Empty text still needs a style, e.g., to be able to size cursors. This
PR plumbs the root style through. I hit this when making font selection
infallible, which tripped up the `builders_empty` test.

(This might also be useful for sizing "struts".)

Because with this change these empty layouts now actually have a style,
that means font selection now succeeds in the `builders_empty` test,
where it previously failed. Not having a font was the only reason there
weren't any runs in the changed test, because, for empty text, `parley`
actually shapes a space character, which should lead to a run.

I've chosen to remove checking that runs are empty from the test, as I
think it's mostly checking the wrong thing at the moment. If we ever
stop synthesizing a space character for empty text, we should revisit
this.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants