1574745 - Preparations for native layers that only cover part of the window

Status: RESOLVED FIXED

(Core :: Graphics: Layers, task)

Comment 1

[Markus Stange [:mstange]]

Attachment: Bug 1574745 - Clarify dest and buffer a little.

The old code was making an autoRestoreTransform around dest and then quickly
overwriting dest with something new. I prefer creating the autoRestoreTransform
for buffer, which doesn't change over the course of this function.

Comment 2

[Markus Stange [:mstange]]

Attachment: Bug 1574745 - Change the coordinate space of CalculateScissorRect and DrawGeometry's aClipRect.

In the past, this rectangle was relative to the intermediate surface of the
closest ancestor container layer with intermediate surface, or in root layer
coordinates if there was no such ancestor layer.
After this patch, the rectangle no longer relative to the intermediate surface;
it is now in the same space as the surface rectangle. The surface origin is no
longer used as an offset for the clip rect.

Example: Container layer with intermediate surface has visible region
(100, 200, 400, 300) and the clip covers the entire surface.
In the past, aClipRect would be (0, 0, 400, 300).
After this patch, the clip rect is (100, 200, 400, 300).

Background:

Before this patch, Compositor::DrawGeometry was called with aClipRect in
relative-to-intermediate-surface-or-root-layer space. All compositor
implementations were treating the aClipRect coordinates as relative to the
current render target, i.e. relative to the current render target's origin.

This worked fine when you were actually rendering to an intermediate surface.
But when rendering to the initial render target, the two spaces are not
necessarily equivalent: Root layer coordinates are not necessarily relative to
the initial render target, because the root layer space has its origin in the
top left corner of the window, whereas the initial render target does not
necessarily have to have its origin in the top left corner of the window.

To be fair, in CompositorOGL and CompositorD3D11, the initial render target
actually does have its origin in the top left corner of the window.
The notable exception is BasicCompositor when BasicCompositor uses a back
buffer: The back buffer only covers the invalid rect, so the render target
around the back buffer has its origin in the top left corner of the invalid
rect, which can be anywhere within the window.
As a result, when drawing into the back buffer render target (as opposed to an
intermediate surface), BasicCompositor::DrawGeometry's aClipRect was not
relative to the render target. So how did BasicCompositor know to clip the back
buffer in the right place? It would need to offset aClipRect by rt->GetOffset()
only for the initial render target, but not for render targets that were
created for intermediate surfaces, because for those render targets the
coordinates would already be correct.
The key difference was the back buffer DrawTarget's transform:
CreateRenderTargetForWindow was setting the transform on the initial render
target's DrawTarget to Translation(-rt->GetOrigin()).
And CreateRenderTarget, which creates render targets for intermediate surfaces,
was not setting a transform on the DrawTargets it creates.
So BasicCompositor::DrawGeometry was using the current render target's transform
only to correct aClipRect's offset, and for nothing else. Directly after
applying the clip, it would overwrite the render target's transform with a new
transform which included the GetOrigin() offset in all types of render targets.

I was rather surprised when I found out about this.

If I wanted to make CompositorOGL behave similarly when drawing to a native
layer which is not located at (0, 0) in the window, I would have to add a
similar annotation on the render target for the native layer that says: "Here's
an extra offset that you need to apply only to clip rects and to nothing else
when rendering to this render target, because Layer::CalculateScissorRect
predicts the wrong surface location for the initial render target".
I'm not a fan of special offsets that only apply in special cases.

So instead of predicting an incorrect offset and then correcting for the
misprediction afterwards, this patch makes us no longer subtract the
intermediate surface offset in CalculateScissorRect(). So then DrawGeometry just
always needs to offset the clip rect by mCurrentRenderTarget->GetOrigin() (which
is the correct offset, known in the correct place) in order to translate the
clip rect into physical render target coordinates.

Since this removes a difference in treatment between BasicCompositor's initial
render target and intermediate render targets, I'm also removing the code that
sets a transform on the initial render target in CreateRenderTargetForWindow.

Comment 3

[Markus Stange [:mstange]]

Attachment: Bug 1574745 - Prepare CompositorOGL for native layers that are not located at (0, 0). r=mattwoodrow

Comment 4

[Markus Stange [:mstange]]

Attachment: Bug 1574745 - Two random cleanups.

Comment 5

[Markus Stange [:mstange]]

Attachment: Bug 1574745 - Use an explicit offset to deal with the fact that aClipRect in Compositor::DrawGeometry is relative to (0, 0) for the root render target, and not in the render target's device space. r=mattwoodrow

aClipRect is relative to
closestAncestorContainerLayerWithIntermediateSurface->GetVisibleRegion().GetBounds().TopLeft()
or (0, 0) if there is no ancestor with an intermediate surface.
It gets computed by Layer::CalculateScissorRect.
So if there is an intermediate surface, that origin matches the render target
origin. But if there is no intermediate surface, it does not always match: the
root render target's origin is not necessarily (0, 0).

In the past, this was dealt with using the transform on the root render target's
mDrawTarget, which gets set in CreateRenderTargetForWindow (renamed to
CreateRootRenderTarget in this patch). Render targets created in the regular
CreateRenderTarget did not have a transform.
Now all render targets have a translation by -offset on their DrawTarget, not just
the root render target.

I went with an explicit "clip space origin" field on the render target.
Other alternatives would have been:

• Having a bool IsRootRenderTarget() and using that to conditionally offset
  aClipRect by the render target origin or not.
• Changing Layer::CalculateScissorRect so that the clip space origin is always
  (0, 0). I actually tried this first but ran into trouble with the MLGPU code.
  We can do it later.

Comment 6

[Markus Stange [:mstange]]

Attachment: Bug 1574745 - Prepare BasicCompositor for layers that are not located at (0, 0). r=mattwoodrow

Depends on D43866

Comment 7

[Markus Stange [:mstange]]

Attachment: Bug 1574745 - Treat aClipRect in CompositorOGL::DrawGeometry correctly when drawing to a root render target that's not at (0, 0). r=mattwoodrow

This uses the same workaround as BasicCompositor.

Depends on D43867

Comment 8

[Markus Stange [:mstange]]

Attachment: Bug 1574745 - Respect render target clip during DrawGeometry culling and overdraw pixel counting. r=mattwoodrow

Depends on D42424

Comment 9

[Markus Stange [:mstange]]

Attachment: Bug 1574745 - Make DrawGeometry culling work more reliably. r=mattwoodrow

TransformAndClipBounds already clips destRect to renderBounds. However, if the
resulting rect is empty, the Inflate call would make it non-empty again. And now
the decision whether we would cull would depend on the rect's position:
Sometimes TransformAndClipBounds would place the empty rect at a position along
an edge of renderBounds, and then the inflated-from-nothingness 2x2 rect would
overlap and we wouldn't cull.

Depends on D43869

Comment 10

[Markus Stange [:mstange]]

Attachment: Bug 1574745 - Rename BeginFrame to BeginFrameForWindow. r=mattwoodrow

In the end we want to have BeginFrameForWindow, BeginFrameForTarget, and
BeginFrameForNativeLayers, the latter with multiple Begin/EndRenderingToNativeLayer
pairs nested inside.

This is the first step.

CompositorOGL and CompositorD3D11 keep their internal BeginFrame method but make
it private.

Depends on D43870

Comment 11

[Markus Stange [:mstange]]

Attachment: Bug 1574745 - Add BeginFrameForTarget and replace Set/ClearTargetContext. r=mattwoodrow

Depends on D43871

Comment 12

[Markus Stange [:mstange]]

Attachment: Bug 1574745 - Add BeginFrameForNativeLayers() and Begin/EndRenderingToNativeLayer. r=mattwoodrow

This will let us render into multiple layers within one compositor frame.

Depends on D43872

Comment 13

[Markus Stange [:mstange]]

Attachment: Bug 1574745 - Calculate mShouldInvalidateWindow at the beginning of the frame. r=mattwoodrow

Depends on D43873

Comment 14

[Markus Stange [:mstange]]

Attachment: Bug 1574745 - Move ShouldRecordFrames() from BasicCompositor to Compositor because CompositorOGL will need it. r=mattwoodrow

Depends on D43874

Comment 15

[Markus Stange [:mstange]]

Attachment: Bug 1574745 - Change BlitFrameBufferToFrameBuffer to take rects instead of sizes. r=jgilbert

Depends on D43875

Comment 16

[Markus Stange [:mstange]]

Attachment: Bug 1574745 - Maintain a full window render target when rendering to native layers with CompositorOGL. r=mattwoodrow

This makes profiler screenshots and frame recording work again, when native
layers are used.

Depends on D43876

Comment 17

[Markus Stange [:mstange]]

Attachment: Bug 1574745 - Reorder layer rendering invocations. r=mattwoodrow

This change groups the work that has to be done per compositing tile.

Depends on D43877

Comment 18

[Markus Stange [:mstange]]

Attachment: Bug 1574745 - Get everything ready for rendering into multiple native layers. r=mattwoodrow

Depends on D43878

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/610cc17c5832
Use an explicit offset to deal with the fact that aClipRect in Compositor::DrawGeometry is relative to (0, 0) for the root render target, and not in the render target's device space. r=mattwoodrow
https://hg.mozilla.org/integration/autoland/rev/8a64ef16fc2e
Prepare BasicCompositor for layers that are not located at (0, 0). r=mattwoodrow
https://hg.mozilla.org/integration/autoland/rev/de0fadd41dc3
Treat aClipRect in CompositorOGL::DrawGeometry correctly when drawing to a root render target that's not at (0, 0). r=mattwoodrow
https://hg.mozilla.org/integration/autoland/rev/dd092bce314d
Prepare CompositorOGL for native layers that are not located at (0, 0). r=mattwoodrow
https://hg.mozilla.org/integration/autoland/rev/f988e453af00
Respect render target clip during DrawGeometry culling and overdraw pixel counting. r=mattwoodrow
https://hg.mozilla.org/integration/autoland/rev/cc84a0e9d5dd
Make DrawGeometry culling work more reliably. r=mattwoodrow
https://hg.mozilla.org/integration/autoland/rev/6035e8ef4f82
Rename BeginFrame to BeginFrameForWindow. r=mattwoodrow
https://hg.mozilla.org/integration/autoland/rev/1591247f9ce8
Add BeginFrameForTarget and replace Set/ClearTargetContext. r=mattwoodrow
https://hg.mozilla.org/integration/autoland/rev/4fcd04e8261e
Add BeginFrameForNativeLayers() and Begin/EndRenderingToNativeLayer. r=mattwoodrow
https://hg.mozilla.org/integration/autoland/rev/ee0a3838c5c1
Calculate mShouldInvalidateWindow at the beginning of the frame. r=mattwoodrow
https://hg.mozilla.org/integration/autoland/rev/6d1b97aed6f0
Move ShouldRecordFrames() from BasicCompositor to Compositor because CompositorOGL will need it. r=mattwoodrow
https://hg.mozilla.org/integration/autoland/rev/29be5f4832ca
Change BlitFrameBufferToFrameBuffer to take rects instead of sizes. r=jgilbert
https://hg.mozilla.org/integration/autoland/rev/ea690c643c8c
Maintain a full window render target when rendering to native layers with CompositorOGL. r=mattwoodrow
https://hg.mozilla.org/integration/autoland/rev/1db2a705c75c
Reorder layer rendering invocations. r=mattwoodrow
https://hg.mozilla.org/integration/autoland/rev/56928dc94586
Get everything ready for rendering into multiple native layers. r=mattwoodrow

Comment 20

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/25005fcdb106
Fix bustages on OS X Cross Compiled. a=bustage-fix CLOSED TREE

Comment 21

[Alexandru Michis [:malexandru]]

I made a quick fix for the bustages seen here:
https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception%2Crunnable&fromchange=56928dc94586cba272761f8af49e67795baab0bf&tochange=25005fcdb10689745308d4eab5d67f4dd2b034ab&selectedJob=264370513

Try push for it:
https://treeherder.mozilla.org/#/jobs?repo=try&group_state=expanded&revision=651bae54491958afb27026f8296839f2c3be406f

Comment 22

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/610cc17c5832
https://hg.mozilla.org/mozilla-central/rev/8a64ef16fc2e
https://hg.mozilla.org/mozilla-central/rev/de0fadd41dc3
https://hg.mozilla.org/mozilla-central/rev/dd092bce314d
https://hg.mozilla.org/mozilla-central/rev/f988e453af00
https://hg.mozilla.org/mozilla-central/rev/cc84a0e9d5dd
https://hg.mozilla.org/mozilla-central/rev/6035e8ef4f82
https://hg.mozilla.org/mozilla-central/rev/1591247f9ce8
https://hg.mozilla.org/mozilla-central/rev/4fcd04e8261e
https://hg.mozilla.org/mozilla-central/rev/ee0a3838c5c1
https://hg.mozilla.org/mozilla-central/rev/6d1b97aed6f0
https://hg.mozilla.org/mozilla-central/rev/29be5f4832ca
https://hg.mozilla.org/mozilla-central/rev/ea690c643c8c
https://hg.mozilla.org/mozilla-central/rev/1db2a705c75c
https://hg.mozilla.org/mozilla-central/rev/56928dc94586
https://hg.mozilla.org/mozilla-central/rev/25005fcdb106

Comment 23

[Markus Stange [:mstange]]

(In reply to Alexandru Michis [:malexandru] from comment #21)

I made a quick fix for the bustages

Thank you!! I'm not sure how this happened. I suppose I must have always compiled when I had the patch from bug 1574586 applied as well, which removes the offending line.

Comment 24

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/releases/mozilla-beta/rev/610cc17c5832
https://hg.mozilla.org/releases/mozilla-beta/rev/8a64ef16fc2e
https://hg.mozilla.org/releases/mozilla-beta/rev/de0fadd41dc3
https://hg.mozilla.org/releases/mozilla-beta/rev/dd092bce314d
https://hg.mozilla.org/releases/mozilla-beta/rev/f988e453af00
https://hg.mozilla.org/releases/mozilla-beta/rev/cc84a0e9d5dd
https://hg.mozilla.org/releases/mozilla-beta/rev/6035e8ef4f82
https://hg.mozilla.org/releases/mozilla-beta/rev/1591247f9ce8
https://hg.mozilla.org/releases/mozilla-beta/rev/4fcd04e8261e
https://hg.mozilla.org/releases/mozilla-beta/rev/ee0a3838c5c1
https://hg.mozilla.org/releases/mozilla-beta/rev/6d1b97aed6f0
https://hg.mozilla.org/releases/mozilla-beta/rev/29be5f4832ca
https://hg.mozilla.org/releases/mozilla-beta/rev/ea690c643c8c
https://hg.mozilla.org/releases/mozilla-beta/rev/1db2a705c75c
https://hg.mozilla.org/releases/mozilla-beta/rev/56928dc94586
https://hg.mozilla.org/releases/mozilla-beta/rev/25005fcdb106