1573343 - Some BasicCompositor cleanup

Status: RESOLVED FIXED

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

Comment 1

[Markus Stange [:mstange]]

Attachment: Bug 1573343 - Always create the back buffer sized to the widget's client size. r=mattwoodrow

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.

Comment 2

[Markus Stange [:mstange]]

Attachment: Bug 1573343 - Reset the transform in a better place. r=mattwoodrow

Depends on D41671

Comment 3

[Markus Stange [:mstange]]

Attachment: Bug 1573343 - Move draw target clearing out of GetBackBufferDrawTarget. r=mattwoodrow

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

Comment 4

[Markus Stange [:mstange]]

Attachment: Bug 1573343 - Apply the translation in CreateRenderTargetForWindow instead of in BeginFrame. r=mattwoodrow

Depends on D41673

Comment 5

[Markus Stange [:mstange]]

Attachment: Bug 1573343 - Pass a precise clear region to CreateRenderTargetForWindow. r=mattwoodrow

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

Comment 6

[Markus Stange [:mstange]]

Attachment: Bug 1573343 - Add mDrawTargetBounds to clarify the position of mDrawTarget in window space. r=mattwoodrow

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

Comment 7

[Markus Stange [:mstange]]

Attachment: Bug 1573343 - Remove an unnecessary null check and turn an emptiness check into an assertion. r=mattwoodrow

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

Comment 8

[Markus Stange [:mstange]]

Attachment: Bug 1573343 - Fix two comments in BasicCompositor. r=mattwoodrow

Depends on D41677

Comment 9

[Markus Stange [:mstange]]

Attachment: Bug 1573343 - Make the aInvalidRegion parameter of EndRemoteDrawing a const reference rather than a regular reference. r=mattwoodrow

There's no point in mutating it, and nobody does.

Depends on D41678

Comment 10

[Markus Stange [:mstange]]

Attachment: Bug 1573343 - Stop using LayoutDevice units in BasicCompositor. r=mattwoodrow

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

Comment 11

[Markus Stange [:mstange]]

Attachment: Bug 1573343 - Use std::any_of to compute hasContentPaint. r=mattwoodrow

Depends on D41680

Comment 12

[Markus Stange [:mstange]]

Attachment: Bug 1573343 - Destroy mFullWindowRenderTarget in a different place. r=mattwoodrow

Depends on D41681

Comment 13

[Pulsebot]

Pushed by [e-mail]:
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

Comment 14

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/16f39b7ca09c
https://hg.mozilla.org/mozilla-central/rev/9c974e6fce9b
https://hg.mozilla.org/mozilla-central/rev/1e4d840a8982
https://hg.mozilla.org/mozilla-central/rev/3c32857ee6ec
https://hg.mozilla.org/mozilla-central/rev/7bbd8ac1b408
https://hg.mozilla.org/mozilla-central/rev/fe629a74a577
https://hg.mozilla.org/mozilla-central/rev/8c08fc16b204
https://hg.mozilla.org/mozilla-central/rev/9d0fed01b154
https://hg.mozilla.org/mozilla-central/rev/c69027259ea0
https://hg.mozilla.org/mozilla-central/rev/3ab4673f6804
https://hg.mozilla.org/mozilla-central/rev/b585cff4e2c8
https://hg.mozilla.org/mozilla-central/rev/14bd73fd1e01