935265 - Support 3D transforms in BasicCompositor

Status: RESOLVED FIXED

(Core :: Graphics: Layers, defect)

Description

[David Anderson [:dvander] - inactive, e-mail if emergency]

Right now the basic compositor can only perform 2D transforms. To enable OMTC everywhere, we'll need it to support 3D transforms as well.

Comment 1

[David Anderson [:dvander] - inactive, e-mail if emergency]

Attachment: WIP v0

Comment 2

[Nick Cameron [:nrc]]

I'm not sure if you will be adaapting it, but I just changed the 3d transforms stuff in basic layer manager. See patch 1.5 in bug 932888 (hopefully it will land soon, it is blocked by a chain of stuff).

Comment 3

[David Anderson [:dvander] - inactive, e-mail if emergency]

Thanks for the info - I was indeed adapting that exact code

Comment 4

[David Anderson [:dvander] - inactive, e-mail if emergency]

Attachment: v1

seems to pass tests (one fails, but it also fails on basic layers)

Comment 5

[Matt Woodrow (:mattwoodrow)]

Comment on attachment 828159 [details] [diff] [review]
v1

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

::: gfx/layers/basic/BasicCompositor.cpp
@@ +445,5 @@
> +  if (aTransform.Is2D()) {
> +    newTransform = aTransform.As2D();
> +  } else {
> +    // Create a temporary surface for the transform.
> +    tempSurface = Factory::CreateDataSourceSurface(IntSize(aRect.width, aRect.height), FORMAT_B8G8R8A8);

aRect.Size()

@@ +453,5 @@
> +    dest = gfxPlatform::GetPlatform()->CreateDrawTargetForData(
> +      tempSurface->GetData(),
> +      tempSurface->GetSize(),
> +      tempSurface->Stride(),
> +      tempSurface->GetFormat()

CreateDrawTargetForData doesn't work for all Moz2D backends at the moment. Can we instead replace both of these calls with gfxPlatform::CreateOffscreenContentDrawTarget, and then use dest->Snapshot()->AsDataSurface() to get a surface to pass to PixmanTransform

@@ +460,5 @@
> +      return;
> +
> +    // Get the bounds post-transform.
> +    To3DMatrix(aTransform, new3DTransform);
> +    transformBounds = new3DTransform.TransformBounds(gfxRect(aRect.x, aRect.y, aRect.width, aRect.height));

gfx/thebes/gfx2DGlue.h has ThebesRect() for this conversion.

@@ +462,5 @@
> +    // Get the bounds post-transform.
> +    To3DMatrix(aTransform, new3DTransform);
> +    transformBounds = new3DTransform.TransformBounds(gfxRect(aRect.x, aRect.y, aRect.width, aRect.height));
> +    transformBounds.IntersectRect(transformBounds,
> +      gfxRect(aOffset.x, aOffset.y, buffer->GetSize().width, buffer->GetSize().height));

If this patch lands after bug 935380, you'll need to change aOffset to mRenderTarget->GetOrigin().

@@ +470,5 @@
> +    newTransform.Translate(transformBounds.x, transformBounds.y);
> +
> +    // When we apply the 3D transformation, we do it against a temporary
> +    // surface, so undo the coordinate offset.
> +    new3DTransform = new3DTransform * gfx3DMatrix::Translation(-transformBounds.x, -transformBounds.y, 0);

Since we're modifying the transform to put things at 0,0 this would change transformBounds if you computed it again.

How about calling transformBounds.MoveTo(Point()); here (changing it to 0,0,transformBounds.width,transformBounds.height), and then you can just use transformBounds directly for the two parameters to DrawSurface below.

@@ +542,5 @@
> +  if (!aTransform.Is2D()) {
> +    dest->Flush();
> +
> +    RefPtr<DataSourceSurface> temp =
> +      Factory::CreateDataSourceSurface(IntSize(transformBounds.width, transformBounds.height), FORMAT_B8G8R8A8);

transformBounds.Size()

Comment 6

[David Anderson [:dvander] - inactive, e-mail if emergency]

Attachment: v2

w/ comments addressed

Comment 7

[Matt Woodrow (:mattwoodrow)]

Comment on attachment 828348 [details] [diff] [review]
v2

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

::: gfx/layers/basic/BasicCompositor.cpp
@@ +346,5 @@
> +
> + private:
> +  RefPtr<DrawTarget> mDrawTarget;
> +  Matrix mOldTransform;
> +};

You could move this to gfx/2d/Helpers.h if you wanted (which is bizarrely empty at the moment), since it'll probably be useful for other people too.

Comment 8

[David Anderson [:dvander] - inactive, e-mail if emergency]

https://hg.mozilla.org/integration/mozilla-inbound/rev/4213b23a0737

Comment 9

[Andreas Gal :gal]

\o/

Comment 10

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/4213b23a0737