Char measurement

spec/display-buffer-spec.coffee

@@ -1159,9 +1159,7 @@ describe "DisplayBuffer", ->
 
         displayBuffer.setLineHeightInPixels(20)
         displayBuffer.setDefaultCharWidth(10)
-
-        for char in ['r', 'e', 't', 'u', 'r', 'n']
-          displayBuffer.setScopedCharWidth(["source.js", "keyword.control.js"], char, 11)
+        displayBuffer.setCharLeftPositionForPoint(5, i - 1, i * 11) for i in [1..6]
 
         {start, end} = marker.getPixelRange()
         expect(start.top).toBe 5 * 20
@@ -1325,21 +1323,23 @@ describe "DisplayBuffer", ->
       buffer.delete([[6, 10], [6, 12]], ' ')
       expect(displayBuffer.getScrollWidth()).toBe 10 * 64 + cursorWidth
 
-    it "recomputes the scroll width when the scoped character widths change", ->
-      operatorWidth = 20
-      displayBuffer.setScopedCharWidth(['source.js', 'keyword.operator.js'], '<', operatorWidth)
-      expect(displayBuffer.getScrollWidth()).toBe 10 * 64 + operatorWidth + cursorWidth
+    it "recomputes the scroll width when character widths change", ->
+      charWidth = 20
+      displayBuffer.setCharLeftPositionForPoint(6, 0, charWidth)
+      expect(displayBuffer.getScrollWidth()).toBe 10 * 64 + charWidth + cursorWidth
 
-    it "recomputes the scroll width when the scoped character widths change in a batch", ->
-      operatorWidth = 20
+    it "recomputes the scroll width when widths change in a batch", ->
+      charWidth = 20
 
       displayBuffer.onDidChangeCharacterWidths changedSpy = jasmine.createSpy()
 
       displayBuffer.batchCharacterMeasurement ->
-        displayBuffer.setScopedCharWidth(['source.js', 'keyword.operator.js'], '<', operatorWidth)
-        displayBuffer.setScopedCharWidth(['source.js', 'keyword.operator.js'], '?', operatorWidth)
+        displayBuffer.setCharLeftPositionForPoint(6, 0, charWidth)
+        displayBuffer.setCharLeftPositionForPoint(6, 0, charWidth)
+        displayBuffer.setCharLeftPositionForPoint(6, 1, charWidth * 2)
+
 
-      expect(displayBuffer.getScrollWidth()).toBe 10 * 63 + operatorWidth * 2 + cursorWidth
+      expect(displayBuffer.getScrollWidth()).toBe 10 * 63 + charWidth * 2 + cursorWidth
       expect(changedSpy.callCount).toBe 1
 
   describe "::getVisibleRowRange()", ->

spec/text-editor-component-spec.coffee

@@ -250,8 +250,8 @@ describe "TextEditorComponent", ->
       nextAnimationFrame()
 
       leafNodes = getLeafNodes(component.lineNodeForScreenRow(0))
-      expect(leafNodes[0].classList.contains('trailing-whitespace')).toBe true
-      expect(leafNodes[0].classList.contains('leading-whitespace')).toBe false
+      expect(leafNodes[1].classList.contains('trailing-whitespace')).toBe true
+      expect(leafNodes[1].classList.contains('leading-whitespace')).toBe false
 
     describe "when showInvisibles is enabled", ->
       invisibles = null
@@ -292,7 +292,8 @@ describe "TextEditorComponent", ->
       it "displays newlines as their own token outside of the other tokens' scopeDescriptor", ->
         editor.setText "var\n"
         nextAnimationFrame()
-        expect(component.lineNodeForScreenRow(0).innerHTML).toBe "<span class=\"source js\"><span class=\"storage modifier js\">var</span></span><span class=\"invisible-character\">#{invisibles.eol}</span>"
+        tokenId = editor.tokenizedLineForScreenRow(0).tokens[0].id
+        expect(component.lineNodeForScreenRow(0).innerHTML).toBe "<span id=\"token-#{tokenId}\" class=\"source js storage modifier js\">var</span><span class=\"invisible-character\">#{invisibles.eol}</span>"
 
       it "displays trailing carriage returns using a visible, non-empty value", ->
         editor.setText "a line that ends with a carriage return\r\n"
@@ -767,10 +768,28 @@ describe "TextEditorComponent", ->
       cursor = componentNode.querySelector('.cursor')
       cursorRect = cursor.getBoundingClientRect()
 
-      cursorLocationTextNode = component.lineNodeForScreenRow(0).querySelector('.source.js').firstChild
+      cursorLocationTextNode = component.lineNodeForScreenRow(0)
+      range = document.createRange()
+      range.setStart(cursorLocationTextNode, 2)
+      range.setEnd(cursorLocationTextNode, 3)
+      rangeRect = range.getBoundingClientRect()
+
+      expect(cursorRect.left).toBe rangeRect.left
+      expect(cursorRect.width).toBe rangeRect.width
+
+    it "accounts for the width of larger characters when positioning cursors", ->
+      atom.config.set('editor.fontFamily', 'sans-serif')
+      editor.setText('a👊b')
+      editor.setCursorBufferPosition([0, 3])
+      nextAnimationFrame()
+
+      cursor = componentNode.querySelector('.cursor')
+      cursorRect = cursor.getBoundingClientRect()
+
+      cursorLocationTextNode = component.lineNodeForScreenRow(0)
       range = document.createRange()
-      range.setStart(cursorLocationTextNode, 3)
-      range.setEnd(cursorLocationTextNode, 4)
+      range.setStart(cursorLocationTextNode, 2)
+      range.setEnd(cursorLocationTextNode, 3)
       rangeRect = range.getBoundingClientRect()
 
       expect(cursorRect.left).toBe rangeRect.left

spec/text-editor-presenter-spec.coffee

@@ -147,15 +147,19 @@ describe "TextEditorPresenter", ->
           expectStateUpdate presenter, -> presenter.setBaseCharacterWidth(15)
           expect(presenter.getState().horizontalScrollbar.scrollWidth).toBe 15 * maxLineLength + 1
 
-        it "updates when the scoped character widths change", ->
+        it "updates according to the measured character widths", ->
           waitsForPromise -> atom.packages.activatePackage('language-javascript')
 
           runs ->
             maxLineLength = editor.getMaxScreenLineLength()
             presenter = buildPresenter(contentFrameWidth: 50, baseCharacterWidth: 10)
 
             expect(presenter.getState().horizontalScrollbar.scrollWidth).toBe 10 * maxLineLength + 1
-            expectStateUpdate presenter, -> presenter.setScopedCharacterWidth(['source.js', 'support.function.js'], 'p', 20)
+
+            editor.setCharLeftPositionForPoint(6, 0, 20)
+            editor.setCharLeftPositionForPoint(6, 1, 40)
+            editor.setScrollLeft(1) # triggers a state update
+
             expect(presenter.getState().horizontalScrollbar.scrollWidth).toBe (10 * (maxLineLength - 2)) + (20 * 2) + 1 # 2 of the characters are 20px wide now instead of 10px wide
 
         it "updates when ::softWrapped changes on the editor", ->
@@ -386,7 +390,10 @@ describe "TextEditorPresenter", ->
             expectStateUpdate presenter, -> presenter.setBaseCharacterWidth(15)
             expect(presenter.getState().hiddenInput.width).toBe 15
 
-            expectStateUpdate presenter, -> presenter.setScopedCharacterWidth(['source.js', 'storage.modifier.js'], 'r', 20)
+            presenter.setCharLeftPositionForPoint(3, i - 1, i * 15) for i in [1..6]
+            presenter.setCharLeftPositionForPoint(3, 6, 6 * 15 + 20)
+
+            editor.setScrollLeft(1) # triggers a state update
             expect(presenter.getState().hiddenInput.width).toBe 20
 
         it "is 2px at the end of lines", ->
@@ -469,15 +476,19 @@ describe "TextEditorPresenter", ->
           expectStateUpdate presenter, -> presenter.setBaseCharacterWidth(15)
           expect(presenter.getState().content.scrollWidth).toBe 15 * maxLineLength + 1
 
-        it "updates when the scoped character widths change", ->
+        it "updates according to the measured character widths", ->
           waitsForPromise -> atom.packages.activatePackage('language-javascript')
 
           runs ->
             maxLineLength = editor.getMaxScreenLineLength()
             presenter = buildPresenter(contentFrameWidth: 50, baseCharacterWidth: 10)
 
             expect(presenter.getState().content.scrollWidth).toBe 10 * maxLineLength + 1
-            expectStateUpdate presenter, -> presenter.setScopedCharacterWidth(['source.js', 'support.function.js'], 'p', 20)
+
+            editor.setCharLeftPositionForPoint(6, 0, 20)
+            editor.setCharLeftPositionForPoint(6, 1, 40)
+            editor.setScrollLeft(1) # triggers a state update
+
             expect(presenter.getState().content.scrollWidth).toBe (10 * (maxLineLength - 2)) + (20 * 2) + 1 # 2 of the characters are 20px wide now instead of 10px wide
 
         it "updates when ::softWrapped changes on the editor", ->
@@ -853,6 +864,58 @@ describe "TextEditorPresenter", ->
           editor.setText('')
 
         describe "[lineId]", -> # line state objects
+          describe ".shouldMeasure", ->
+            presenter = null
+
+            beforeEach ->
+              presenter = buildPresenter(explicitHeight: 30, scrollTop: 0, lineHeight: 10)
+
+            describe "when the model is changed", ->
+              it "is true when line is first built (not scrolling)", ->
+                expect(lineStateForScreenRow(presenter, 0).shouldMeasure).toBe(true)
+
+              it "is true when line gets built after scrolling because of an insertion", ->
+                editor.setText("")
+                editor.insertNewline()
+                editor.insertNewline()
+                editor.insertNewline()
+
+                expect(lineStateForScreenRow(presenter, 2).shouldMeasure).toBe(true)
+
+              it "is true when line gets built after scrolling because a cursor has been moved", ->
+                editor.setCursorScreenPosition([4, 0], autoscroll: true)
+
+                expect(lineStateForScreenRow(presenter, 4).shouldMeasure).toBe(true)
+
+              it "is true when line gets built after scrolling because a cursor has been added", ->
+                editor.addCursorAtScreenPosition([4, 0])
+
+                expect(lineStateForScreenRow(presenter, 4).shouldMeasure).toBe(true)
+
+              it "is true when line gets built after scrolling because a selection has been added", ->
+                editor.setSelectedScreenRanges([
+                  [[0, 0], [0, 2]],
+                  [[4, 0], [4, 2]]
+                ])
+
+                expect(lineStateForScreenRow(presenter, 4).shouldMeasure).toBe(true)
+
+            describe "when the model is not changed", ->
+              it "is false by default, and becomes true only when the line stays on screen for a sufficient number of times", ->
+                presenter.setScrollTop(10)
+                expect(lineStateForScreenRow(presenter, 4).shouldMeasure).toBe(false)
+                presenter.setScrollTop(20)
+                expect(lineStateForScreenRow(presenter, 4).shouldMeasure).toBe(false)
+                presenter.setScrollTop(30)
+                expect(lineStateForScreenRow(presenter, 4).shouldMeasure).toBe(true)
+
+            it "is false if it has already been measured", ->
+              editor.setText("Hello")
+              editor.insertNewline()
+              expect(lineStateForScreenRow(presenter, 0).shouldMeasure).toBe(true)
+              editor.insertNewline()
+              expect(lineStateForScreenRow(presenter, 0).shouldMeasure).toBe(false)
+
           it "includes the .endOfLineInvisibles if the editor.showInvisibles config option is true", ->
             editor.setText("hello\nworld\r\n")
             presenter = buildPresenter(explicitHeight: 25, scrollTop: 0, lineHeight: 10)
@@ -1087,18 +1150,23 @@ describe "TextEditorPresenter", ->
           expectStateUpdate presenter, -> presenter.setBaseCharacterWidth(20)
           expect(stateForCursor(presenter, 0)).toEqual {top: 2 * 10, left: 4 * 20, width: 20, height: 10}
 
-        it "updates when scoped character widths change", ->
+        it "updates according to the measured character widths", ->
           waitsForPromise ->
             atom.packages.activatePackage('language-javascript')
 
           runs ->
             editor.setCursorBufferPosition([1, 4])
             presenter = buildPresenter(explicitHeight: 20)
 
-            expectStateUpdate presenter, -> presenter.setScopedCharacterWidth(['source.js', 'storage.modifier.js'], 'v', 20)
+            presenter.setCharLeftPositionForPoint(1, 0, 10)
+            presenter.setCharLeftPositionForPoint(1, 1, 20)
+            presenter.setCharLeftPositionForPoint(1, 2, 40)
+            presenter.setScrollTop(1) # triggers a state update
             expect(stateForCursor(presenter, 0)).toEqual {top: 1 * 10, left: (3 * 10) + 20, width: 10, height: 10}
 
-            expectStateUpdate presenter, -> presenter.setScopedCharacterWidth(['source.js', 'storage.modifier.js'], 'r', 20)
+            presenter.setCharLeftPositionForPoint(1, 3, 50)
+            presenter.setCharLeftPositionForPoint(1, 4, 70)
+            presenter.setScrollTop(0) # triggers a state update
             expect(stateForCursor(presenter, 0)).toEqual {top: 1 * 10, left: (3 * 10) + 20, width: 20, height: 10}
 
         it "updates when cursors are added, moved, hidden, shown, or destroyed", ->
@@ -1388,7 +1456,7 @@ describe "TextEditorPresenter", ->
             regions: [{top: 2 * 10, left: 2 * 20, width: 2 * 20, height: 10}]
           }
 
-        it "updates when scoped character widths change", ->
+        it "updates according to the measured character widths", ->
           waitsForPromise ->
             atom.packages.activatePackage('language-javascript')
 
@@ -1402,7 +1470,10 @@ describe "TextEditorPresenter", ->
             expectValues stateForSelection(presenter, 0), {
               regions: [{top: 2 * 10, left: 4 * 10, width: 2 * 10, height: 10}]
             }
-            expectStateUpdate presenter, -> presenter.setScopedCharacterWidth(['source.js', 'keyword.control.js'], 'i', 20)
+
+            presenter.setCharLeftPositionForPoint(2, i - 1, i * 10) for i in [1..4]
+            presenter.setCharLeftPositionForPoint(2, 4, 60)
+            editor.setScrollTop(1) # triggers a state update
             expectValues stateForSelection(presenter, 0), {
               regions: [{top: 2 * 10, left: 4 * 10, width: 20 + 10, height: 10}]
             }

src/display-buffer.coffee

@@ -22,7 +22,7 @@ class DisplayBuffer extends Model
 
   verticalScrollMargin: 2
   horizontalScrollMargin: 6
-  scopedCharacterWidthsChangeCount: 0
+  characterWidthsChanged: false
 
   constructor: ({tabLength, @editorWidthInChars, @tokenizedBuffer, buffer, @invisibles}={}) ->
     super
@@ -32,7 +32,7 @@ class DisplayBuffer extends Model
 
     @tokenizedBuffer ?= new TokenizedBuffer({tabLength, buffer, @invisibles})
     @buffer = @tokenizedBuffer.buffer
-    @charWidthsByScope = {}
+    @charWidthsByRow = {}
     @markers = {}
     @foldsByMarkerId = {}
     @decorationsById = {}
@@ -293,36 +293,39 @@ class DisplayBuffer extends Model
 
   getCursorWidth: -> 1
 
-  getScopedCharWidth: (scopeNames, char) ->
-    @getScopedCharWidths(scopeNames)[char]
-
-  getScopedCharWidths: (scopeNames) ->
-    scope = @charWidthsByScope
-    for scopeName in scopeNames
-      scope[scopeName] ?= {}
-      scope = scope[scopeName]
-    scope.charWidths ?= {}
-    scope.charWidths
-
   batchCharacterMeasurement: (fn) ->
-    oldChangeCount = @scopedCharacterWidthsChangeCount
     @batchingCharacterMeasurement = true
     fn()
     @batchingCharacterMeasurement = false
-    @characterWidthsChanged() if oldChangeCount isnt @scopedCharacterWidthsChangeCount
+    @handleCharacterWidthsChanged() if @characterWidthsChanged
 
-  setScopedCharWidth: (scopeNames, char, width) ->
-    @getScopedCharWidths(scopeNames)[char] = width
-    @scopedCharacterWidthsChangeCount++
-    @characterWidthsChanged() unless @batchingCharacterMeasurement
+  setCharLeftPositionForPoint: (row, column, charWidth) ->
+    @charWidthsByRow[row] ?= {}
+    @charWidthsByRow[row][column] = charWidth
+    @characterWidthsChanged = true
+    @handleCharacterWidthsChanged() unless @batchingCharacterMeasurement
 
-  characterWidthsChanged: ->
-    @computeScrollWidth()
-    @emit 'character-widths-changed', @scopedCharacterWidthsChangeCount if Grim.includeDeprecatedAPIs
-    @emitter.emit 'did-change-character-widths', @scopedCharacterWidthsChangeCount
+  getCharLeftPositionForPoint: (row, column) ->
+    rowCharWidths = @charWidthsByRow[row]
+
+    return @getDefaultCharWidth() * (column + 1) unless rowCharWidths?
+
+    measuredCharWidth = rowCharWidths[column]
+    if measuredCharWidth?
+      measuredCharWidth
+    else
+      # TODO: should really keep this stuff here? We should *never* have a
+      # situation where a character is not yet measured.
+      [..., last] = Object.keys(rowCharWidths)
+      rowCharWidths[last] + @getDefaultCharWidth() * (column - last)
+
+  handleCharacterWidthsChanged: ->
+    return unless @characterWidthsChanged
+    @characterWidthsChanged = false
 
-  clearScopedCharWidths: ->
-    @charWidthsByScope = {}
+    @computeScrollWidth()
+    @emit 'character-widths-changed'
+    @emitter.emit 'did-change-character-widths'
 
   getScrollHeight: ->
     lineHeight = @getLineHeightInPixels()
@@ -646,13 +649,14 @@ class DisplayBuffer extends Model
 
     targetRow = screenPosition.row
     targetColumn = screenPosition.column
-    defaultCharWidth = @defaultCharWidth
+
+    tokenizedLine = @tokenizedLineForScreenRow(targetRow)
 
     top = targetRow * @lineHeightInPixels
     left = 0
     column = 0
-    for token in @tokenizedLineForScreenRow(targetRow).tokens
-      charWidths = @getScopedCharWidths(token.scopes)
+
+    for token in tokenizedLine.tokens
       valueIndex = 0
       while valueIndex < token.value.length
         if token.hasPairedCharacter
@@ -665,8 +669,9 @@ class DisplayBuffer extends Model
           valueIndex++
 
         return {top, left} if column is targetColumn
-        left += charWidths[char] ? defaultCharWidth unless char is '\0'
+        left = @getCharLeftPositionForPoint(targetRow, column) unless char is '\0'
         column += charLength
+
     {top, left}
 
   screenPositionForPixelPosition: (pixelPosition) ->
@@ -679,10 +684,12 @@ class DisplayBuffer extends Model
     row = Math.min(row, @getLastRow())
     row = Math.max(0, row)
 
+    tokenizedLine = @tokenizedLineForScreenRow(row)
+
     left = 0
     column = 0
-    for token in @tokenizedLineForScreenRow(row).tokens
-      charWidths = @getScopedCharWidths(token.scopes)
+
+    for token in tokenizedLine.tokens
       valueIndex = 0
       while valueIndex < token.value.length
         if token.hasPairedCharacter
@@ -694,9 +701,9 @@ class DisplayBuffer extends Model
           charLength = 1
           valueIndex++
 
-        charWidth = charWidths[char] ? defaultCharWidth
-        break if targetLeft <= left + (charWidth / 2)
-        left += charWidth
+        nextLeft = @getCharLeftPositionForPoint(row, column)
+        break if targetLeft <= (left + nextLeft) / 2
+        left = nextLeft
         column += charLength
 
     new Point(row, column)