Repository navigation
Speed up calculate_content_widths on top of upstream #790 (fused cluster flags + flat per-cluster scan) - #46
Closed
nicoburns wants to merge 6 commits into
Conversation
…er#811) LLM Contributions: Unknown for linebender#620, c-emoji. I used them for review, bootstrapping the new crate to follow https://linebender.org/wiki/formatting-scheme/, and getting/implementing the technique of sticking the properties into our existing property infrastructure, plus rebasing/reordering the series. Adaptation of linebender#620 by @fundon. This uses a state machine based on @chansen's [c-emoji](https://github.com/chansen/c-emoji) to implement detection of Emoji sequences, as described in [tr-51](https://www.unicode.org/reports/tr51/tr51-29.html). This is Unicode 17 Emoji, as opposed to Unicode 18 (releasing tomorrow...), but I don't think the difference has a huge impact for us. The state machine lives in a new crate `parley-emoji`, so as to follow c-emoji's license. We were unable to get its code relicensed under our MIT/Apache 2.0 license. With this, we are *not* completely compliant with Unicode Emoji, but this gives us a significantly better jumping off point. Things I've chosen to defer: - Better testing. I have local changes which generate a suitable subset of Noto Color Emoji, but I think end-to-end testing needs more care. - Also not yet tested using the upstream Unicode data. - Splitting a single grapheme cluster into its 'text presentation/emoji presentation' preferred sections. This can happen in three cases that I'm aware of: - Prepend character then Emoji, all within one grapheme cluster - Non-Emoji modifier base, then Fitzgerald skin tone modifier - ZWJ sequence containing VS15 text variation selectors. - Having the Emoji font in the fallback chain, even if text presentation is preferred. - Thinking through how the 'reject' case should be handled - Doing the proper rendering of malformed tag sequences, which is required by spec, but which as far as I know, no system implements. - [#font-tech > Appearance of unsupported emoji sequences](https://xi.zulipchat.com/#narrow/channel/296640-font-tech/topic/Appearance.20of.20unsupported.20emoji.20sequences/with/623471132). - Verifying that the handling of RI sequences is correct - in particular, the way that three regional indicators in a row merge into grapheme clusters can lead to jank. - Fix the editing behaviour to use this properly (see also linebender#693) - Documenting our level of compliance with the Unicode Emoji spec. - Handling legacy modifier sequences with an intervening VS16. There's a TODO in the code for this. - The 'emoji presentation locale' thing. I don't believe any of those are blocking - this is still a better state than before. Fixes linebender#744. Supersedes linebender#620, linebender#788, linebender#692. cc @kane50613 (wrt linebender#745). **Changelog** > ### Fixed > > - Parley's detection of Emoji is now more robust. --------- Co-authored-by: Fangdun Tsai <fundon@pindash.io> Co-authored-by: Tom Churchman <thomas@churchman.nl>
LLM Contributions: Generated with Fable 5.1 Low. - Builds on linebender#812 Converts `WhiteSpaceCollapse` to be a proper style rather than being handled in a custom way on `TreeBuilder`. This makes the style available to layout (line breaking / content widths computation) and is a pre-requisite for implementing proper mode-aware whitespace hanging. (this turned out to be surprisingly straightforward) **Changelog** > ### Added > > - Breaking change: `WhiteSpaceCollapse` is now a style rather than a special method on the `TreeBuilder` --------- Signed-off-by: Nico Burns <nico@nicoburns.com> Co-authored-by: Tom Churchman <thomas@churchman.nl>
|
I'll fix CI failures and address comments from users with write access that start with 'DevinAI' or '@devin'.
|
LLM Contributions: Generated with Devin (Fable 5.1 Low)
**Changelog: None**
## Changes made
- Remove direct `glifo` dependency
- Upgrade `vello_cpu` to latest `0.2.0`
- Snapshot comparison is now exact. `Pixmap::into_png` unpremultiplies
with rounding but `Pixmap::from_png` re-premultiplies with truncation,
so a decoded snapshot was never bit-identical to the pixmap it was
written from. We. now roundtrip new snapshots through png encode/decode
to work around this. Allows us to entirely remove the tolerance for
several snapshots (otherwise had to increase from ~5 to over 100)
Most of the changes are mechanical changes around the switch from
`glifo` to `vello_cpu` in the test runner, and adapting to the new
`vello_cpu` API in examples.
## Snapshots
- Almost all updated, but most are just AA noise from the vello_cpu
upgrade.
- The `draw_colr_emoji-{nohint,hint,2x_nohint,2x_hint}_skew` COLR glyphs
are now skewed noticeably less than before. This was an intentional
upstream bugfix in glifo.
- The macOS bitmap emoji bitmaps are resampled with a softer filter
(looks better to me)
AI-generated diff images included below for the snapshots with non-AA
changes:


<!-- Please ensure that you have reviewed our LLM ("AI") policy at
https://linebender.org/wiki/llm-policy/.
If you did not use any LLM tools, please replace `Unspecified` with
`None`. -->
LLM Contributions: Based on
#33, initial implementation,
review, added tests.
This implements the "conditionally hanging" whitespace part of CSS Text
4 § 4.3.2, where whitespace with `white-space-collapse: preserve` hangs
conditionally if it ends the layout or is followed by a newline.
Whitespace that hangs conditionally, only hangs as far as it overflows
the line.
# WPT Results
WPT result using Blitz, compared against current `main`. 45
newly-passing, 4 newly failing. The newly-failing ones use styles we
don't implement yet (see Nico's
linebender#820!).
```
Fail => Pass css/css-text/hyphens/hyphens-auto-002.html
Fail => Pass css/css-text/hyphens/hyphens-auto-003.html
Fail => Pass css/css-text/line-break/line-break-anywhere-and-white-space-006.html
Fail => Pass css/css-text/text-transform/text-transform-fullwidth-009.html
Pass => Fail css/css-text/white-space/break-spaces-001.html
Pass => Fail css/css-text/white-space/break-spaces-010.html
Pass => Fail css/css-text/white-space/break-spaces-with-ideographic-space-001.html
Pass => Fail css/css-text/white-space/break-spaces-with-ideographic-space-010.html
Fail => Pass css/css-text/white-space/hanging-whitespace-001.tentative.html
Fail => Pass css/css-text/white-space/hanging-whitespace-004.html
Fail => Pass css/css-text/white-space/pre-wrap-001.html
Fail => Pass css/css-text/white-space/pre-wrap-002.html
Fail => Pass css/css-text/white-space/pre-wrap-003.html
Fail => Pass css/css-text/white-space/pre-wrap-004.html
Fail => Pass css/css-text/white-space/pre-wrap-005.html
Fail => Pass css/css-text/white-space/pre-wrap-006.html
Fail => Pass css/css-text/white-space/pre-wrap-007.html
Fail => Pass css/css-text/white-space/pre-wrap-017.html
Fail => Pass css/css-text/white-space/pre-wrap-018.html
Fail => Pass css/css-text/white-space/pre-wrap-019.html
Fail => Pass css/css-text/white-space/pre-wrap-leading-spaces-015.html
Fail => Pass css/css-text/white-space/pre-wrap-leading-spaces-017.html
Fail => Pass css/css-text/white-space/trailing-ideographic-space-001.html
Fail => Pass css/css-text/white-space/trailing-ideographic-space-004.html
Fail => Pass css/css-text/white-space/trailing-ideographic-space-005.html
Fail => Pass css/css-text/white-space/trailing-ideographic-space-010.html
Fail => Pass css/css-text/white-space/trailing-ideographic-space-017.html
Fail => Pass css/css-text/white-space/trailing-ideographic-space-018.html
Fail => Pass css/css-text/white-space/trailing-ideographic-space-019.html
Fail => Pass css/css-text/white-space/trailing-ideographic-space-020.html
Fail => Pass css/css-text/white-space/trailing-ideographic-space-021.html
Fail => Pass css/css-text/white-space/trailing-ideographic-space-022.html
Fail => Pass css/css-text/white-space/trailing-ideographic-space-023.html
Fail => Pass css/css-text/white-space/trailing-ideographic-space-024.html
Fail => Pass css/css-text/white-space/trailing-ideographic-space-025.html
Fail => Pass css/css-text/white-space/white-space-intrinsic-size-002.html
Fail => Pass css/css-text/white-space/white-space-intrinsic-size-003.html
Fail => Pass css/css-text/white-space/white-space-intrinsic-size-004.html
Fail => Pass css/css-text/white-space/white-space-intrinsic-size-013.html
Fail => Pass css/css-text/white-space/white-space-intrinsic-size-014.html
Fail => Pass css/css-text/white-space/white-space-intrinsic-size-017.html
Fail => Pass css/css-text/white-space/white-space-intrinsic-size-019.html
Fail => Pass css/css-text/white-space/white-space-pre-wrap-trailing-spaces-001.html
Fail => Pass css/css-text/white-space/white-space-pre-wrap-trailing-spaces-003.html
Fail => Pass css/css-text/white-space/white-space-pre-wrap-trailing-spaces-005.html
Fail => Pass css/css-text/white-space/white-space-pre-wrap-trailing-spaces-007.html
Fail => Pass css/css-text/white-space/white-space-pre-wrap-trailing-spaces-008.html
Fail => Pass css/css-text/white-space/white-space-pre-wrap-trailing-spaces-011.html
Fail => Pass css/css-text/white-space/white-space-pre-wrap-trailing-spaces-021.html
```
<!--
If our users need to know about this change, please describe that in the
quote block below.
What you write here will be edited by us later - it doesn't need to be
perfect.
If this change doesn't need a changelog entry, please replace the next
line with `**Changelog: None**`.
-->
**Changelog**
> ### Changed
>
> - Following CSS Text 4 § 4.3.2, whitespace with
`WhiteSpaceCollapse::Preserve` now hangs only if it's not on the last
line, and is not followed by a newline.
…r; use it in calculate_content_widths The width scan no longer touches the character array in its fast path: boundary (2 bits) and whitespace class (3 bits) of a cluster's first character are stored in the free bits of ShapedClusterFlags at shape time. calculate_content_widths walks shaped clusters by index and only slices into characters for multi-cluster/multi-character atoms.
staging-devin-ai-integration
Bot
force-pushed
the
devin/1789599436-content-widths-790
branch
from
September 17, 2026 12:02
7e6ed02 to
d23fc54
Compare
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.
LLM Contributions: Written by Devin (Cognition AI) under direction of @nicoburns.
Rebase of #32 (and the relevant part of #26) onto upstream linebender#790 (
hang-conditionally, at cbf2fa1; the base branch here is a verbatim copy of it). Supersedes #32.Corpus benchmark (250 KB real text,
calculate_content_widths, CPU-pinned, interleaved, n=12 x 3):ClusterDatascan)widths-resultidentical (min: 1160.2813, max: 17458.617). Profile of the new code is a single fully-inlined loop (noAtoms::next/characters[]traffic).Commit 1 — engine:
ShapedClusterFlagshas 5 free bits; they now hold the first character'sBoundary(2 bits) andWhitespace(3 bits — the current 8-variant enum fits exactly), set inprocess_shaped_clusters::flushfromfirst_character.info. Additive API onShapedCluster:boundary_before(),whitespace(),char_len().ShapedClusterstays 20 bytes. A round-trip unit test covers all variants.Commit 2 — layout: when the run has no letter/word spacing (the common case)
calculate_content_widthsscansshaped_clustersdirectly instead ofatoms_start():i == 0 || is_grapheme_start()), with askip_atomflag to skip a Newline atom's continuation clusters like the atom loop'scontinuedoes;if cluster_hangs { += advance } else { = 0 }), which is equivalent toatom_hanging_advance's "hangs entirely / hangs partially from the logical end" rule when there are no gaps;whitespace_hangs(first char, style)is checked beforechar_len() == 1, so letters never loadchars_range; multi-char clusters fall back to checking every character (with its own style, asatom_hanging_advancedoes).hangs_conditionallyis set from the cluster's style; Implement CSS's conditional hanging of whitespace linebender/parley#790 uses the atom's first-character style, which only differs for mixed-style multi-cluster atoms (the edge case already flagged in Implement CSS's conditional hanging of whitespace linebender/parley#790 review).Runs with spacing keep linebender#790's
atoms_start()loop unchanged (it needsAtomforspacing.atom_advance/atom_hanging_advance).Note: the first commit's message still mentions the layout change from #32; the layout part is in the second commit.
Changelog: None
Link to Devin session: https://dioxus.staging.devinenterprise.com/sessions/2564f38c1f574d04ac6e9a57a7284d93
Open in Devin Desktop: https://dioxus.staging.devinenterprise.com/desktop/session/2564f38c1f574d04ac6e9a57a7284d93?variant=devin-insiders