Closed Bug 926268 Opened 6 years ago Closed 6 years ago

Layers in Settings app changes size, causes buffer re-allocations

Categories

(Core :: Graphics: Layers, defect)

x86
macOS
defect
Not set

Tracking

()

VERIFIED FIXED
mozilla28
Tracking Status
firefox28 --- verified

People

(Reporter: BenWa, Assigned: mattwoodrow)

References

(Depends on 1 open bug, Blocks 1 open bug)

Details

Attachments

(1 file, 1 obsolete file)

While scrolling in the Settings app with layer border the content the layer keeps changing sizes. I don't know this for a fact but I suspects it is what's responsible for the sporadic buffer (re)allocations while scrolling. We need to investigate and fix/mitigate this somehow.
Blocks: 882383
Some progress. Looks like the thebes buffer is somehow getting size 0:
I/Gecko   ( 9847): Buffer changed size 0x43dbd608, 0, 0, to 320, 3
I/Gecko   ( 9847): Buffer changed size 0x43dbd608, 0, 0, to 320, 3
I/Gecko   ( 9847): Buffer changed size 0x43dbd608, 0, 0, to 320, 3
I/Gecko   ( 9847): Buffer changed size 0x43dbd608, 0, 0, to 320, 3
I/Gecko   ( 9847): Buffer changed size 0x43dbd608, 0, 0, to 320, 3
I/Gecko   ( 9847): Buffer changed size 0x43dbd608, 0, 0, to 320, 3
I/Gecko   ( 9847): Buffer changed size 0x43dbd608, 0, 0, to 320, 3
I/Gecko   ( 9847): Buffer changed size 0x43dbd608, 0, 0, to 320, 3
I/Gecko   ( 9847): Buffer changed size 0x43dbd608, 0, 0, to 320, 3
Now that the inspector is working well on b2g that should be the first thing to look into.
Note that the impact of this has been mitigated in the settings app by bug 938785 but the underlying problem is still there.
I can reproduce this on this page:
http://www.reddit.com/r/Minecraft/comments/1rwzbo/nanofarm_v3_stack_of_wheat_in_35_seconds/

A relatively simple forum page with a fixed position background. We're very slow to scroll this page.
Matt, for a page like the one in Comment 4 we can solve a large class of problem like that if we don't shrink scrolling layer and have them fit the entire scroll frame. We're trying too hard to save memory and suffer in these kinds of cases.

What do you think?
Flags: needinfo?(matt.woodrow)
What I see on desktop doesn't match the original description, but it's interesting.

The fixed position image appears to be between the text and the page background, so we sometimes have to put the text in it's own (component alpha) layer.

We detect that most of the text doesn't actually cover the image, so this gets merged with the background.

Sometimes (but not all of them time?) we think some of the text at the same Y position as the image does cover it, so we push this block of text into a new layer.

Looks like a bug or two here, I'll look into it with a debugger today.
Flags: needinfo?(matt.woodrow)
This fixes the bug causing us to reallocate buffers on reddit.

I doubt it does anything for your b2g test case though, this was most likely an unrelated bug.
Attachment #8342077 - Flags: review?(roc)
Comment on attachment 8342077 [details] [diff] [review]
Don't include the whole frame bounds if we're only painting one border size

Review of attachment 8342077 [details] [diff] [review]:
-----------------------------------------------------------------

::: layout/base/nsDisplayList.cpp
@@ +2557,5 @@
> +  } else {
> +    nsMargin border = styleBorder->GetComputedBorder();
> +    nsRegion result;
> +    if (border.top > 0) {
> +      result.Or(result, nsRect(borderBounds.X(), borderBounds.Y(), borderBounds.Width(), border.top));

Just assign since we know it's empty currently.

@@ +2567,5 @@
> +      result.Or(result, nsRect(borderBounds.X(), borderBounds.YMost() - border.bottom, borderBounds.Width(), border.bottom));
> +    }
> +    if (border.left > 0) {
> +      result.Or(result, nsRect(borderBounds.X(), borderBounds.Y(), border.left, borderBounds.Height()));
> +    }

Why use a region here? Just use a rect and UnionRect.
Attachment #8342077 - Flags: review?(roc) → review-
https://hg.mozilla.org/mozilla-central/rev/052685e62b34
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla28
Can we check whether this fixes the B2G test case and if not file a bug to continue the work there?
Flags: needinfo?(bgirard)
Settings app painting is fine now.
Flags: needinfo?(bgirard)
Status: RESOLVED → VERIFIED
Depends on: 1445706
You need to log in before you can comment on or make changes to this bug.