Closed Bug 935265 Opened 11 years ago Closed 11 years ago

Support 3D transforms in BasicCompositor

Categories

(Core :: Graphics: Layers, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla28

People

(Reporter: dvander, Assigned: dvander)

References

Details

(Whiteboard: [qa-])

Attachments

(1 file, 2 obsolete files)

Right now the basic compositor can only perform 2D transforms. To enable OMTC everywhere, we'll need it to support 3D transforms as well.
Assignee: nobody → dvander
Attached patch WIP v0 (obsolete) — Splinter Review
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).
Thanks for the info - I was indeed adapting that exact code
Attached patch v1 (obsolete) — Splinter Review
seems to pass tests (one fails, but it also fails on basic layers)
Attachment #827666 - Attachment is obsolete: true
Attachment #828159 - Flags: review?(matt.woodrow)
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()
Attached patch v2Splinter Review
w/ comments addressed
Attachment #828159 - Attachment is obsolete: true
Attachment #828159 - Flags: review?(matt.woodrow)
Attachment #828348 - Flags: review?(matt.woodrow)
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.
Attachment #828348 - Flags: review?(matt.woodrow) → review+
\o/
https://hg.mozilla.org/mozilla-central/rev/4213b23a0737
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla28
Whiteboard: [qa-]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: