Closed Bug 1573343 Opened 2 years ago Closed 2 years ago

Some BasicCompositor cleanup

Categories

(Core :: Graphics: Layers, task, P3)

task

Tracking

()

RESOLVED FIXED
mozilla70
Tracking Status
firefox70 --- fixed

People

(Reporter: mstange, Assigned: mstange)

Details

Attachments

(12 files)

47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
No description provided.

This should be equivalent to what we had before but clearer.
Before this patch, we would reallocate the back buffer if

  • the back buffer was larger than the client size (meaning the window had shrunk) or
  • the invalid region didn't fit in the back buffer.
    Whenever the window is resized, we have a frame where the invalid region covers
    the entire window, and thus size would be equal clientSize in that frame.
    So effectively, the back buffer size was always kept in sync with the client size.

On Windows, where the DrawTarget is rewrapped in a Cairo/pixman DrawTarget,
this will now cause Cairo/pixman to be used for the clear.

Depends on D41672

In the past, BeginFrame would clip mDrawTarget to the invalid region and then have
CreateRenderTargetForWindow clear a rectangle. So it was effectively clearing a
region, but only if BeginFrame's region clip happened to be set on the correct
DrawTarget. In the case where a back buffer was used, this was not the case:
The clip would be on mDrawTarget but the clear would happen on the back buffer
draw target. So we were clearing more than necessary.

Now we limit clearing to the invalid region even if a back buffer is used.

Depends on D41674

There was an interesting "ExpandToEnclose(IntPoint(0, 0))" call in
CreateRenderTargetForWindow that snuck in some surprising behavior that this
change makes a bit more visible.

Depends on D41675

The null is unnecessary because mDrawTarget is already null-checked above:
If mTarget is non-null, then mDrawTarget = mTarget so mDrawTarget is non-null.
If mTarget is null, then we would enter a branch that had another null check
for mDrawTarget.

As for the assertion: When rendering to an external target (reftests),
LayerManagerComposite always sets the invalid region to the entire window, so it
should never be empty here.

Depends on D41676

ScreenPixels would be a more appropriate unit. But inside BasicCompositor everything
is in the same coordinate space anyway so we're not getting much benefit from the
units. This patch eliminates a lot of .ToUnknown*() calls.

Depends on D41679

Pushed by mstange@themasta.com:
https://hg.mozilla.org/integration/autoland/rev/16f39b7ca09c
Always create the back buffer sized to the widget's client size. r=mattwoodrow
https://hg.mozilla.org/integration/autoland/rev/9c974e6fce9b
Reset the transform in a better place. r=mattwoodrow
https://hg.mozilla.org/integration/autoland/rev/1e4d840a8982
Move draw target clearing out of GetBackBufferDrawTarget. r=mattwoodrow
https://hg.mozilla.org/integration/autoland/rev/3c32857ee6ec
Apply the translation in CreateRenderTargetForWindow instead of in BeginFrame. r=mattwoodrow
https://hg.mozilla.org/integration/autoland/rev/7bbd8ac1b408
Pass a precise clear region to CreateRenderTargetForWindow. r=mattwoodrow
https://hg.mozilla.org/integration/autoland/rev/fe629a74a577
Add mDrawTargetBounds to clarify the position of mDrawTarget in window space. r=mattwoodrow
https://hg.mozilla.org/integration/autoland/rev/8c08fc16b204
Remove an unnecessary null check and turn an emptiness check into an assertion. r=mattwoodrow
https://hg.mozilla.org/integration/autoland/rev/9d0fed01b154
Fix two comments in BasicCompositor. r=mattwoodrow
https://hg.mozilla.org/integration/autoland/rev/c69027259ea0
Make the aInvalidRegion parameter of EndRemoteDrawing a const reference rather than a regular reference. r=mattwoodrow
https://hg.mozilla.org/integration/autoland/rev/3ab4673f6804
Stop using LayoutDevice units in BasicCompositor. r=mattwoodrow
https://hg.mozilla.org/integration/autoland/rev/b585cff4e2c8
Use std::any_of to compute hasContentPaint. r=mattwoodrow
https://hg.mozilla.org/integration/autoland/rev/14bd73fd1e01
Destroy mFullWindowRenderTarget in a different place. r=mattwoodrow
You need to log in before you can comment on or make changes to this bug.