Closed Bug 1068961 Opened 6 years ago Closed 6 years ago
Layer not fully rendered when zoomed
STR: 1. Load http://people.mozilla.org/~bballo/square-color.html in the B2G browser. 2. Zoom in. 3. Scroll to the bottom-left of the subframe, and then scroll up. Expected results: The scrolled content is all green. Actual results: Only part of the scrolled content is green, the rest is white. The problem does not occur with http://people.mozilla.org/~bballo/square.html, which adds some text to force a Thebes layer rather than a Color layer. This makes me suspect that the problem might be related to bug 1066713.
ColorLayerComposite.cpp just uses mBounds. I'd look there first. A quick comparison against ThebesLayerComposite.cpp and that uses the EffectiveVisibleRegion. This could explain this problem (if it does not I'd test color layers under a intermediate surface container layer).
The problem does not occur with the layout.async-containerless-scrolling.enabled pref set to false, making this a multi-layer-apz regression.
I debugged this with Timothy's help. The problem is that the clip rect of the color layer does not increase as the page is zoomed in. The reason this doesn't happen with Thebes layers is that FrameLayerBuilder resets the clip rect of Thebes layers (but not Color layers) in PopThebesLayerData(). SetupScrollingMetadata() then sees that the layer already has a clip rect, and intersects the existing (pre-zoom, and thus smaller) clip rect with the one computed by ComputeFrameMetrics() (which is post-zoom and thus larger) . As a result, the clip rect fails to grow as we zoom. Modifying FrameLayerBuilder to clear the clip rect for Color layers too in PopThebesLayerData() seems to fix the problem. Timothy, is this what you had in mind?  https://mxr.mozilla.org/mozilla-central/source/layout/base/FrameLayerBuilder.cpp?rev=8252eae8278c#3517
Assignee: nobody → botond
Attachment #8493961 - Flags: feedback?(tnikkel)
Comment on attachment 8493961 [details] [diff] [review] Reset the clip rect for color layers in FrameLayerBuilder Yeah, I think the problem was that we never had clip rects on color layers before multi layer apzc. Now that we do we need to clear the old clip rect.
Attachment #8493961 - Flags: review?(roc) → review+
(In reply to Botond Ballo [:botond] from comment #5) > https://tbpl.mozilla.org/?tree=Try&rev=f58c80c60240 The Try push has some async-scrolling reftest failures that look like they could be caused by this patch.
The problem was that if ThebesLayerData::mItemClip had a clip set, the patch used that clip rather than clearing the clip. However, for a ColorLayer this is the incorrect clip to use. The solution is to always just clear the clip for ColorLayers. Clean Try push: https://tbpl.mozilla.org/?tree=Try&rev=97cb4d8c05d7
The problem occurs because there can be two types of clips: those that are induced by a containing scroll frame, and all other clips. The first type doesn't move with the content, the second does. SetupScrollingMetaData does the first, everything else is the second. When APZC moves a layer it doesn't move the clip, so this can only work if the clip on the layer is of the first type. And intersecting a clip of the first and the second type will also produce this problem. The reason this only affects color layers is because thebes layers usually don't get a clip because the items in the thebes layer usually don't share the same clip, so the layer doesn't get a clip, they just get clipped individually when drawn. Image layers do get a clip because they contain one item, but we use the ASSUME_DRAWING_RESTRICTED_TO_CONTENT_RECT flag to skip unnecessary clips so that the image display item will usually not have a clip. So I think there is still an underlying bug where a thebes layer could get a non-scroll induced clip, it just doesn't happen very often.
Attachment #8495991 - Flags: review?(roc) → review+
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
Comment on attachment 8495991 [details] [diff] [review] Reset the clip rect for color layers in FrameLayerBuilder We need this in order to make the landing of bug 1062792 on b2g34 green. NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings. [Approval Request Comment] Bug caused by (feature/regressing bug #): bug 967844 User impact if declined: wrong rendering in some cases Testing completed: been on m-c for a while now Risk to taking this patch (and alternatives if risky): not very risky String or UUID changes made by this patch: none
Attachment #8495991 - Flags: approval-mozilla-b2g34?
Comment on attachment 8495991 [details] [diff] [review] Reset the clip rect for color layers in FrameLayerBuilder Approving as this is needed for 1062792
Attachment #8495991 - Flags: approval-mozilla-b2g34? → approval-mozilla-b2g34+
Issue verified fixed on Flame 2.1 and Flame 2.2 Actual Results: All parts of scrolling colored square are green. Device: Flame 2.1 (319mb)(Kitkat Base)(Full Flash) BuildID: 20141104001202 Gaia: 8b0cf889ae0d48a9eb7ecdcb9b67590de45cc5e5 Gecko: 388b03efe92d Gonk: 48835395daa6a49b281db62c50805bd6ca24077e Version: 34.0 (2.1) Firmware: V188 User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0 Device: Flame 2.2 Master (319mb)(Kitkat Base)(Full Flash) BuildID: 20141104040207 Gaia: 3c50520982560ccba301474d1ac43706138fc851 Gecko: 54d05732f29b Gonk: 48835395daa6a49b281db62c50805bd6ca24077e Version: 36.0a1 (2.2 Master) Firmware: V188 User Agent: Mozilla/5.0 (Mobile; rv:36.0) Gecko/36.0 Firefox/36.0
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Comment on attachment 8495991 [details] [diff] [review] Reset the clip rect for color layers in FrameLayerBuilder Approval Request Comment Bug caused by (feature/regressing bug #): bug 967844 User impact if declined: wrong rendering in some cases Testing completed: been on m-c for a while now Risk to taking this patch (and alternatives if risky): not very risky String or UUID changes made by this patch: none
Attachment #8495991 - Flags: approval-mozilla-beta?
Comment on attachment 8495991 [details] [diff] [review] Reset the clip rect for color layers in FrameLayerBuilder Beta+
Attachment #8495991 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.