Closed Bug 1126045 Opened 5 years ago Closed 5 years ago

Frequent displayport invalidation due to resampling

Categories

(Core :: Layout, defect)

defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: dvander, Assigned: dvander)

References

Details

Attachments

(3 files)

On some sites (it's extremely frequent on engadget.com), we invalidate the displayport almost every scroll due to this path here:

  http://mxr.mozilla.org/mozilla-central/source/gfx/layers/RotatedBuffer.cpp#489

We decide the content type is COLOR, switch it to COLOR_ALPHA and invalidate, then switch it back to COLOR on the second iteration.
I'd prefer to get this off the widget, but it doesn't look very easy.
Assignee: nobody → dvander
Status: NEW → ASSIGNED
Attachment #8554876 - Flags: review?(bugmail.mozilla)
There is another bug here where sometimes the visible region of the scrollable layer changes, often without any display list invalidations. This ends up invalidating the entire buffer. Still trying to track that down.

Just the first patch improves things immensely though, and the second alone also helps quite a bit.
Comment on attachment 8554876 [details] [diff] [review]
part 1, don't resample if we don't support async zoom

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

This looks ok to me but I don't know this code really. Bouncing r? to matt.

::: gfx/layers/client/ClientLayerManager.cpp
@@ +209,5 @@
>    // a chance to repaint, so we have to ensure that it's all valid
>    // and not rotated.
> +  //
> +  // Desktop does not support async zoom yet, so we ignore this for those
> +  // platforms.

I'd move this part of the comment closer to the ifdef so it's more obvious that's what this is referring to
Attachment #8554876 - Flags: review?(matt.woodrow)
Attachment #8554876 - Flags: review?(bugmail.mozilla)
Attachment #8554876 - Flags: feedback+
Comment on attachment 8554891 [details] [diff] [review]
part 2, don't snap displayports to tile boundaries if we're not tiling

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

LGTM, thanks!
Attachment #8554891 - Flags: review?(bugmail.mozilla) → review+
Comment on attachment 8554876 [details] [diff] [review]
part 1, don't resample if we don't support async zoom

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

We'll have to revert this at some point (once we support async zooming for desktop), so this is a temporary fix. Would should try fix the other issues too so that this doesn't bite us again later.
Attachment #8554876 - Flags: review?(matt.woodrow) → review+
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/5df638c467f8
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/a9a751813c5e

The last two bugs are (1) the displayport changes size as you scroll near the edges of the page, because we clamp it to the edges of the scrollport - I think we should just extend the displayport in the opposite direction. And (2) the rotated buffer could be a little smarter, it's throwing away valid pixels when they could be re-used.
Keywords: leave-open
Attachment #8555717 - Flags: review?(matt.woodrow) → review+
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Keywords: leave-open
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.