From f9039a35f6ba14e6ba334b86a3a839a92eb27d2e Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Sat, 14 May 2016 17:28:54 +0200 Subject: [PATCH 1/3] Refactor `isRowVisible` to `isRowRendered` There's a distinction to make between rendered and visible rows, and we were using the former as if it was the latter. In particular, if a tile is visible, all its rows get rendered on screen, even though they might not necessarily be visible by the user. --- src/text-editor-component.coffee | 8 ++++---- src/text-editor-presenter.coffee | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/text-editor-component.coffee b/src/text-editor-component.coffee index 8c20055ed..048033a20 100644 --- a/src/text-editor-component.coffee +++ b/src/text-editor-component.coffee @@ -491,7 +491,7 @@ class TextEditorComponent screenPosition = Point.fromObject(screenPosition) screenPosition = @editor.clipScreenPosition(screenPosition) if clip - unless @presenter.isRowVisible(screenPosition.row) + unless @presenter.isRowRendered(screenPosition.row) @presenter.setScreenRowsToMeasure([screenPosition.row]) unless @linesComponent.lineNodeForScreenRow(screenPosition.row)? @@ -503,7 +503,7 @@ class TextEditorComponent screenPositionForPixelPosition: (pixelPosition) -> row = @linesYardstick.measuredRowForPixelPosition(pixelPosition) - if row? and not @presenter.isRowVisible(row) + if row? and not @presenter.isRowRendered(row) @presenter.setScreenRowsToMeasure([row]) @updateSyncPreMeasurement() @@ -513,9 +513,9 @@ class TextEditorComponent pixelRectForScreenRange: (screenRange) -> rowsToMeasure = [] - unless @presenter.isRowVisible(screenRange.start.row) + unless @presenter.isRowRendered(screenRange.start.row) rowsToMeasure.push(screenRange.start.row) - unless @presenter.isRowVisible(screenRange.end.row) + unless @presenter.isRowRendered(screenRange.end.row) rowsToMeasure.push(screenRange.end.row) if rowsToMeasure.length > 0 diff --git a/src/text-editor-presenter.coffee b/src/text-editor-presenter.coffee index 01a0293f6..1f48bc4fa 100644 --- a/src/text-editor-presenter.coffee +++ b/src/text-editor-presenter.coffee @@ -1550,8 +1550,8 @@ class TextEditorPresenter getVisibleRowRange: -> [@startRow, @endRow] - isRowVisible: (row) -> - @startRow <= row < @endRow + isRowRendered: (row) -> + @getStartTileRow() <= row < @constrainRow(@getEndTileRow() + @tileSize) isOpenTagCode: (tagCode) -> @displayLayer.isOpenTagCode(tagCode) From b5b324875e000809e06574ce8f8b0d17f04d0bc3 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Mon, 16 May 2016 10:41:55 +0200 Subject: [PATCH 2/3] Don't render line-numbers corresponding to lines that need measuring Rendering those line numbers in the gutter isn't useful, and it puts unneeded pressure to the DOM. In the process of changing `updateLineNumbersState`, we have also refactored it to stop relying on row ranges being contiguous. This allows that code path to be: 1. Less error-prone, because we were trying to access rows that weren't actually rendered, thus potentially throwing errors when measuring non-contiguous screen rows that weren't visible. 2. Tighter, because we can just iterate over each screen row and ask for its soft-wrap descriptor. --- spec/text-editor-presenter-spec.coffee | 12 ++++--- src/text-editor-presenter.coffee | 47 +++++++------------------- 2 files changed, 20 insertions(+), 39 deletions(-) diff --git a/spec/text-editor-presenter-spec.coffee b/spec/text-editor-presenter-spec.coffee index 8cdc4e61a..5a91c74f5 100644 --- a/spec/text-editor-presenter-spec.coffee +++ b/spec/text-editor-presenter-spec.coffee @@ -2805,16 +2805,20 @@ describe "TextEditorPresenter", -> editor.setSoftWrapped(true) editor.setDefaultCharWidth(1) editor.setEditorWidthInChars(51) - presenter = buildPresenter(explicitHeight: 25, scrollTop: 30, lineHeight: 10, tileSize: 2) + presenter = buildPresenter(explicitHeight: 25, scrollTop: 30, lineHeight: 10, tileSize: 3) + presenter.setScreenRowsToMeasure([9, 11]) - expect(lineNumberStateForScreenRow(presenter, 1)).toBeUndefined() - expectValues lineNumberStateForScreenRow(presenter, 2), {screenRow: 2, bufferRow: 2, softWrapped: false} + expect(lineNumberStateForScreenRow(presenter, 2)).toBeUndefined() expectValues lineNumberStateForScreenRow(presenter, 3), {screenRow: 3, bufferRow: 3, softWrapped: false} expectValues lineNumberStateForScreenRow(presenter, 4), {screenRow: 4, bufferRow: 3, softWrapped: true} expectValues lineNumberStateForScreenRow(presenter, 5), {screenRow: 5, bufferRow: 4, softWrapped: false} expectValues lineNumberStateForScreenRow(presenter, 6), {screenRow: 6, bufferRow: 7, softWrapped: false} expectValues lineNumberStateForScreenRow(presenter, 7), {screenRow: 7, bufferRow: 8, softWrapped: false} - expect(lineNumberStateForScreenRow(presenter, 8)).toBeUndefined() + expectValues lineNumberStateForScreenRow(presenter, 8), {screenRow: 8, bufferRow: 8, softWrapped: true} + expect(lineNumberStateForScreenRow(presenter, 9)).toBeUndefined() + expect(lineNumberStateForScreenRow(presenter, 10)).toBeUndefined() + expect(lineNumberStateForScreenRow(presenter, 11)).toBeUndefined() + expect(lineNumberStateForScreenRow(presenter, 12)).toBeUndefined() it "updates when the editor's content changes", -> editor.foldBufferRow(4) diff --git a/src/text-editor-presenter.coffee b/src/text-editor-presenter.coffee index 1f48bc4fa..95c46e5bf 100644 --- a/src/text-editor-presenter.coffee +++ b/src/text-editor-presenter.coffee @@ -601,42 +601,19 @@ class TextEditorPresenter tileState.lineNumbers ?= {} visibleLineNumberIds = {} - startRow = screenRows[screenRows.length - 1] - endRow = Math.min(screenRows[0] + 1, @model.getScreenLineCount()) + for screenRow in screenRows when @isRowRendered(screenRow) + lineId = @linesByScreenRow.get(screenRow).id + {bufferRow, softWrappedAtStart: softWrapped} = @displayLayer.softWrapDescriptorForScreenRow(screenRow) + foldable = not softWrapped and @model.isFoldableAtBufferRow(bufferRow) + decorationClasses = @lineNumberDecorationClassesForRow(screenRow) + blockDecorationsBeforeCurrentScreenRowHeight = @lineTopIndex.pixelPositionAfterBlocksForRow(screenRow) - @lineTopIndex.pixelPositionBeforeBlocksForRow(screenRow) + blockDecorationsHeight = blockDecorationsBeforeCurrentScreenRowHeight + if screenRow % @tileSize isnt 0 + blockDecorationsAfterPreviousScreenRowHeight = @lineTopIndex.pixelPositionBeforeBlocksForRow(screenRow) - @lineHeight - @lineTopIndex.pixelPositionAfterBlocksForRow(screenRow - 1) + blockDecorationsHeight += blockDecorationsAfterPreviousScreenRowHeight - if startRow > 0 - rowBeforeStartRow = startRow - 1 - lastBufferRow = @model.bufferRowForScreenRow(rowBeforeStartRow) - else - lastBufferRow = null - - if endRow > startRow - bufferRows = @model.bufferRowsForScreenRows(startRow, endRow - 1) - previousBufferRow = -1 - foldable = false - for bufferRow, i in bufferRows - # don't compute foldability more than once per buffer row - if previousBufferRow isnt bufferRow - foldable = @model.isFoldableAtBufferRow(bufferRow) - previousBufferRow = bufferRow - - if bufferRow is lastBufferRow - softWrapped = true - else - lastBufferRow = bufferRow - softWrapped = false - - screenRow = startRow + i - lineId = @linesByScreenRow.get(screenRow).id - decorationClasses = @lineNumberDecorationClassesForRow(screenRow) - blockDecorationsBeforeCurrentScreenRowHeight = @lineTopIndex.pixelPositionAfterBlocksForRow(screenRow) - @lineTopIndex.pixelPositionBeforeBlocksForRow(screenRow) - blockDecorationsHeight = blockDecorationsBeforeCurrentScreenRowHeight - if screenRow % @tileSize isnt 0 - blockDecorationsAfterPreviousScreenRowHeight = @lineTopIndex.pixelPositionBeforeBlocksForRow(screenRow) - @lineHeight - @lineTopIndex.pixelPositionAfterBlocksForRow(screenRow - 1) - blockDecorationsHeight += blockDecorationsAfterPreviousScreenRowHeight - - tileState.lineNumbers[lineId] = {screenRow, bufferRow, softWrapped, decorationClasses, foldable, blockDecorationsHeight} - visibleLineNumberIds[lineId] = true + tileState.lineNumbers[lineId] = {screenRow, bufferRow, softWrapped, decorationClasses, foldable, blockDecorationsHeight} + visibleLineNumberIds[lineId] = true for id of tileState.lineNumbers delete tileState.lineNumbers[id] unless visibleLineNumberIds[id] From a18369586096191b4ceb22db123d4dbc9fe154d7 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Mon, 16 May 2016 13:36:46 +0200 Subject: [PATCH 3/3] :arrow_up: text-buffer --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index d512e3ddf..80aa134e3 100644 --- a/package.json +++ b/package.json @@ -54,7 +54,7 @@ "service-hub": "^0.7.0", "source-map-support": "^0.3.2", "temp": "0.8.1", - "text-buffer": "9.0.0", + "text-buffer": "9.1.0", "typescript-simple": "1.0.0", "underscore-plus": "^1.6.6", "yargs": "^3.23.0"