602431 - Post-process shadow layer tree to make its coordinates match up with parent's layer tree

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

We need to
 (1) add a ShadowLayer base class
 (2) add ShadowContainerLayer and ShadowColorLayer
 (3) give ShadowLayer its own mShadowTransform, mShadowVisible, and mShadowClip (I think that's all we need atm)
 (4) on shadow-tree updates, run a pass over the shadow tree somewhat analogous to FrameLayerBuilder (but much simpler!) to update the shadows' mShadowVisible et al. to be match compositor-process coordinates, taking viewportScale/Scroll etc. into account.  This will be an expanded version of the current SetTransformFor() in RenderFrameParent.
 (5) have *Shadow*Layer*::Paint/Render use the mShadowVisible et al. values when painting

We need all of these to be able to composite shadow layers with the GL backend.  (Various hacks to get this working to various degrees are possible, but they're going to be fragile.  Let's fix it for realz now.)  We'll extend part (4) in bug 586288 to also take sub-frame scrolling into account when updating mShadow*.

Comment 1

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Note re: risk: we can implement all this and (mostly) leave the current basic layers code alone, since it's forgiving enough to work with the values from the content process.

Comment 2

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

(In reply to comment #0)
>  (4) on shadow-tree updates, run a pass over the shadow tree somewhat analogous
> to FrameLayerBuilder

Correction: this should run on nsDisplayRemote::BuildLayer().

Comment 3

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

We might be able to re-use this mechanism for recompositing layer subtrees in -moz-element and <canvas> for both local and remote layer trees: create a shadow subtree from the original subtree and then adjust the shadow attributes as necessary for the secondary render target.

Comment 4

[Brian Crowder]

Making you (cjones) own this for now.  Disown as you deem necessary.

Comment 5

[Robert O'Callahan (:roc) (email my personal email if necessary)]

It's not clear to me why we need to add mShadow state. Why can't we just pass a context parameter block down through Paint/Render which we use to compute the final state-to-use?

Comment 6

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

The hard requirement for this is to transform (well, translate and scale) subtrees of the main shadow tree independently of the main tree (bug 586288).  We don't want to touch the layers' own transform/clip/visible values because the shadow is immutable in the parent process.  I can't think of a sane way to pass a context block down the tree to implement that; we can't do this with basic layers either.

Another cool use (though farfetched) for this would be to implement a "layers-spy" app.  We could use shadow attributes to split the sublayers apart.

We can also use this feature to correct even untransformed shadow layers' visible and clip rects to parent-window space.  Passing a transform matrix down the tree would also work, but that would change layer semantics, and we'd have to implement that for D3D9 and D3D10 as well.  I'd rather fix up shadow layers so that they don't (completely) rely on cairo.  (The exception is resolution-scaled drawing.)

This would also work well for -moz-element and <canvas>, but passing down a context matrix would work too.  Same case as above.

Comment 7

[Robert O'Callahan (:roc) (email my personal email if necessary)]

OK, that makes sense.

Comment 8

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Attachment: part 1: Add a ShadowLayer base class

Comment 9

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Attachment: part 2: Add a ShadowColorLayer base class and BasicShadowColorLayer implementation

Comment 10

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Attachment: part 3: Add a ShadowContainerLayer base class and BasicShadowContainerLayer implementation

Comment 11

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Attachment: part 4: Refactor computation of <browser> shadow-tree transforms into its own function

Comment 12

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Attachment: part 5: Add an extra phase to update shadow-layer attributes (not used for anything interesting yet)

Comment 13

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Attachment: part 6: Honor shadow-layer attributes in the basic layers backend and take off the container-transform training wheels

Comment 14

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Attachment: part 7: Honor shadow-layer attributes in the GL layers backend

Comment 15

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

What I had in mind for applying this mechanism with -moz-element and canvas is
 - move ShadowLayers.h into gfx/layers
 - make IPC layers special cases of shadows, Shadow*LayerIPC or something
 - allow creating Shadow*Layer from *Layer
 - postprocess Shadow*Layers for particular use

Comment 16

[Robert O'Callahan (:roc) (email my personal email if necessary)]

That might be overkill for -moz-element. I was thinking of creating a new layer type, which might have some advantages. But we can discuss that elsewhere.

Comment 17

[Timothy Nikkel (:tnikkel)]

Comment on attachment 482463 [details] [diff] [review]
part 5: Add an extra phase to update shadow-layer attributes (not used for anything interesting yet)

>+TransformShadowTreeTo(ContainerLayer* aRoot,
>+  gfxMatrix shadowTransform;
>+  shadow->GetShadowTransform().Is2D(&shadowTransform);
>+  // Pre-multiply this transform into the shadow's transform, so that
>+  // it occurs before any transform set by the child
>+  shadowTransform.Translate(gfxPoint(aTranslation.x, aTranslation.y));
>+  shadowTransform.Scale(aXScale, aYScale);
>+  shadow->SetShadowTransform(gfx3DMatrix::From2D(shadowTransform));

It seems like every time BuildLayer calls this function we will adding one more copy of the translate and scale. Is that not the case?

Comment 18

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

(In reply to comment #17)
> Comment on attachment 482463 [details] [diff] [review]
> part 5: Add an extra phase to update shadow-layer attributes (not used for
> anything interesting yet)
> 
> >+TransformShadowTreeTo(ContainerLayer* aRoot,
> >+  gfxMatrix shadowTransform;
> >+  shadow->GetShadowTransform().Is2D(&shadowTransform);
> >+  // Pre-multiply this transform into the shadow's transform, so that
> >+  // it occurs before any transform set by the child
> >+  shadowTransform.Translate(gfxPoint(aTranslation.x, aTranslation.y));
> >+  shadowTransform.Scale(aXScale, aYScale);
> >+  shadow->SetShadowTransform(gfx3DMatrix::From2D(shadowTransform));
> 
> It seems like every time BuildLayer calls this function we will adding one more
> copy of the translate and scale. Is that not the case?

No.  The UpdateShadowTree() pass will copy over the attributes from the child process, then this code will modify those attributes anew on each BuildLayer.

Comment 19

[Timothy Nikkel (:tnikkel)]

(In reply to comment #18)
> No.  The UpdateShadowTree() pass will copy over the attributes from the child
> process, then this code will modify those attributes anew on each BuildLayer.

Ah, ok.

Comment 20

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

http://hg.mozilla.org/mozilla-central/rev/0a5ff3652a09
http://hg.mozilla.org/mozilla-central/rev/a27e7290bad9
http://hg.mozilla.org/mozilla-central/rev/0beb5c409b69
http://hg.mozilla.org/mozilla-central/rev/d35dc3b25cfa
http://hg.mozilla.org/mozilla-central/rev/d580cff50255
http://hg.mozilla.org/mozilla-central/rev/8652586c3908
http://hg.mozilla.org/mozilla-central/rev/6ed4740747a0