Skip to content

[hotfix:] Layout After New/Removed Lines #97

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -87,30 +87,28 @@ extension TextLayoutManager {
#if DEBUG
var laidOutLines: Set<TextLine.ID> = []
#endif

// Layout all lines, fetching lines lazily as they are laid out.
for linePosition in linesStartingAt(minY, until: maxY).lazy {
guard linePosition.yPos < maxY else { continue }
// Three ways to determine if a line needs to be re-calculated.
let changedWidth = linePosition.data.needsLayout(maxWidth: maxLineLayoutWidth)
let linePositionNeedsLayout = linePosition.data.needsLayout(maxWidth: maxLineLayoutWidth)
let wasNotVisible = !visibleLineIds.contains(linePosition.data.id)
let lineNotEntirelyLaidOut = linePosition.height != linePosition.data.lineFragments.height

if forceLayout || changedWidth || wasNotVisible || lineNotEntirelyLaidOut {
if forceLayout || linePositionNeedsLayout || wasNotVisible || lineNotEntirelyLaidOut {
let lineSize = layoutLine(
linePosition,
textStorage: textStorage,
layoutData: LineLayoutData(minY: minY, maxY: maxY, maxWidth: maxLineLayoutWidth),
laidOutFragmentIDs: &usedFragmentIDs
)
if lineSize.height != linePosition.height {
let wasLineHeightChanged = lineSize.height != linePosition.height
if wasLineHeightChanged {
lineStorage.update(
atOffset: linePosition.range.location,
delta: 0,
deltaHeight: lineSize.height - linePosition.height
)
// If we've updated a line's height, force re-layout for the rest of the pass.
forceLayout = true

if linePosition.yPos < minY {
// Adjust the scroll position by the difference between the new height and old.
Expand All @@ -123,6 +121,14 @@ extension TextLayoutManager {
#if DEBUG
laidOutLines.insert(linePosition.data.id)
#endif
// If we've updated a line's height, or a line position was newly laid out, force re-layout for the
// rest of the pass (going down the screen).
//
// These two signals identify:
// - New lines being inserted & Lines being deleted (lineNotEntirelyLaidOut)
// - Line updated for width change (wasLineHeightChanged)

forceLayout = forceLayout || wasLineHeightChanged || lineNotEntirelyLaidOut
} else {
// Make sure the used fragment views aren't dequeued.
usedFragmentIDs.formUnion(linePosition.data.lineFragments.map(\.data.id))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,9 @@ struct TextLayoutManagerTests {
#expect(layoutManager.needsLayout == false)
}

/// Invalidating a range shouldn't cause a layout on any other lines next layout pass.
/// Note that this is correct behavior, and edits that add or remove lines will trigger another heuristic.
/// See `editsWithNewlinesForceLayoutGoingDownScreen`
@Test
func invalidatingRangeLaysOutLines() {
layoutManager.layoutLines(in: NSRect(x: 0, y: 0, width: 1000, height: 1000))
Expand All @@ -203,6 +206,26 @@ struct TextLayoutManagerTests {

let invalidatedLineIds = layoutManager.layoutLines()

#expect(invalidatedLineIds == lineIds, "Invalidated lines != lines that were laid out in next pass.")
#expect(
invalidatedLineIds.isSuperset(of: lineIds),
"Invalidated lines != lines that were laid out in next pass."
)
}

/// Inserting a new line should cause layout going down the rest of the screen, because the following lines
/// should have moved their position to accomodate the new line.
@Test
func editsWithNewlinesForceLayoutGoingDownScreen() {
layoutManager.layoutLines(in: NSRect(x: 0, y: 0, width: 1000, height: 1000))
textStorage.replaceCharacters(in: NSRange(start: 4, end: 4), with: "Z\n")

let expectedLineIds = Array(
layoutManager.lineStorage.linesInRange(NSRange(location: 4, length: 9))
).map { $0.data.id }

#expect(layoutManager.needsLayout == false) // No forced layout for entire view

let invalidatedLineIds = layoutManager.layoutLines()
#expect(Set(expectedLineIds) == invalidatedLineIds)
}
}