1034247 - Scaled content is rendered wrong

Status: RESOLVED FIXED

(Core :: Layout, defect, P1)

Description

[Jeff Muizelaar [:jrmuizel]]

Attachment: checker.html

The attached testcase draws all kind of wrong in firefox compared to other browsers

Comment 1

[Timothy Nikkel (:tnikkel)]

Can you expand on "all kinds of wrong" a little?

Comment 2

[Markus Stange [:mstange]]

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

[Kartikaya Gupta (email:[e-mail])]

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

Comment 4

[Matt Woodrow (:mattwoodrow)]

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.

Comment 5

[Matt Woodrow (:mattwoodrow)]

Attachment: Adjust layer scale to avoid going over the max texture size

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.

Comment 6

[Matt Woodrow (:mattwoodrow)]

https://tbpl.mozilla.org/?tree=Try&rev=a1d70b1e542f

Comment 7

[Robert O'Callahan (:roc) (email my personal email if necessary)]

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.

Comment 8

[Matt Woodrow (:mattwoodrow)]

 https://hg.mozilla.org/integration/mozilla-inbound/rev/cc20208a6eb4

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/cc20208a6eb4

Comment 10

[Kartikaya Gupta (email:[e-mail])]

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?

Comment 11

[Kartikaya Gupta (email:[e-mail])]

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

Comment 12

[Kartikaya Gupta (email:[e-mail])]

Attachment: Layers dump from a janky-scrolling B2G homescreen on Nexus4

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

[Kartikaya Gupta (email:[e-mail])]

I filed bug 1036967 for APZ handling of different scale factors per axis.

Comment 14

[Joel Maher ( :jmaher ) (UTC -8)]

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!

Comment 15

[Matt Woodrow (:mattwoodrow)]

Attachment: Adjust layer scale to avoid going over the max texture size v2

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.

Comment 16

[Kartikaya Gupta (email:[e-mail])]

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.

Comment 17

[Matt Woodrow (:mattwoodrow)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/1aa504e62034

Comment 18

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/1aa504e62034