Closed
Bug 1034247
Opened 10 years ago
Closed 10 years ago
Scaled content is rendered wrong
Categories
(Core :: Layout, defect, P1)
Tracking
()
RESOLVED
FIXED
mozilla33
People
(Reporter: jrmuizel, Assigned: mattwoodrow)
References
Details
(Keywords: perf, Whiteboard: [c=regression p= s=2014.07.18.t u=])
Attachments
(3 files, 1 obsolete file)
The attached testcase draws all kind of wrong in firefox compared to other browsers
Comment 1•10 years ago
|
||
Can you expand on "all kinds of wrong" a little?
Comment 2•10 years ago
|
||
This looks similar to what happens to the testcase I attached to bug 1020556 (attachment 8450183 [details]): In that testcase we double the resolution of the transformed layer and end up with a visible region that's wider than 4096 pixels, so we bail out in ContentClientIncremental::BeginPaintBuffer due to this check: if (destBufferRect.width > mForwarder->GetMaxTextureSize() || destBufferRect.height > mForwarder->GetMaxTextureSize()) { return result; } and don't repaint the layer contents, so they still have the 1:1 resolution contents. (1024 CSS pixels on HiDPI = 2048 device pixels, that times 2 for the CSS transform resolution gives 4096)
Comment 3•10 years ago
|
||
We have a number of 4k checks in the code, at least on B2G. I know I added at least two, at [1] and [2]. We might be able to take these out now that we do tiling? I'm not sure. [1] http://mxr.mozilla.org/mozilla-central/source/layout/generic/nsGfxScrollFrame.cpp?rev=3bb11b1b5166#2370 [2] http://mxr.mozilla.org/mozilla-central/source/gfx/layers/ipc/SharedBufferManagerParent.cpp?rev=312fd139c63e#175
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → matt.woodrow
Assignee | ||
Comment 4•10 years ago
|
||
I'll be implementing the solution suggested in bug 1028216 comment 21 since it's the same underlying issue. This should also fix bug 847653.
Assignee | ||
Comment 5•10 years ago
|
||
This ended up being simpler than I expected, but I had to move it from CreateOrRecycleThebesLayer into ChooseScaleAndSetTransform. We can't set a resolution for the ThebesLayer that is different from what the ContainerState is using for the items since it breaks a lot of assumptions. Folding the scale down should only matter for ThebesLayers anyway, so I think this is equivalent. This fixes this bug and the testcase from bug 847653.
Attachment #8452115 -
Flags: review?(roc)
Assignee | ||
Comment 6•10 years ago
|
||
https://tbpl.mozilla.org/?tree=Try&rev=a1d70b1e542f
Comment on attachment 8452115 [details] [diff] [review] Adjust layer scale to avoid going over the max texture size Review of attachment 8452115 [details] [diff] [review]: ----------------------------------------------------------------- Maybe it's now worth eliminating the check on the viewport size too.
Attachment #8452115 -
Flags: review?(roc) → review+
Assignee | ||
Comment 8•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/cc20208a6eb4
Comment 9•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/cc20208a6eb4
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
Comment 10•10 years ago
|
||
This made scrolling perma-janky on the nexus 4, see bug 1036518. I'm guessing the change in scale ends up looking like a CSS transform on the compositor side and APZ isn't playing well with it?
Depends on: 1036518
Comment 11•10 years ago
|
||
Backed out on central for the very visible (on some devices at least) regressions in Fennec and B2G. See bug 1036518 and dupes. https://hg.mozilla.org/mozilla-central/rev/f93c0ef45597
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Updated•10 years ago
|
Target Milestone: mozilla33 → ---
Comment 12•10 years ago
|
||
If it helps, here's a layers dump from the homescreen on a Nexus4, which is subject to the janky scrolling in bug 1036518. I noticed that the main scrollable layer (0xaba6b000) has a preScale=1, 2. I don't know if the APZ code is set up to handle a pre-scale that differs on the x and y axes. That might be part of the problem. We probably also don't need the texture limits on tiled content to begin with, or something.
Comment 13•10 years ago
|
||
I filed bug 1036967 for APZ handling of different scale factors per axis.
Comment 14•10 years ago
|
||
oh, this patch fixes android 4.0.4 robopan in a serious way: http://graphs.mozilla.org/graph.html#tests=[[174,63,29]]&sel=none&displayrange=7&datatype=running I will look forward to it landing again!
Assignee | ||
Comment 15•10 years ago
|
||
I've updated the patch to only downscale large content that has been affected by a CSS transform. Kats, could you please check to confirm that this doesn't regress scrolling on the nexus 4.
Attachment #8452115 -
Attachment is obsolete: true
Attachment #8455145 -
Flags: feedback?(bugmail.mozilla)
Comment 16•10 years ago
|
||
Comment on attachment 8455145 [details] [diff] [review] Adjust layer scale to avoid going over the max texture size v2 Review of attachment 8455145 [details] [diff] [review]: ----------------------------------------------------------------- Tested basic usage on both B2G and Fennec on the nexus 4, both seem to be ok as far as I can tell.
Attachment #8455145 -
Flags: feedback?(bugmail.mozilla) → feedback+
Assignee | ||
Comment 17•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/1aa504e62034
Comment 18•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/1aa504e62034
Status: REOPENED → RESOLVED
Closed: 10 years ago → 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
Updated•10 years ago
|
Priority: -- → P1
Whiteboard: [c=regression p= s= u=] → [c=regression p= s=2014.07.18.t u=]
You need to log in
before you can comment on or make changes to this bug.
Description
•