Closed
Bug 1264297
Opened 8 years ago
Closed 8 years ago
CSS parallax doesn't paint correctly during scroll with APZ
Categories
(Core :: Panning and Zooming, defect)
Tracking
()
RESOLVED
FIXED
mozilla49
People
(Reporter: keith, Assigned: kats)
References
(Depends on 1 open bug)
Details
(Keywords: regression, Whiteboard: [gfx-noted])
Attachments
(1 file)
58 bytes,
text/x-review-board-request
|
mstange
:
review+
Sylvestre
:
approval-mozilla-aurora+
|
Details |
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_11_3) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/49.0.2623.112 Safari/537.36 Steps to reproduce: 1) Visit: http://keithclark.co.uk/articles/practical-css-parallax/smooth-scroll/ 2) Scroll up and down the page noting when the various parallax sections appear. Note: the empty space at the bottom of the page is a known issue and was reported in bug 1198135 Actual results: Sections of the page don't paint until scrolling has completely finished. Setting `apz.paint_skipping.enabled` to false appears to fix the issue. This is an issue in 47.0a2 Dev Edition and in the 48.0a1 (2016-04-13) Nightly Expected results: Page content should be visible during scroll.
Summary: CSS parallax doesn't paint correctly duing scroll with APZ → CSS parallax doesn't paint correctly during scroll with APZ
It regressed 2 times: 1) Regression 1: painting doesn't "follow" scrolling and takes time. https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=eca3a6b94f7934377b099ddb22ae9c91f340cbfc&tochange=9066ef31441959ae15b83d59395ed34d3ed03c0a Bug 1192910. 2) Regression 2: it's smooth now but blank areas are visible before painting after scrolling. https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=bf186476cb90f713243dd1d1d195d25de53914aa&tochange=e0b2fffc136004c1f61ec104823bd316cbb5b71f Bug 1238564 probably.
Status: UNCONFIRMED → NEW
status-firefox46:
--- → unaffected
status-firefox47:
--- → affected
status-firefox48:
--- → affected
tracking-firefox47:
--- → ?
tracking-firefox48:
--- → ?
Component: Untriaged → Panning and Zooming
Ever confirmed: true
Flags: needinfo?(mstange)
Flags: needinfo?(bugmail.mozilla)
Keywords: regression
Product: Firefox → Core
Assignee | ||
Comment 3•8 years ago
|
||
(In reply to Markus Stange [:mstange] from comment #2) > This might be caused by bug 1254260. Sounds plausible. It does seem like a painting issue, where the painted content doesn't contain the things it should from the displayport.
Depends on: 1254260
Flags: needinfo?(bugmail.mozilla)
Assignee | ||
Updated•8 years ago
|
Whiteboard: [gfx-noted]
47 will not ship with APZ, e10s turned on by default. Tracked for Fx48.
Assignee | ||
Updated•8 years ago
|
status-firefox49:
--- → affected
Assignee | ||
Comment 6•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/52846/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/52846/
Attachment #8752898 -
Flags: review?(mstange)
Assignee | ||
Comment 7•8 years ago
|
||
I notice that the test case in bug 1254260 has reftest attributes in the HTML, is that ready to be used as a reftest? If so I can include it in this patch.
Comment 8•8 years ago
|
||
Hmm, that test case looks incomplete. You could turn it into a reftest, but I don't think your patch will make it pass, because your patch relies on content fixing itself up after the scroll.
Updated•8 years ago
|
Attachment #8752898 -
Flags: review?(mstange) → review+
Comment 9•8 years ago
|
||
Comment on attachment 8752898 [details] MozReview Request: Bug 1264297 - Don't do paint-skipping for elements with perspective, until we can properly populate the displayport. r?mstange https://reviewboard.mozilla.org/r/52846/#review49754 It's an unfortunate workaround, but it's better than doing nothing about this.
Assignee | ||
Comment 10•8 years ago
|
||
(In reply to Markus Stange [:mstange] from comment #8) > Hmm, that test case looks incomplete. You could turn it into a reftest, but > I don't think your patch will make it pass, because your patch relies on > content fixing itself up after the scroll. After some massaging of the test case I was able to turn it into a reftest, but as you said, this patch doesn't make it pass because using reftest-async-scroll-y is an imperfect simulation of what happens when the user scrolls. Specifically setting reftest-async-scroll-y doesn't trigger a repaint whereas that would happen with an actual user scroll, and that's what my patch here relies upon. I tried modifying the test case to better capture the paint-skipping codepaths but wasn't very successful. I'll post the updated version of the test case to bug 1254260; presumably the correct fix will make it pass.
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → bugmail.mozilla
Comment 12•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/d214625e5bcf
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
Comment 13•8 years ago
|
||
We are not going to ship APZ with 47 so changing status-firefox47 to unaffected
Updated•8 years ago
|
Comment 14•8 years ago
|
||
Since this patch also affects 48, are you also considering to uplift this patch to 48?
Flags: needinfo?(bugmail.mozilla)
Assignee | ||
Comment 15•8 years ago
|
||
e10s and therefore APZ will be user-enableable on 47, so even though it's not a default configuration it's possible that users will enable it. For that, and for consistency with other bugs, I'd like to keep 47 as wontfix rather than unaffected. And yes, i will request uplift after a few days.
Flags: needinfo?(bugmail.mozilla)
Assignee | ||
Comment 16•8 years ago
|
||
Comment on attachment 8752898 [details] MozReview Request: Bug 1264297 - Don't do paint-skipping for elements with perspective, until we can properly populate the displayport. r?mstange Approval Request Comment [Feature/regressing bug #]: Various APZ-related changes [User impact if declined]: On some pages with perspective, while scrolling, elements don't paint properly [Describe test coverage new/current, TreeHerder]: tested locally [Risks and why]: pretty low-risk, this just disables an optimization in the case that the element being scrolled has perspective. The removal of the optimization mitigates the problem [String/UUID change made/needed]: none
Attachment #8752898 -
Flags: approval-mozilla-aurora?
Comment 17•8 years ago
|
||
Comment on attachment 8752898 [details] MozReview Request: Bug 1264297 - Don't do paint-skipping for elements with perspective, until we can properly populate the displayport. r?mstange Improve APZ, taking it.
Attachment #8752898 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment 18•8 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-aurora/rev/b235305398fd
You need to log in
before you can comment on or make changes to this bug.
Description
•