From 8add5ccd7efca08d0052adf8783bf536771711c3 Mon Sep 17 00:00:00 2001 From: Nathan Sobo Date: Wed, 2 Jul 2014 08:46:56 -0600 Subject: [PATCH 1/5] Remove unused EditorComponent::componentWillUpdate hook --- src/editor-component.coffee | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/editor-component.coffee b/src/editor-component.coffee index 8e1efdd3e..28cccfe33 100644 --- a/src/editor-component.coffee +++ b/src/editor-component.coffee @@ -183,8 +183,6 @@ EditorComponent = React.createClass clearInterval(@scrollViewMeasurementIntervalId) @scrollViewMeasurementIntervalId = null - componentWillUpdate: -> - componentDidUpdate: (prevProps, prevState) -> cursorsMoved = @cursorsMoved selectionChanged = @selectionChanged From e5ab2c65072d8b0c08189450e9136ef5bd1569c2 Mon Sep 17 00:00:00 2001 From: Nathan Sobo Date: Wed, 2 Jul 2014 08:47:32 -0600 Subject: [PATCH 2/5] Pause scroll view measurement when requesting animation frames We don't want any extra DOM reading that could introduce hitches in the animation we're running. --- src/editor-component.coffee | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/src/editor-component.coffee b/src/editor-component.coffee index 28cccfe33..0fc0a7895 100644 --- a/src/editor-component.coffee +++ b/src/editor-component.coffee @@ -44,6 +44,7 @@ EditorComponent = React.createClass inputEnabled: true scrollViewMeasurementInterval: 100 scopedCharacterWidthsChangeCount: null + scrollViewMeasurementPaused: false render: -> {focused, fontSize, lineHeight, fontFamily, showIndentGuide, showInvisibles, showLineNumbers, visible} = @state @@ -163,7 +164,7 @@ EditorComponent = React.createClass componentDidMount: -> {editor} = @props - @scrollViewMeasurementIntervalId = setInterval(@requestScrollViewMeasurement, @scrollViewMeasurementInterval) + @scrollViewMeasurementIntervalId = setInterval(@measureScrollView, @scrollViewMeasurementInterval) @observeEditor() @listenForDOMEvents() @@ -517,6 +518,7 @@ EditorComponent = React.createClass animationFramePending = @pendingScrollTop? @pendingScrollTop = scrollTop unless animationFramePending + @pauseScrollViewMeasurement() requestAnimationFrame => pendingScrollTop = @pendingScrollTop @pendingScrollTop = null @@ -530,6 +532,7 @@ EditorComponent = React.createClass animationFramePending = @pendingScrollLeft? @pendingScrollLeft = scrollLeft unless animationFramePending + @pauseScrollViewMeasurement() requestAnimationFrame => @props.editor.setScrollLeft(@pendingScrollLeft) @pendingScrollLeft = null @@ -551,6 +554,7 @@ EditorComponent = React.createClass @clearMouseWheelScreenRowAfterDelay() unless animationFramePending + @pauseScrollViewMeasurement() requestAnimationFrame => {editor} = @props editor.setScrollTop(editor.getScrollTop() + @pendingVerticalScrollDelta) @@ -679,6 +683,7 @@ EditorComponent = React.createClass dragging = false lastMousePosition = {} animationLoop = => + @pauseScrollViewMeasurement() requestAnimationFrame => if dragging screenPosition = @screenPositionForMouseEvent(lastMousePosition) @@ -706,8 +711,18 @@ EditorComponent = React.createClass window.addEventListener('mousemove', onMouseMove) window.addEventListener('mouseup', onMouseUp) + pauseScrollViewMeasurement: -> + @scrollViewMeasurementPaused = true + @resumeScrollViewMeasurementAfterDelay ?= debounce(@resumeScrollViewMeasurement, 100) + @resumeScrollViewMeasurementAfterDelay() + + resumeScrollViewMeasurement: -> + @scrollViewMeasurementPaused = false + + resumeScrollViewMeasurementAfterDelay: null # created lazily + requestScrollViewMeasurement: -> - return if @measurementPending + return if @scrollViewMeasurementRequested @scrollViewMeasurementRequested = true requestAnimationFrame => @@ -719,6 +734,7 @@ EditorComponent = React.createClass # and use the scrollHeight / scrollWidth as its height and width in # calculations. measureScrollView: -> + return if @scrollViewMeasurementPaused return unless @isMounted() {editor} = @props From 9508909a9f0be9b64a6a2c36098e5b61ef906b9a Mon Sep 17 00:00:00 2001 From: Nathan Sobo Date: Wed, 2 Jul 2014 09:18:59 -0600 Subject: [PATCH 3/5] Don't defer updates with setImmediate in animation frames I previously thought this was okay, but now I'm experiencing jitter when scrolling with the trackpad when updates are deferred, and the frames seem jagged. So this commit restores a synchronous approach to display updates whenever we use animation frames. --- spec/editor-component-spec.coffee | 10 ---------- src/editor-component.coffee | 27 +++++++++++++++++++++------ 2 files changed, 21 insertions(+), 16 deletions(-) diff --git a/spec/editor-component-spec.coffee b/spec/editor-component-spec.coffee index 018d1bcb7..2b1e3c93e 100644 --- a/spec/editor-component-spec.coffee +++ b/spec/editor-component-spec.coffee @@ -84,7 +84,6 @@ describe "EditorComponent", -> verticalScrollbarNode.scrollTop = 4.5 * lineHeightInPixels verticalScrollbarNode.dispatchEvent(new UIEvent('scroll')) - runSetImmediateCallbacks() expect(linesNode.style['-webkit-transform']).toBe "translate3d(0px, #{-4.5 * lineHeightInPixels}px, 0px)" expect(node.querySelectorAll('.line').length).toBe 6 + 4 # margin above and below @@ -329,7 +328,6 @@ describe "EditorComponent", -> verticalScrollbarNode.scrollTop = 2.5 * lineHeightInPixels verticalScrollbarNode.dispatchEvent(new UIEvent('scroll')) - runSetImmediateCallbacks() expect(node.querySelectorAll('.line-number').length).toBe 6 + 4 + 1 # line overdraw margin above/below + dummy line number @@ -520,7 +518,6 @@ describe "EditorComponent", -> verticalScrollbarNode.dispatchEvent(new UIEvent('scroll')) horizontalScrollbarNode.scrollLeft = 3.5 * charWidth horizontalScrollbarNode.dispatchEvent(new UIEvent('scroll')) - runSetImmediateCallbacks() cursorNodes = node.querySelectorAll('.cursor') expect(cursorNodes.length).toBe 2 @@ -877,7 +874,6 @@ describe "EditorComponent", -> verticalScrollbarNode.scrollTop = 3.5 * lineHeightInPixels verticalScrollbarNode.dispatchEvent(new UIEvent('scroll')) - runSetImmediateCallbacks() regions = node.querySelectorAll('.some-highlight .region') @@ -1397,36 +1393,30 @@ describe "EditorComponent", -> expect(horizontalScrollbarNode.scrollLeft).toBe 0 node.dispatchEvent(new WheelEvent('mousewheel', wheelDeltaX: -5, wheelDeltaY: -10)) - runSetImmediateCallbacks() expect(verticalScrollbarNode.scrollTop).toBe 10 expect(horizontalScrollbarNode.scrollLeft).toBe 0 node.dispatchEvent(new WheelEvent('mousewheel', wheelDeltaX: -15, wheelDeltaY: -5)) - runSetImmediateCallbacks() expect(verticalScrollbarNode.scrollTop).toBe 10 expect(horizontalScrollbarNode.scrollLeft).toBe 15 it "updates the scrollLeft or scrollTop according to the scroll sensitivity", -> atom.config.set('editor.scrollSensitivity', 50) node.dispatchEvent(new WheelEvent('mousewheel', wheelDeltaX: -5, wheelDeltaY: -10)) - runSetImmediateCallbacks() expect(horizontalScrollbarNode.scrollLeft).toBe 0 node.dispatchEvent(new WheelEvent('mousewheel', wheelDeltaX: -15, wheelDeltaY: -5)) - runSetImmediateCallbacks() expect(verticalScrollbarNode.scrollTop).toBe 5 expect(horizontalScrollbarNode.scrollLeft).toBe 7 it "uses the previous scrollSensitivity when the value is not an int", -> atom.config.set('editor.scrollSensitivity', 'nope') node.dispatchEvent(new WheelEvent('mousewheel', wheelDeltaX: 0, wheelDeltaY: -10)) - runSetImmediateCallbacks() expect(verticalScrollbarNode.scrollTop).toBe 10 it "parses negative scrollSensitivity values as positive", -> atom.config.set('editor.scrollSensitivity', -50) node.dispatchEvent(new WheelEvent('mousewheel', wheelDeltaX: 0, wheelDeltaY: -10)) - runSetImmediateCallbacks() expect(verticalScrollbarNode.scrollTop).toBe 5 describe "when the mousewheel event's target is a line", -> diff --git a/src/editor-component.coffee b/src/editor-component.coffee index 0fc0a7895..917a8202c 100644 --- a/src/editor-component.coffee +++ b/src/editor-component.coffee @@ -26,6 +26,8 @@ EditorComponent = React.createClass pendingScrollLeft: null selectOnMouseMove: false updateRequested: false + updatesPaused: false + updateRequestedWhilePaused: false cursorsMoved: false selectionChanged: false selectionAdded: false @@ -203,6 +205,10 @@ EditorComponent = React.createClass @remeasureCharacterWidthsIfNeeded(prevState) requestUpdate: -> + if @updatesPaused + @updateRequestedWhilePaused = true + return + if @performSyncUpdates ? EditorComponent.performSyncUpdates @forceUpdate() else unless @updateRequested @@ -211,6 +217,15 @@ EditorComponent = React.createClass @updateRequested = false @forceUpdate() if @isMounted() + requestAnimationFrame: (fn) -> + @updatesPaused = true + requestAnimationFrame => + fn() + @updatesPaused = false + if @updateRequestedWhilePaused and @isMounted() + @updateRequestedWhilePaused = false + @forceUpdate() + getRenderedRowRange: -> {editor, lineOverdrawMargin} = @props [visibleStartRow, visibleEndRow] = editor.getVisibleRowRange() @@ -519,7 +534,7 @@ EditorComponent = React.createClass @pendingScrollTop = scrollTop unless animationFramePending @pauseScrollViewMeasurement() - requestAnimationFrame => + @requestAnimationFrame => pendingScrollTop = @pendingScrollTop @pendingScrollTop = null @props.editor.setScrollTop(pendingScrollTop) @@ -533,7 +548,7 @@ EditorComponent = React.createClass @pendingScrollLeft = scrollLeft unless animationFramePending @pauseScrollViewMeasurement() - requestAnimationFrame => + @requestAnimationFrame => @props.editor.setScrollLeft(@pendingScrollLeft) @pendingScrollLeft = null @@ -555,7 +570,7 @@ EditorComponent = React.createClass unless animationFramePending @pauseScrollViewMeasurement() - requestAnimationFrame => + @requestAnimationFrame => {editor} = @props editor.setScrollTop(editor.getScrollTop() + @pendingVerticalScrollDelta) editor.setScrollLeft(editor.getScrollLeft() + @pendingHorizontalScrollDelta) @@ -656,7 +671,7 @@ EditorComponent = React.createClass onScrollTopChanged: -> @scrollingVertically = true @requestUpdate() - @onStoppedScrollingAfterDelay ?= debounce(@onStoppedScrolling, 100) + @onStoppedScrollingAfterDelay ?= debounce(@onStoppedScrolling, 200) @onStoppedScrollingAfterDelay() onStoppedScrolling: -> @@ -684,7 +699,7 @@ EditorComponent = React.createClass lastMousePosition = {} animationLoop = => @pauseScrollViewMeasurement() - requestAnimationFrame => + @requestAnimationFrame => if dragging screenPosition = @screenPositionForMouseEvent(lastMousePosition) dragHandler(screenPosition) @@ -725,7 +740,7 @@ EditorComponent = React.createClass return if @scrollViewMeasurementRequested @scrollViewMeasurementRequested = true - requestAnimationFrame => + @requestAnimationFrame => @scrollViewMeasurementRequested = false @measureScrollView() From 7202f497dbd9677e8e17550411593fa5eb2b8210 Mon Sep 17 00:00:00 2001 From: Nathan Sobo Date: Wed, 2 Jul 2014 09:30:44 -0600 Subject: [PATCH 4/5] Pause scroll view measurement in requestAnimationFrame helper method --- src/editor-component.coffee | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/editor-component.coffee b/src/editor-component.coffee index 917a8202c..e101e00f4 100644 --- a/src/editor-component.coffee +++ b/src/editor-component.coffee @@ -219,6 +219,7 @@ EditorComponent = React.createClass requestAnimationFrame: (fn) -> @updatesPaused = true + @pauseScrollViewMeasurement() requestAnimationFrame => fn() @updatesPaused = false @@ -533,7 +534,6 @@ EditorComponent = React.createClass animationFramePending = @pendingScrollTop? @pendingScrollTop = scrollTop unless animationFramePending - @pauseScrollViewMeasurement() @requestAnimationFrame => pendingScrollTop = @pendingScrollTop @pendingScrollTop = null @@ -547,7 +547,6 @@ EditorComponent = React.createClass animationFramePending = @pendingScrollLeft? @pendingScrollLeft = scrollLeft unless animationFramePending - @pauseScrollViewMeasurement() @requestAnimationFrame => @props.editor.setScrollLeft(@pendingScrollLeft) @pendingScrollLeft = null @@ -569,7 +568,6 @@ EditorComponent = React.createClass @clearMouseWheelScreenRowAfterDelay() unless animationFramePending - @pauseScrollViewMeasurement() @requestAnimationFrame => {editor} = @props editor.setScrollTop(editor.getScrollTop() + @pendingVerticalScrollDelta) @@ -698,7 +696,6 @@ EditorComponent = React.createClass dragging = false lastMousePosition = {} animationLoop = => - @pauseScrollViewMeasurement() @requestAnimationFrame => if dragging screenPosition = @screenPositionForMouseEvent(lastMousePosition) @@ -740,7 +737,7 @@ EditorComponent = React.createClass return if @scrollViewMeasurementRequested @scrollViewMeasurementRequested = true - @requestAnimationFrame => + requestAnimationFrame => @scrollViewMeasurementRequested = false @measureScrollView() From 17fa580ecd762024a3c854de4321d24bc9d6bc81 Mon Sep 17 00:00:00 2001 From: Nathan Sobo Date: Wed, 2 Jul 2014 10:11:35 -0600 Subject: [PATCH 5/5] Ignore 'scroll' events when an update is pending This prevents feedback loops where we handle stale 'scroll' events for scrolls requested in the model layer. It prevents jitter when autoscrolling with the cursor. --- spec/editor-component-spec.coffee | 2 +- src/editor-component.coffee | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/editor-component-spec.coffee b/spec/editor-component-spec.coffee index 2b1e3c93e..0fbdb8858 100644 --- a/spec/editor-component-spec.coffee +++ b/spec/editor-component-spec.coffee @@ -724,11 +724,11 @@ describe "EditorComponent", -> # Add decorations that are out of range marker2 = editor.displayBuffer.markBufferRange([[9, 0], [9, 0]]) editor.addDecorationForMarker(marker2, type: ['gutter', 'line'], class: 'b') + runSetImmediateCallbacks() # Scroll decorations into view verticalScrollbarNode.scrollTop = 2.5 * lineHeightInPixels verticalScrollbarNode.dispatchEvent(new UIEvent('scroll')) - runSetImmediateCallbacks() expect(lineAndLineNumberHaveClass(9, 'b')).toBe true # Fold a line to move the decorations diff --git a/src/editor-component.coffee b/src/editor-component.coffee index e101e00f4..714f72735 100644 --- a/src/editor-component.coffee +++ b/src/editor-component.coffee @@ -529,7 +529,7 @@ EditorComponent = React.createClass onVerticalScroll: (scrollTop) -> {editor} = @props - return if scrollTop is editor.getScrollTop() + return if @updateRequested or scrollTop is editor.getScrollTop() animationFramePending = @pendingScrollTop? @pendingScrollTop = scrollTop @@ -542,7 +542,7 @@ EditorComponent = React.createClass onHorizontalScroll: (scrollLeft) -> {editor} = @props - return if scrollLeft is editor.getScrollLeft() + return if @updateRequested or scrollLeft is editor.getScrollLeft() animationFramePending = @pendingScrollLeft? @pendingScrollLeft = scrollLeft