bugzilla.mozilla.org has resumed normal operation. Attachments prior to 2014 will be unavailable for a few days. This is tracked in Bug 1475801.
Please report any other irregularities here.

[APZ] code mirror's panel with line numbers twitches when I scroll code area horizontally

RESOLVED DUPLICATE of bug 1257641

Status

()

Core
Panning and Zooming
RESOLVED DUPLICATE of bug 1257641
2 years ago
2 years ago

People

(Reporter: arni2033, Assigned: kats)

Tracking

({regression})

Trunk
mozilla48
regression
Points:
---

Firefox Tracking Flags

(firefox47 unaffected, firefox48 verified)

Details

(Whiteboard: [gfx-noted])

Attachments

(1 attachment)

(Reporter)

Description

2 years ago
>>>   My Info:   Win7_64, Nightly 48, 32bit, ID 20160313030418
STR:
1. Open https://www.mozilla.org/en-US/firefox/nightly/firstrun/
2. Open Style editor, make sure that the biggest stylesheet is selected
3. Click in the beginning of line 12:   "/*! updated 2015-06-17 */@font-face {"
4. Hover mouse over the code area, rotate mouse wheel down 10 times to make caret not visible
5. Press Right arrow
6. Hover mouse over horizontal scrollbar in source code area, hold left mouse button, 
   then move mouse to the left and right 5 times. Release left mouse button

AR:  The panel with line numbers twitches a log
ER:  Line numbers should stay still

This is regression from bug 1253860. Regression range:
> https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=412c5cae8dea7b52da7c6981eec6a2a2884897c9&tochange=cd65b31bdeb565c34ae8e9d56107289ce71d9885
Component: Developer Tools: Source Editor → Panning and Zooming
Product: Firefox → Core
Whiteboard: [gfx-noted]
Assignee: nobody → bugmail.mozilla
Ok, so I think this is happening because the scroll-offset-update message that I added in bug 1253860 gets sent before the scroll event fires, and so the updates that the scroll event does lag behind the visual scroll position.

Before:
- main thread scroll update
- refresh driver tick
- scroll listener runs
- layout/paint
- transaction to main thread
- visual scroll position update by APZ

Now:
- main thread scroll update
- scroll offset update to compositor
- refresh driver tick
- visual scroll position update by APZ
- scroll listener runs
- layout/paint
- transaction to main thread
- effects of scroll listener visible to user
To fix this I tried moving the scroll-offset-update message to run after the scroll event was dispatched (synchronously after the call to mHelper->FireScrollEvent() but that didn't quite fix the problem either. It helps, but there's still occasionally a lag between the offset update message arriving, and the updated painted content arriving in the compositor. If we composite between those two then the lag is user-visible.

I'll see if disabling the scroll-offset-update feature on pages with scroll-linked-effects solves the problem. It's analogous to how we had to disable it for windowed plugins.
Created attachment 8731741 [details] [diff] [review]
Disable main-thread paint-skipping on pages with scroll-linked effects [functionally backed out in comment 12]

Yeah this does the job
Attachment #8731741 - Flags: review?(mstange)
I guess we could turn the scroll-offset-update message into a layer transaction message. So if we don't have a paint, we'll send an empty transaction with just that scroll-offset-update message, and it will be sent in the stage of the refresh tick that matches existing behavior.
Comment on attachment 8731741 [details] [diff] [review]
Disable main-thread paint-skipping on pages with scroll-linked effects [functionally backed out in comment 12]

Review of attachment 8731741 [details] [diff] [review]:
-----------------------------------------------------------------

But this is a fine short term workaround.
Attachment #8731741 - Flags: review?(mstange) → review+
(In reply to Markus Stange [:mstange] from comment #4)
> I guess we could turn the scroll-offset-update message into a layer
> transaction message. So if we don't have a paint, we'll send an empty
> transaction with just that scroll-offset-update message, and it will be sent
> in the stage of the refresh tick that matches existing behavior.

Yeah that seems like it would be better. I'll need to properly understand how empty transactions work in order to implement that. Will file a follow-up for it, and land this for now.
Depends on: 1257641
Filed bug 1257641 for it.
status-firefox47: --- → unaffected

Comment 9

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/59a1ca7d784c
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox48: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
(Reporter)

Comment 10

2 years ago
Fixed on:   Win7_64, Nightly 48, 32bit, ID 20160319030558

(there was a typo in AR in comment 0: "...twitches A LOT")
Status: RESOLVED → VERIFIED
status-firefox48: fixed → verified
Now that bug 1257641 is landed and seems to be behaving well, I'm going to back this out and dupe this bug to that one, since that is the proper fix for this issue, as mentioned in comment 4.
Status: VERIFIED → RESOLVED
Last Resolved: 2 years ago2 years ago
No longer depends on: 1257641
Resolution: FIXED → DUPLICATE
Duplicate of bug: 1257641
Attachment #8731741 - Attachment description: Disable main-thread paint-skipping on pages with scroll-linked effects → Disable main-thread paint-skipping on pages with scroll-linked effects [functionally backed out in comment 12]
You need to log in before you can comment on or make changes to this bug.