From af41b71cd8f245fe45c3fae411466194c15a85a4 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Sun, 20 Sep 2015 12:10:09 +0200 Subject: [PATCH] Redesign LinesYardstick --- spec/lines-yardstick-spec.coffee | 68 ------------------ spec/text-editor-presenter-spec.coffee | 24 +++---- src/lines-component.coffee | 4 ++ src/lines-tile-component.coffee | 2 + src/lines-yardstick.coffee | 21 ++++-- src/text-editor-component.coffee | 4 ++ src/text-editor-presenter.coffee | 97 ++++++++++++++------------ 7 files changed, 85 insertions(+), 135 deletions(-) delete mode 100644 spec/lines-yardstick-spec.coffee diff --git a/spec/lines-yardstick-spec.coffee b/spec/lines-yardstick-spec.coffee deleted file mode 100644 index e68ac15e5..000000000 --- a/spec/lines-yardstick-spec.coffee +++ /dev/null @@ -1,68 +0,0 @@ -LinesYardstick = require '../src/lines-yardstick' -MockLinesComponent = require './mock-lines-component' - -describe "LinesYardstick", -> - [editor, mockPresenter, mockLinesComponent, linesYardstick] = [] - - beforeEach -> - waitsForPromise -> - atom.packages.activatePackage('language-javascript') - - waitsForPromise -> - atom.project.open('sample.js').then (o) -> editor = o - - runs -> - mockPresenter = {getStateForMeasurements: jasmine.createSpy()} - mockLinesComponent = new MockLinesComponent(editor) - linesYardstick = new LinesYardstick(editor, mockPresenter, mockLinesComponent) - - mockLinesComponent.setDefaultFont("14px monospace") - - afterEach -> - doSomething = true - - it "converts screen positions to pixel positions", -> - stubState = {anything: {}} - mockPresenter.getStateForMeasurements.andReturn(stubState) - - linesYardstick.prepareScreenRowsForMeasurement([0, 1, 2]) - - expect(mockPresenter.getStateForMeasurements).toHaveBeenCalledWith([0, 1, 2]) - expect(mockLinesComponent.updateSync).toHaveBeenCalledWith(stubState) - - conversionTable = [ - [[0, 0], {left: 0, top: editor.getLineHeightInPixels() * 0}] - [[0, 3], {left: 24, top: editor.getLineHeightInPixels() * 0}] - [[0, 4], {left: 32, top: editor.getLineHeightInPixels() * 0}] - [[0, 5], {left: 40, top: editor.getLineHeightInPixels() * 0}] - [[1, 0], {left: 0, top: editor.getLineHeightInPixels() * 1}] - [[1, 1], {left: 0, top: editor.getLineHeightInPixels() * 1}] - [[1, 6], {left: 48, top: editor.getLineHeightInPixels() * 1}] - [[1, Infinity], {left: 240, top: editor.getLineHeightInPixels() * 1}] - ] - - for [point, position] in conversionTable - expect( - linesYardstick.pixelPositionForScreenPosition(point) - ).toEqual(position) - - mockLinesComponent.setFontForScopes( - ["source.js", "storage.modifier.js"], "16px monospace" - ) - linesYardstick.clearCache() - - conversionTable = [ - [[0, 0], {left: 0, top: editor.getLineHeightInPixels() * 0}] - [[0, 3], {left: 30, top: editor.getLineHeightInPixels() * 0}] - [[0, 4], {left: 38, top: editor.getLineHeightInPixels() * 0}] - [[0, 5], {left: 46, top: editor.getLineHeightInPixels() * 0}] - [[1, 0], {left: 0, top: editor.getLineHeightInPixels() * 1}] - [[1, 1], {left: 0, top: editor.getLineHeightInPixels() * 1}] - [[1, 6], {left: 54, top: editor.getLineHeightInPixels() * 1}] - [[1, Infinity], {left: 246, top: editor.getLineHeightInPixels() * 1}] - ] - - for [point, position] in conversionTable - expect( - linesYardstick.pixelPositionForScreenPosition(point) - ).toEqual(position) diff --git a/spec/text-editor-presenter-spec.coffee b/spec/text-editor-presenter-spec.coffee index 98b12930e..e497bf2e9 100644 --- a/spec/text-editor-presenter-spec.coffee +++ b/spec/text-editor-presenter-spec.coffee @@ -4,15 +4,17 @@ TextBuffer = require 'text-buffer' {Point, Range} = TextBuffer TextEditor = require '../src/text-editor' TextEditorPresenter = require '../src/text-editor-presenter' +LinesYardstick = require '../src/lines-yardstick' -fdescribe "TextEditorPresenter", -> - [buffer, editor] = [] +describe "TextEditorPresenter", -> + [buffer, editor, linesYardstick, mockLineNodesProvider] = [] beforeEach -> # These *should* be mocked in the spec helper, but changing that now would break packages :-( spyOn(window, "setInterval").andCallFake window.fakeSetInterval spyOn(window, "clearInterval").andCallFake window.fakeClearInterval + mockLineNodesProvider = {updateSync: ->} buffer = new TextBuffer(filePath: require.resolve('./fixtures/sample.js')) editor = new TextEditor({buffer}) waitsForPromise -> buffer.load() @@ -37,7 +39,10 @@ fdescribe "TextEditorPresenter", -> scrollTop: 0 scrollLeft: 0 - new TextEditorPresenter(params) + presenter = new TextEditorPresenter(params) + linesYardstick = new LinesYardstick(editor, presenter, mockLineNodesProvider) + presenter.setLinesYardstick(linesYardstick) + presenter expectValues = (actual, expected) -> for key, value of expected @@ -55,19 +60,6 @@ fdescribe "TextEditorPresenter", -> expectNoStateUpdate = (presenter, fn) -> expectStateUpdatedToBe(false, presenter, fn) - describe "::getStateForMeasurements(screenRows)", -> - it "contains states for tiles of the visible rows + the supplied ones", -> - presenter = buildPresenter(explicitHeight: 6, scrollTop: 0, lineHeight: 1, tileSize: 2) - state = presenter.getStateForMeasurements([10, 11]) - - expect(state.content.tiles[0]).toBeDefined() - expect(state.content.tiles[2]).toBeDefined() - expect(state.content.tiles[4]).toBeDefined() - expect(state.content.tiles[6]).toBeDefined() - expect(state.content.tiles[8]).toBeUndefined() - expect(state.content.tiles[10]).toBeDefined() - expect(state.content.tiles[12]).toBeUndefined() - # These `describe` and `it` blocks mirror the structure of the ::state object. # Please maintain this structure when adding specs for new state fields. describe "::getState()", -> diff --git a/src/lines-component.coffee b/src/lines-component.coffee index 237f24958..b3e83fc40 100644 --- a/src/lines-component.coffee +++ b/src/lines-component.coffee @@ -97,3 +97,7 @@ class LinesComponent extends TiledComponent component.clearMeasurements() @presenter.clearScopedCharacterWidths() + + lineNodeForLineIdAndScreenRow: (lineId, screenRow) -> + tile = @presenter.tileForRow(screenRow) + @getComponentForTile(screenRow)?.lineNodeForLineId(lineId) diff --git a/src/lines-tile-component.coffee b/src/lines-tile-component.coffee index 3c70c1199..28afe7d5c 100644 --- a/src/lines-tile-component.coffee +++ b/src/lines-tile-component.coffee @@ -399,3 +399,5 @@ class LinesTileComponent clearMeasurements: -> @measuredLines.clear() + + lineNodeForLineId: (id) -> @lineNodesByLineId[id] diff --git a/src/lines-yardstick.coffee b/src/lines-yardstick.coffee index ab1177071..0ec946413 100644 --- a/src/lines-yardstick.coffee +++ b/src/lines-yardstick.coffee @@ -13,8 +13,17 @@ class LinesYardstick @cachedPositionsByLineId = {} prepareScreenRowsForMeasurement: (screenRows) -> - state = @presenter.getStateForMeasurements(screenRows) - @lineNodesProvider.updateSync(state) + @presenter.setScreenRowsToMeasure(screenRows) + @lineNodesProvider.updateSync(@presenter.getStateForMeasurements()) + + cleanup: -> + @presenter.clearScreenRowsToMeasure() + @lineNodesProvider.updateSync(@presenter.getStateForMeasurements()) + + measure: (screenRows, fn) -> + @prepareScreenRowsForMeasurement(screenRows) + fn() + @cleanup() pixelPositionForScreenPosition: (screenPosition, clip=true) -> screenPosition = Point.fromObject(screenPosition) @@ -47,7 +56,7 @@ class LinesYardstick @tokenIterator.reset(tokenizedLine) while @tokenIterator.next() - break if indexWithinTextNode? + break if foundIndexWithinTextNode? text = @tokenIterator.getText() @@ -75,16 +84,16 @@ class LinesYardstick nextTextNodeIndex = textNodeIndex + textNodeLength if charIndex is column - indexWithinTextNode = charIndex - textNodeIndex + foundIndexWithinTextNode = charIndex - textNodeIndex break charIndex += charLength if textNode? - indexWithinTextNode ?= textNode.textContent.length + foundIndexWithinTextNode ?= textNode.textContent.length @cachedPositionsByLineId[tokenizedLine.id] ?= {} @cachedPositionsByLineId[tokenizedLine.id][column] = - @leftPixelPositionForCharInTextNode(lineNode, textNode, indexWithinTextNode) + @leftPixelPositionForCharInTextNode(lineNode, textNode, foundIndexWithinTextNode) else 0 diff --git a/src/text-editor-component.coffee b/src/text-editor-component.coffee index aa198bb3c..39b0e1ee4 100644 --- a/src/text-editor-component.coffee +++ b/src/text-editor-component.coffee @@ -13,6 +13,7 @@ ScrollbarComponent = require './scrollbar-component' ScrollbarCornerComponent = require './scrollbar-corner-component' OverlayManager = require './overlay-manager' DOMElementPool = require './dom-element-pool' +LinesYardstick = require './lines-yardstick' module.exports = class TextEditorComponent @@ -79,6 +80,9 @@ class TextEditorComponent @linesComponent = new LinesComponent({@presenter, @hostElement, @useShadowDOM, @domElementPool}) @scrollViewNode.appendChild(@linesComponent.getDomNode()) + @linesYardstick = new LinesYardstick(@editor, @presenter, @linesComponent) + @presenter.setLinesYardstick(@linesYardstick) + @horizontalScrollbarComponent = new ScrollbarComponent({orientation: 'horizontal', onScroll: @onHorizontalScroll}) @scrollViewNode.appendChild(@horizontalScrollbarComponent.getDomNode()) diff --git a/src/text-editor-presenter.coffee b/src/text-editor-presenter.coffee index 9663aaaf1..27ff26190 100644 --- a/src/text-editor-presenter.coffee +++ b/src/text-editor-presenter.coffee @@ -67,16 +67,17 @@ class TextEditorPresenter isBatching: -> @updating is false - getStateForMeasurements: (screenRows) -> + getStateForMeasurements: -> @updateVerticalDimensions() @updateScrollbarDimensions() @updateStartRow() @updateEndRow() - screenRows = _.without(screenRows, null, undefined, @getVisibleRows()...) + @updateLineDecorations() if @shouldUpdateDecorations + @updateTilesState() if @shouldUpdateLineNumbersState or @shouldUpdateLinesState - @updateVisibleTilesState() if @shouldUpdateLineNumbersState or @shouldUpdateLinesState - @updateTilesState(screenRows, true) + @shouldUpdateLinesState = false + @shouldUpdateLineNumbersState = false @state @@ -85,29 +86,25 @@ class TextEditorPresenter getState: -> @updating = true - @linesYardstick.prepareScreenRowsForMeasurement([ - @model.getLongestScreenRow() - ]) - @deleteTemporaryTiles() + @linesYardstick.measure [@model.getLongestScreenRow()], => + @updateCommonGutterState() + @updateHorizontalDimensions() + @updateReflowState() - @updateCommonGutterState() - @updateHorizontalDimensions() - @updateReflowState() + @updateFocusedState() if @shouldUpdateFocusedState + @updateHeightState() if @shouldUpdateHeightState + @updateVerticalScrollState() if @shouldUpdateVerticalScrollState + @updateHorizontalScrollState() if @shouldUpdateHorizontalScrollState + @updateScrollbarsState() if @shouldUpdateScrollbarsState + @updateHiddenInputState() if @shouldUpdateHiddenInputState + @updateContentState() if @shouldUpdateContentState + @updateHighlightDecorations() if @shouldUpdateDecorations + @updateCursorsState() if @shouldUpdateCursorsState + @updateOverlaysState() if @shouldUpdateOverlaysState + @updateLineNumberGutterState() if @shouldUpdateLineNumberGutterState + @updateGutterOrderState() if @shouldUpdateGutterOrderState + @updateCustomGutterDecorationState() if @shouldUpdateCustomGutterDecorationState - @updateFocusedState() if @shouldUpdateFocusedState - @updateHeightState() if @shouldUpdateHeightState - @updateVerticalScrollState() if @shouldUpdateVerticalScrollState - @updateHorizontalScrollState() if @shouldUpdateHorizontalScrollState - @updateScrollbarsState() if @shouldUpdateScrollbarsState - @updateHiddenInputState() if @shouldUpdateHiddenInputState - @updateContentState() if @shouldUpdateContentState - @updateDecorations() if @shouldUpdateDecorations - @updateVisibleTilesState() if @shouldUpdateLinesState or @shouldUpdateLineNumbersState - @updateCursorsState() if @shouldUpdateCursorsState - @updateOverlaysState() if @shouldUpdateOverlaysState - @updateLineNumberGutterState() if @shouldUpdateLineNumberGutterState - @updateGutterOrderState() if @shouldUpdateGutterOrderState - @updateCustomGutterDecorationState() if @shouldUpdateCustomGutterDecorationState @updating = false @resetTrackedUpdates() @@ -354,25 +351,31 @@ class TextEditorPresenter @tileForRow(@model.getScreenLineCount()), @tileForRow(@endRow) ) - getVisibleRows: -> + getVisibleScreenRows: -> startRow = @getStartTileRow() endRow = Math.min(@model.getScreenLineCount(), @getEndTileRow() + @tileSize) [startRow...endRow] - updateVisibleTilesState: -> - @updateTilesState(@getVisibleRows()) + getScreenRows: -> + screenRows = @getVisibleScreenRows().concat(@screenRowsToMeasure) + screenRows.sort (a, b) -> a - b + _.uniq(screenRows, true) - deleteTemporaryTiles: -> - for tileId, tileState of @state.content.tiles when tileState.temporary - delete @state.content.tiles[tileId] - delete @lineNumberGutter.tiles[tileId] + setScreenRowsToMeasure: (screenRows) -> + @screenRowsToMeasure = screenRows + @shouldUpdateLinesState = true + @shouldUpdateDecorations = true - updateTilesState: (screenRows, temporary = false) -> + clearScreenRowsToMeasure: -> + @screenRowsToMeasure = [] + @shouldUpdateLinesState = true + @shouldUpdateDecorations = true + + updateTilesState: -> return unless @startRow? and @endRow? and @lineHeight? - screenRows.sort (a, b) -> a - b - + screenRows = @getScreenRows() visibleTiles = {} startRow = screenRows[0] endRow = screenRows[screenRows.length - 1] @@ -398,14 +401,12 @@ class TextEditorPresenter tile.display = "block" tile.zIndex = zIndex tile.highlights ?= {} - tile.temporary = temporary gutterTile = @lineNumberGutter.tiles[tileStartRow] ?= {} gutterTile.top = tileStartRow * @lineHeight - @scrollTop gutterTile.height = @tileSize * @lineHeight gutterTile.display = "block" gutterTile.zIndex = zIndex - gutterTile.temporary = temporary @updateLinesState(tile, rowsWithinTile) if @shouldUpdateLinesState @updateLineNumbersState(gutterTile, rowsWithinTile) if @shouldUpdateLineNumbersState @@ -413,8 +414,6 @@ class TextEditorPresenter visibleTiles[tileStartRow] = true zIndex++ - return if temporary - if @mouseWheelScreenRow? and @model.tokenizedLineForScreenRow(@mouseWheelScreenRow)? mouseWheelTile = @tileForRow(@mouseWheelScreenRow) @@ -1194,22 +1193,30 @@ class TextEditorPresenter @emitDidUpdateState() - updateDecorations: -> + updateLineDecorations: -> @rangesByDecorationId = {} @lineDecorationsByScreenRow = {} @lineNumberDecorationsByScreenRow = {} @customGutterDecorationsByGutterNameAndScreenRow = {} + + return unless 0 <= @startRow <= @endRow <= Infinity + + for row in @getScreenRows() + for markerId, decorations of @model.decorationsForScreenRowRange(row, row) + range = @model.getMarker(markerId).getScreenRange() + for decoration in decorations + if decoration.isType('line') or decoration.isType('gutter') + @addToLineDecorationCaches(decoration, range) + + updateHighlightDecorations: -> @visibleHighlights = {} return unless 0 <= @startRow <= @endRow <= Infinity for markerId, decorations of @model.decorationsForScreenRowRange(@startRow, @endRow - 1) range = @model.getMarker(markerId).getScreenRange() - for decoration in decorations - if decoration.isType('line') or decoration.isType('gutter') - @addToLineDecorationCaches(decoration, range) - else if decoration.isType('highlight') - @updateHighlightState(decoration, range) + for decoration in decorations when decoration.isType('highlight') + @updateHighlightState(decoration, range) for tileId, tileState of @state.content.tiles for id, highlight of tileState.highlights