Closed Bug 1034247 Opened 5 years ago Closed 5 years ago

Scaled content is rendered wrong


(Core :: Layout, defect, P1)






(Reporter: jrmuizel, Assigned: mattwoodrow)



(Keywords: perf, Whiteboard: [c=regression p= s=2014.07.18.t u=])


(3 files, 1 obsolete file)

Attached file checker.html
The attached testcase draws all kind of wrong in firefox compared to other browsers
Can you expand on "all kinds of wrong" a little?
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)
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.

Assignee: nobody → matt.woodrow
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.
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)
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+
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
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
Keywords: perf
Whiteboard: [c=regression p= s= u=]
Backed out on central for the very visible (on some devices at least) regressions in Fennec and B2G. See bug 1036518 and dupes.
Resolution: FIXED → ---
Target Milestone: mozilla33 → ---
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.
I filed bug 1036967 for APZ handling of different scale factors per axis.
oh, this patch fixes android 4.0.4 robopan in a serious way:[[174,63,29]]&sel=none&displayrange=7&datatype=running

I will look forward to it landing again!
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 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+
Depends on: 1038579
No longer depends on: 1038579
Closed: 5 years ago5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
Priority: -- → P1
Whiteboard: [c=regression p= s= u=] → [c=regression p= s=2014.07.18.t u=]
Depends on: 1044571
Depends on: 1049450
You need to log in before you can comment on or make changes to this bug.