fix: sync streaming line-break stepper with batch walker#135
Open
voidborne-d wants to merge 1 commit intochenglou:mainfrom
Open
fix: sync streaming line-break stepper with batch walker#135voidborne-d wants to merge 1 commit intochenglou:mainfrom
voidborne-d wants to merge 1 commit intochenglou:mainfrom
Conversation
Two mismatches between layoutNextLine/layoutNextLineRange (streaming) and layoutWithLines/walkLineRanges (batch) caused the streaming APIs to produce different line breaks for certain inputs involving mixed scripts, soft hyphens, and non-simple whitespace modes. 1. maybeFinishAtSoftHyphen premature fallback (pre-wrap + soft-hyphen) When an overflow occurred at a breakAfter segment (preserved-space, tab, etc.) and a soft-hyphen pending break was active, the step function's maybeFinishAtSoftHyphen had a fallback path that used the soft-hyphen break even when the segment's breakableFitAdvances was null. The batch walker's equivalent (continueSoftHyphenBreakableSegment) returns false in this case, allowing the closer breakAfter opportunity to be chosen instead. Remove the fallback from maybeFinishAtSoftHyphen so the general pending-break check handles it, matching the batch walker order. 2. normalizeLineStartInChunk over-strips spaces (chunked path) normalizeLineStartInChunk skipped space segments at line starts, but the chunked batch walker (walkPreparedLinesRaw non-simple branch) does not strip leading spaces when starting a new line within a chunk. This caused the streaming stepper to skip a leading space that the batch walker kept, producing fewer lines. Condition the space-skip on simpleLineWalkFastPath so only the simple path strips spaces (matching normalizeSimpleLineStartSegmentIndex and the simple batch walker). Add regression tests covering both fixes with the reproducer from chenglou#121.
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.
Fixes #121
Two mismatches between
layoutNextLine/layoutNextLineRange(streaming) andlayoutWithLines/walkLineRanges(batch) caused the streaming APIs to produce different line breaks for certain inputs involving mixed scripts, soft hyphens, and non-simple whitespace modes.1.
maybeFinishAtSoftHyphenpremature fallback (pre-wrap + soft-hyphen)When an overflow occurred at a
breakAftersegment (preserved-space, tab, etc.) and a soft-hyphen pending break was active, the step function'smaybeFinishAtSoftHyphenhad a fallback path that used the soft-hyphen break even when the segment'sbreakableFitAdvanceswasnull. The batch walker's equivalent (continueSoftHyphenBreakableSegment) returnsfalsein this case, allowing the closerbreakAfteropportunity to be chosen instead.Fix: Remove the fallback from
maybeFinishAtSoftHyphenso the general pending-break check handles it, matching the batch walker's check order.2.
normalizeLineStartInChunkover-strips spaces (chunked path)normalizeLineStartInChunkskippedspacesegments at line starts, but the chunked batch walker (walkPreparedLinesRawnon-simple branch) does not strip leading spaces when starting a new line within a chunk. This caused the streaming stepper to skip a leading space that the batch walker kept, producing fewer lines.Fix: Condition the space-skip on
simpleLineWalkFastPathso only the simple path strips spaces (matchingnormalizeSimpleLineStartSegmentIndexand the simple batch walker).Reproduction
Using the reproducer from #121:
Tests
Added 3 regression tests:
pre-wrap soft-hyphen does not preempt a closer breakAfter opportunitylayoutNextLine keeps leading space inside a non-simple chunk when the batch walker doesmixed-script Arabic+CJK text keeps streaming aligned with batch layoutAll 87 tests pass (
bun test src/layout.test.ts).