Closed Bug 1327095 Opened 3 years ago Closed 3 years ago

[APZ] Scrollable area doesn't repaint for several seconds if I scroll the whole page

Categories

(Core :: Panning and Zooming, defect, P2)

48 Branch
Unspecified
Windows
defect

Tracking

()

VERIFIED FIXED
mozilla53
Tracking Status
firefox51 - wontfix
firefox52 - wontfix
firefox53 - verified

People

(Reporter: arni2033, Assigned: kats)

References

()

Details

(Keywords: regression, Whiteboard: [gfx-noted])

Attachments

(1 file)

>>>   My Info:   Win7_64, Nightly 49, 32bit, ID 20160526082509
STR_1:
1. Open attachment 8770666 [details]
2. Scroll the small scrollable area (by rotating mouse wheel) down, then up
3. In less than 3 seconds, autoscroll the page down, then up

AR:  The small scrollable area stays blank for 0.5 second. Sometimes it stays blank for 5+ seconds.
ER:  The small scrollable area shouldn't become blank

You should check the regression range.

This is regression from bug 1253860. Regression range:
> https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=412c5cae8dea7b52da7c6981eec6a2a2884897c9&tochange=cd65b31bdeb565c34ae8e9d56107289ce71d9885
No longer blocks: 1277113
Component: Untriaged → Panning and Zooming
Product: Firefox → Core
[Tracking Requested - why for this release]: rendering glitch persists


To make matters worse, the rendering glitch persists
3. middleclick on the root element to start autoscroll and move mouse downward, then upward and then click mouse on toolbar.
Priority: -- → P2
Dropping needinfo, I'll look at this again when I triage the rest of the bugs arni filed that ended up in the APZ component.
Flags: needinfo?(bugmail)
OS: Unspecified → Windows
Priority: P2 → --
Version: Trunk → 48 Branch
I can't reproduce this, but it sounds related to bug 1259593. I might need to get logs from somebody that can reproduce it.
Priority: -- → P2
See Also: → 1259593
Whiteboard: [gfx-noted]
I can reproduce where part of the small scrollable area stays blank for about a second.
See Also: → 1328066
Ah, I can reproduce now. I wasn't scrolling the main page far enough - with the minimap on I can see that we need to scroll the main page far enough to cause the displayport to move down, and then scroll back up. If you only scroll so that the iframe is out of view that might not be far enough.
If I'm understanding this properly, the problem is similar to the one in bug 1259593, in that the intersection of the dirtyRect (i.e. the ancestor scrollframe's displayport), the scrollPort (of the subframe) and the rootCompBounds comes out empty. The paint in which this happens looks kind of like this:

+--- Ancestor Displayport ---+ <-- top of displayport (dirtyRect)
|                            |
|   +-- Subframe -------+    |
|   |                   |    |
|   |                   |    |
|   |                   |    |
|   +-------------------+    |
|                            |
+ - - - - - - - - - - - - - -+ <-- top of user-visible area (rootCompBounds)
|                            |
|                            |
|                            |
|      User-visible area     |
|                            |
|                            |
|                            |
+ - - - - - - - - - - - - - -+ <-- bottom of user-visible area
|                            |
+----------------------------+ <-- bottom of displayport

Note that the "rootCompBounds" ends up being the "user visible area" in the diagram, which is disjoint from the "mScrollPort" (the subframe), even though both are inside the "aDirtyRect" (the ancestor displayport). I think in this case we want to having a displayport base on the subframe which is the size of mScrollPort but because we just intersect the three values it comes out empty.

A specific numerical example I observed had these values (in app units):

dirtyRect: (x=-12780, y=-12780, w=114300, h=238080)
scrollPort: (x=0, y=0, w=87720, h=23400)
rootCompbounds: (x=-12779, y=76259, w=114300, h=65339)
So based on this I'm thinking the correct fix is what Timothy described in bug 1259593 comment 1, but instead of taking an equal amount from all sides, we want to do something a little smarter in the way we crop it. I'll cook up a patch, probably on Monday.
So my fix here basically translates the rootCompBounds value to maximize overlap with the subframe before doing the intersection. So in the diagram above, rootCompBounds would move "up" until the bottom edge of the rootCompBounds lines up with the bottom edge of the subframe box. If it turns out that the subframe is really really tall, this still does the right thing (the intersection takes the bottom rootCompBounds.height-sized chunk of the subframe) and if the subframe is shorter than rootCompBounds it just takes the whole height. The analogous change applies to all four directions.

Try push is at https://treeherder.mozilla.org/#/jobs?repo=try&revision=3421d952851ee50d3d9276ddd9e7af247f37eb5f, I'm going to verify it doesn't reintroduce bug 1262151 before putting it up for review.
Verified that the patch doesn't seem to regress bug 1262151.
Comment on attachment 8823442 [details]
Bug 1327095 - Shift the rootCompBounds to maximize overlap with the displayportBase before intersecting.

https://reviewboard.mozilla.org/r/101964/#review103996

I like it! Not as complicated as I thought.
Attachment #8823442 - Flags: review?(tnikkel) → review+
Pushed by kgupta@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b1d687691a9f
Shift the rootCompBounds to maximize overlap with the displayportBase before intersecting. r=tnikkel
https://hg.mozilla.org/mozilla-central/rev/b1d687691a9f
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
Going to let this ride on 53 since this code has been brittle in the past.
Not tracking on branches based on comment 16.
Flags: qe-verify+
Reproduced the initial issue with 53.0a1 2017-01-01.
Verified as fixed using Firefox 53 beta 1 under Win 10 64-bit.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.