Skip to content

Commit

Permalink
🐎 Speed up scrolling with lots of decorations
Browse files Browse the repository at this point in the history
Use case: Opening bootstrap.min.css with `minimap-pigments` enabled and
scrolling make the UI all laggy due to the permanent computation of the
decoration marker’s screen range.

Now we’ll cache the mapping type->rows->decorations for the whole
buffer and rebuild it each time a decoration is added/removed or a
marker change.
  • Loading branch information
abe33 committed Aug 12, 2015
1 parent ffdcecc commit 7b495e9
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 22 deletions.
2 changes: 1 addition & 1 deletion lib/mixins/canvas-drawer.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ class CanvasDrawer extends Mixin
charWidth = @minimap.getCharWidth() * devicePixelRatio
canvasWidth = @canvas.width
displayCodeHighlights = @displayCodeHighlights
decorations = @minimap.decorationsForScreenRowRangeByTypeThenRows(firstRow, lastRow)
decorations = @minimap.decorationsByTypeThenRows(firstRow, lastRow)

line = lines[0]

Expand Down
44 changes: 25 additions & 19 deletions lib/mixins/decoration-management.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -117,22 +117,26 @@ class DecorationManagement extends Mixin
# endScreenRow - The ending row index.
#
# Returns an {Object}.
decorationsForScreenRowRangeByTypeThenRows: (startScreenRow, endScreenRow) ->
decorationsByMarkerType = {}
for marker in @findMarkers(intersectsScreenRowRange: [startScreenRow, endScreenRow])
if decorations = @decorationsByMarkerId[marker.id]
range = marker.getScreenRange()
rows = [range.start.row..range.end.row]
decorationsByTypeThenRows: (startScreenRow, endScreenRow) ->
return @decorationsByTypeThenRowsCache if @decorationsByTypeThenRowsCache?

for decoration in decorations
{type} = decoration.getProperties()
decorationsByMarkerType[type] ?= {}
cache = {}

for id, decoration of @decorationsById
range = decoration.marker.getScreenRange()
rows = [range.start.row..range.end.row]

{type} = decoration.getProperties()
cache[type] ?= {}

for row in rows
cache[type][row] ?= []
cache[type][row].push(decoration)

for row in rows
decorationsByMarkerType[type][row] ?= []
decorationsByMarkerType[type][row].push(decoration)
@decorationsByTypeThenRowsCache = cache

decorationsByMarkerType
invalidateDecorationForScreenRowsCache: ->
@decorationsByTypeThenRowsCache = null

# Public: Adds a decoration that tracks a `Marker`. When the marker moves,
# is invalidated, or is destroyed, the decoration will be updated to reflect
Expand Down Expand Up @@ -186,6 +190,7 @@ class DecorationManagement extends Mixin

@decorationMarkerChangedSubscriptions[marker.id] ?= marker.onDidChange (event) =>
decorations = @decorationsByMarkerId[marker.id]
@invalidateDecorationForScreenRowsCache()

# Why check existence? Markers may get destroyed or decorations removed
# in the change handler. Bookmarks does this.
Expand Down Expand Up @@ -241,6 +246,7 @@ class DecorationManagement extends Mixin
# decoration - The `Decoration` to register changes for.
emitDecorationChanges: (decoration) ->
return if decoration.marker.displayBuffer.isDestroyed()
@invalidateDecorationForScreenRowsCache()
range = decoration.marker.getScreenRange()
return unless range?

Expand Down Expand Up @@ -269,21 +275,21 @@ class DecorationManagement extends Mixin
removeDecoration: (decoration) ->
return unless decoration?
{marker} = decoration
return unless decorations = @decorationsByMarkerId[marker.id]

@emitDecorationChanges(decoration)
delete @decorationsById[decoration.id]

@decorationUpdatedSubscriptions[decoration.id].dispose()
@decorationDestroyedSubscriptions[decoration.id].dispose()
@decorationUpdatedSubscriptions[decoration.id]?.dispose()
@decorationDestroyedSubscriptions[decoration.id]?.dispose()

delete @decorationUpdatedSubscriptions[decoration.id]
delete @decorationDestroyedSubscriptions[decoration.id]

return unless decorations = @decorationsByMarkerId[marker.id]

@emitDecorationChanges(decoration)
index = decorations.indexOf(decoration)

if index > -1
decorations.splice(index, 1)
delete @decorationsById[decoration.id]
@emitter.emit 'did-remove-decoration', {marker, decoration}
@removedAllMarkerDecorations(marker) if decorations.length is 0

Expand Down
4 changes: 2 additions & 2 deletions spec/minimap-spec.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -296,7 +296,7 @@ describe 'Minimap', ->

expect(decoration).toBeUndefined()

describe '::decorationsForScreenRowRangeByTypeThenRows', ->
describe '::decorationsByTypeThenRows', ->
[decorations] = []

beforeEach ->
Expand All @@ -313,7 +313,7 @@ describe 'Minimap', ->
createDecoration 'line', [[12,0], [12,0]]
createDecoration 'highlight-under', [[0,0], [10,1]]

decorations = minimap.decorationsForScreenRowRangeByTypeThenRows(0, 12)
decorations = minimap.decorationsByTypeThenRows(0, 12)

it 'returns an object whose keys are the decorations types', ->
expect(Object.keys(decorations).sort()).toEqual(['highlight-over', 'highlight-under', 'line'])
Expand Down

0 comments on commit 7b495e9

Please sign in to comment.