1064479 - Redo how screen rotation is implemented on B2G/Layers

Status: RESOLVED FIXED

(Core :: Graphics: Layers, defect)

Description

[Jeff Muizelaar [:jrmuizel]]

Currently we use a world transform on the LayerManager. This gets passed to the screen render target which adjusts the gl transform to accomplish the rotation. This causes a lot of the system to have to know about the transform.

Instead we can just bake the transform into the root layer's shadow transform. Everything now mostly just works.

Comment 1

[Jeff Muizelaar [:jrmuizel]]

Attachment: Rough out an implementation

Comment 2

[Jeff Muizelaar [:jrmuizel]]

Attachment: Implementation and cleanup included together

This patch shows the simplication that this approach allows. It will also fix bug 1057461

Comment 3

[Jeff Muizelaar [:jrmuizel]]

Attachment: Implementation and cleanup included together

Comment 4

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 8485922 [details] [diff] [review]
Rough out an implementation

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

The main part of this change is:

::: gfx/layers/composite/AsyncCompositionManager.cpp
@@ +126,2 @@
>        ComputeTransformForRotation(mTargetConfig.naturalBounds(),
> +                                  mTargetConfig.rotation());

Store the world transform on the AsyncCompositionManager instead of the layer manager.

@@ +980,5 @@
> +  LayerComposite* rootComposite = root->AsLayerComposite();
> +
> +  gfx::Matrix4x4 trans = rootComposite->GetShadowTransform();
> +  trans *= gfx::Matrix4x4::From2D(mWorldTransform);
> +  rootComposite->SetShadowTransform(trans);

This is the main part of this change.

::: gfx/layers/ipc/CompositorParent.cpp
@@ +800,5 @@
>  
>    // Instruct the LayerManager to update its render bounds now. Since all the orientation
>    // change, dimension change would be done at the stage, update the size here is free of
>    // race condition.
> +  mLayerManager->UpdateRenderBounds(aTargetConfig.naturalBounds());

Set the render bounds to the natural bounds instead of the client bounds so that we don't have to unrotate them in CompositorOGL.

Comment 5

[Matt Woodrow (:mattwoodrow)]

Comment on attachment 8485922 [details] [diff] [review]
Rough out an implementation

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

A nice simplification.

Comment 6

[Milan Sreckovic [:milan] (needinfo for best results)]

This is the solution to bug 1057461, making it 2.1+

Comment 7

[Jeff Muizelaar [:jrmuizel]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/0ba56b8f5946

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/0ba56b8f5946

Comment 9

[No-Jun Park [:njpark]]

Actually, before uplifting it to 2.1, could we check whether it's not causing Bug 1066664?  The image I see there is quite similar to the Bug 1057461

Comment 10

[Jason Smith [:jsmith]]

Marking NO_UPLIFT to check to see if this caused the fallout in bug 1066664.

Jeff - Can you find out if this caused bug 1066664?

Comment 11

[No-Jun Park [:njpark]]

Clearing NO_UPLIFT flag since Bug 1066664 landed in master.  This should land in 2.1 with Bug 1066664

Comment 12

[Ryan VanderMeulen [:RyanVM]]

Please nominate this patch for Aurora approval when you get a chance :)

Comment 13

[Milan Sreckovic [:milan] (needinfo for best results)]

Comment on attachment 8485953 [details] [diff] [review]
Implementation and cleanup included together

Approval Request Comment
See the Aurora request comments on bug 1071241.

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/d96e19abd61e