Closed Bug 1164406 Opened 7 years ago Closed 7 years ago

Screen elements jump with APZ enabled

Categories

(Core :: Panning and Zooming, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla41
Tracking Status
firefox41 --- fixed

People

(Reporter: mayankleoboy1, Assigned: botond)

References

()

Details

Attachments

(6 files, 1 obsolete file)

Attached file about support.txt
User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:41.0) Gecko/20100101 Firefox/41.0
Build ID: 20150512030215

Steps to reproduce:

1. Create a new profile. (enable e10s)
2. set layers.async-pan-zoom.enabled  = true
3. Go to http://www.snapdeal.com/product/hp-15r205tu-notebook-k8u05pa-5th/683139390240

4. Scroll down.  May have to let the comments also load.


Actual results:

the page elements on the bottom of the screen shift from right-left.


Expected results:

not so.
Blocks: apz-windows
Attached video bug.avi
Blocks: apz-desktop
No longer blocks: apz-windows
Component: General → Panning and Zooming
It looks like all position:fixed elements on the page briefly forget about the vertical scrollbar and jump to the right by the scrollbar width.
I can repro on Windows7
Status: UNCONFIRMED → NEW
Ever confirmed: true
I can repro on Linux also. Increasing the apz repaint interval prefs makes it easier to repro.
Assignee: nobody → botond
Blocks: apz-windows, apz-linux
No longer blocks: apz-desktop
I'll note that the following message appears repeatedly on the console while browsing this page:

###!!! ASSERTION: 3D transform found between fixedpos content and its viewport (should never happen): 'Error', file /home/botond/dev/mozilla/central/layout/base/nsLayoutUtils.cpp, line 1726

Not sure yet if it's relevant.
(In reply to Botond Ballo [:botond] from comment #5)
> I'll note that the following message appears repeatedly on the console while
> browsing this page:
> 
> ###!!! ASSERTION: 3D transform found between fixedpos content and its
> viewport (should never happen): 'Error', file
> /home/botond/dev/mozilla/central/layout/base/nsLayoutUtils.cpp, line 1726
> 
> Not sure yet if it's relevant.

I think this is unrelated, because the layer which has the 3D transform is not visible, and the layers which are jumping around do not have 3D transforms.
The fixed-position items appear to be incorrectly positioned in the stable state, and then temporarily jump to their correct position during scrolling.

However, the adjustment is not being done in the compositor - the transform Layout sets on the layers is changing.

I have attached display list and layer dumps for both the correct and incorrect positioning. One of the layers which is jumping is 0x5cfda30 (client-side address) - its transform has x=1866 in the correct case and x=1879 in the incorrect case.
I debugged this further with Timothy's help, and we determined that Layout is putting the position:fixed frame in the wrong spot. I'm continuing to investigate.
The position of the fixed frame is in turn determined by the SPC-SPS, whose width is changing between 1525 (good frames) and 1536 (bad frames).
The bad value of the SPC-SPS, which fails to exclude the size of the scrollbars, is coming from TabChild::HandlePossibleViewportSize()'s call to nsLayoutUtils::SetScrollPositionClampingScrollPortSize(). We pass in the composition bounds, which come from TabChild::mInnerSize, without anyone subtracting the scrollbar sizes. This is in contrast to other places where we calculate the composition bounds, where we do exclude the scrollbars.
Attached file MozReview Request: bz://1164406/botond (obsolete) —
/r/9269 - Bug 1164406 - Exclude the scrollbars from the composition bounds in TabChild::HandleCalculateViewportChange() when appropriate. r=kats

Pull down this commit:

hg pull -r fda8530860ddc4b5ed9214b93deaf3ef1d2bf5e4 https://reviewboard-hg.mozilla.org/gecko/
Attachment #8609497 - Flags: review?(bugmail.mozilla)
The attached patch fixes the problem for me.
https://reviewboard.mozilla.org/r/9269/#review7953

::: dom/ipc/TabChild.cpp:346
(Diff revision 1)
> +    if (nsIScrollableFrame* rootScrollableFrame = shell->GetRootScrollFrameAsScrollable()) {

Can we reuse nsLayoutUtils::DeflateScrollbarAreaFromCompositionBoundsFor here? We can modify that function a little to return the nsMargin instead of a bool to encapsulate more of the code and reduce the need to get the nsIScrollableFrame at the callsite.
Comment on attachment 8609497 [details]
MozReview Request: bz://1164406/botond

/r/9275 - Bug 1164406 - Expose scrollbar-area deflation in nsLayoutUtils. r=kats
/r/9269 - Bug 1164406 - Exclude the scrollbars from the composition bounds in TabChild::HandleCalculateViewportChange() when appropriate. r=kats

Pull down these commits:

hg pull -r 5e63b044025d339a6899f0ecb6b64d79eec5641d https://reviewboard-hg.mozilla.org/gecko/
https://reviewboard.mozilla.org/r/9269/#review8001

::: dom/ipc/TabChild.cpp:347
(Diff revision 2)
> +      nsMargin scrollbarsAppUnits = nsLayoutUtils::ScrollbarAreaToExcludeFromCompositionBoundsFor(rootScrollFrame);

You can reduce one level of indentation by just passing shell->GetRootScrollFrame() directly to the ScrollbarArea function. It will return 0 if passed a nullptr
Attachment #8609497 - Flags: review?(bugmail.mozilla) → review+
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #17)
> You can reduce one level of indentation by just passing
> shell->GetRootScrollFrame() directly to the ScrollbarArea function. It will
> return 0 if passed a nullptr

Updated locally. 

Try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=34852266b598
https://hg.mozilla.org/mozilla-central/rev/d37b2f90f5af
https://hg.mozilla.org/mozilla-central/rev/256f3225ca4e
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
Depends on: 1167882
Depends on: 1167891
No longer blocks: 1157179
Duplicate of this bug: 1157179
Attachment #8609497 - Attachment is obsolete: true
Attachment #8620278 - Flags: review+
Attachment #8620279 - Flags: review+
You need to log in before you can comment on or make changes to this bug.