Skip to content

Commit 69282e2

Browse files
[hotfix:] Layout After New/Removed Lines (#97)
### Description Hotfix to fix mistakenly broken editing capabilities for inserting or deleting new lines. Updates the layout pass to correctly detect new lines, and if a new one is laid out, continues to layout lines after that line. Eg: ``` 1 2 <- Insert "\n" 3 ``` Before: ``` [visible text] 1 2 <- Layout invalidated 3 <- Layout _NOT_ invalidated, missing new line. ``` Now: ``` [visible text] 1 2 <- Layout invalidated <- Layout invalidated 3 <- Layout invalidated ``` Adds a new test case for this. ### Related Issues ### Checklist - [x] I read and understood the [contributing guide](https://github.com/CodeEditApp/CodeEdit/blob/main/CONTRIBUTING.md) as well as the [code of conduct](https://github.com/CodeEditApp/CodeEdit/blob/main/CODE_OF_CONDUCT.md) - [x] The issues this PR addresses are related to each other - [x] My changes generate no new warnings - [x] My code builds and runs on my machine - [x] My changes are all related to the related issue above - [x] I documented my code ### Screenshots
1 parent bc405e4 commit 69282e2

File tree

2 files changed

+36
-7
lines changed

2 files changed

+36
-7
lines changed

Sources/CodeEditTextView/TextLayoutManager/TextLayoutManager+Layout.swift

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -87,30 +87,28 @@ extension TextLayoutManager {
8787
#if DEBUG
8888
var laidOutLines: Set<TextLine.ID> = []
8989
#endif
90-
9190
// Layout all lines, fetching lines lazily as they are laid out.
9291
for linePosition in linesStartingAt(minY, until: maxY).lazy {
9392
guard linePosition.yPos < maxY else { continue }
9493
// Three ways to determine if a line needs to be re-calculated.
95-
let changedWidth = linePosition.data.needsLayout(maxWidth: maxLineLayoutWidth)
94+
let linePositionNeedsLayout = linePosition.data.needsLayout(maxWidth: maxLineLayoutWidth)
9695
let wasNotVisible = !visibleLineIds.contains(linePosition.data.id)
9796
let lineNotEntirelyLaidOut = linePosition.height != linePosition.data.lineFragments.height
9897

99-
if forceLayout || changedWidth || wasNotVisible || lineNotEntirelyLaidOut {
98+
if forceLayout || linePositionNeedsLayout || wasNotVisible || lineNotEntirelyLaidOut {
10099
let lineSize = layoutLine(
101100
linePosition,
102101
textStorage: textStorage,
103102
layoutData: LineLayoutData(minY: minY, maxY: maxY, maxWidth: maxLineLayoutWidth),
104103
laidOutFragmentIDs: &usedFragmentIDs
105104
)
106-
if lineSize.height != linePosition.height {
105+
let wasLineHeightChanged = lineSize.height != linePosition.height
106+
if wasLineHeightChanged {
107107
lineStorage.update(
108108
atOffset: linePosition.range.location,
109109
delta: 0,
110110
deltaHeight: lineSize.height - linePosition.height
111111
)
112-
// If we've updated a line's height, force re-layout for the rest of the pass.
113-
forceLayout = true
114112

115113
if linePosition.yPos < minY {
116114
// Adjust the scroll position by the difference between the new height and old.
@@ -123,6 +121,14 @@ extension TextLayoutManager {
123121
#if DEBUG
124122
laidOutLines.insert(linePosition.data.id)
125123
#endif
124+
// If we've updated a line's height, or a line position was newly laid out, force re-layout for the
125+
// rest of the pass (going down the screen).
126+
//
127+
// These two signals identify:
128+
// - New lines being inserted & Lines being deleted (lineNotEntirelyLaidOut)
129+
// - Line updated for width change (wasLineHeightChanged)
130+
131+
forceLayout = forceLayout || wasLineHeightChanged || lineNotEntirelyLaidOut
126132
} else {
127133
// Make sure the used fragment views aren't dequeued.
128134
usedFragmentIDs.formUnion(linePosition.data.lineFragments.map(\.data.id))

Tests/CodeEditTextViewTests/LayoutManager/TextLayoutManagerTests.swift

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -187,6 +187,9 @@ struct TextLayoutManagerTests {
187187
#expect(layoutManager.needsLayout == false)
188188
}
189189

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

204207
let invalidatedLineIds = layoutManager.layoutLines()
205208

206-
#expect(invalidatedLineIds == lineIds, "Invalidated lines != lines that were laid out in next pass.")
209+
#expect(
210+
invalidatedLineIds.isSuperset(of: lineIds),
211+
"Invalidated lines != lines that were laid out in next pass."
212+
)
213+
}
214+
215+
/// Inserting a new line should cause layout going down the rest of the screen, because the following lines
216+
/// should have moved their position to accomodate the new line.
217+
@Test
218+
func editsWithNewlinesForceLayoutGoingDownScreen() {
219+
layoutManager.layoutLines(in: NSRect(x: 0, y: 0, width: 1000, height: 1000))
220+
textStorage.replaceCharacters(in: NSRange(start: 4, end: 4), with: "Z\n")
221+
222+
let expectedLineIds = Array(
223+
layoutManager.lineStorage.linesInRange(NSRange(location: 4, length: 9))
224+
).map { $0.data.id }
225+
226+
#expect(layoutManager.needsLayout == false) // No forced layout for entire view
227+
228+
let invalidatedLineIds = layoutManager.layoutLines()
229+
#expect(Set(expectedLineIds) == invalidatedLineIds)
207230
}
208231
}

0 commit comments

Comments
 (0)