CSS 3d Cube is very slow in OGL backend

RESOLVED FIXED in mozilla10

Status

()

Core
Graphics
RESOLVED FIXED
6 years ago
11 months ago

People

(Reporter: romaxa, Assigned: mattwoodrow)

Tracking

Trunk
mozilla10
x86
Linux
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [inbound])

Attachments

(2 attachments, 2 obsolete attachments)

(Reporter)

Description

6 years ago
Created attachment 562623 [details]
Test case

Attached testcase is very slow in OGL Fennec/Firefox,
profile data showing that we allocating temporary buffers for each paint...
**********************
#5  0x3c473f74 in mozilla::gl::GLContext::fDeleteTextures (this=<value optimized out>, names=0x43b83a80, n=<value optimized out>) at ../../dist/include/GLContext.h:2191
#6  0x3c474974 in ContainerRender<mozilla::layers::ShadowContainerLayerOGL> (this=0x3b190d40, aPreviousFrameBuffer=0, aOffset=<value optimized out>)
    at gfx/layers/opengl/ContainerLayerOGL.cpp:290
#7  mozilla::layers::ShadowContainerLayerOGL::RenderLayer (this=0x3b190d40, aPreviousFrameBuffer=0, aOffset=<value optimized out>)
    at gfx/layers/opengl/ContainerLayerOGL.cpp:386
#8  0x3c47482c in ContainerRender<mozilla::layers::ShadowContainerLayerOGL> (this=0x3b18fc60, aPreviousFrameBuffer=0, aOffset=<value optimized out>)
    at gfx/layers/opengl/ContainerLayerOGL.cpp:246
#9  mozilla::layers::ShadowContainerLayerOGL::RenderLayer (this=0x3b18fc60, aPreviousFrameBuffer=0, aOffset=<value optimized out>)
    at gfx/layers/opengl/ContainerLayerOGL.cpp:386
#10 0x3c474418 in ContainerRender<mozilla::layers::ContainerLayerOGL> (this=0x3b150c10, aPreviousFrameBuffer=0, aOffset=<value optimized out>)
    at gfx/layers/opengl/ContainerLayerOGL.cpp:246
#11 mozilla::layers::ContainerLayerOGL::RenderLayer (this=0x3b150c10, aPreviousFrameBuffer=0, aOffset=<value optimized out>)
    at gfx/layers/opengl/ContainerLayerOGL.cpp:339
#12 0x3c474418 in ContainerRender<mozilla::layers::ContainerLayerOGL> (this=0x3b14f1d0, aPreviousFrameBuffer=0, aOffset=<value optimized out>)
    at gfx/layers/opengl/ContainerLayerOGL.cpp:246
#13 mozilla::layers::ContainerLayerOGL::RenderLayer (this=0x3b14f1d0, aPreviousFrameBuffer=0, aOffset=<value optimized out>)
    at gfx/layers/opengl/ContainerLayerOGL.cpp:339
#14 0x3c47a978 in mozilla::layers::LayerManagerOGL::Render (this=0x43b0ee80) at gfx/layers/opengl/LayerManagerOGL.cpp:801
#15 0x3c47ad00 in mozilla::layers::LayerManagerOGL::EndTransaction (this=0x43b0ee80, aCallback=
    0x3b9eca48 <mozilla::FrameLayerBuilder::DrawThebesLayer(mozilla::layers::ThebesLayer*, gfxContext*, nsIntRegion const&, nsIntRegion const&, void*)>, aCallbackData=0xaea001d8, 
    aFlags=<value optimized out>) at gfx/layers/opengl/LayerManagerOGL.cpp:426
#16 0x3ba14258 in nsDisplayList::PaintForFrame (this=0xaea005a8, aBuilder=0xaea001d8, aCtx=<value optimized out>, aForFrame=0x4345c800, aFlags=2929724388)
    at layout/base/nsDisplayList.cpp:627
#17 0x3ba142d8 in nsDisplayList::PaintRoot (this=<value optimized out>, aBuilder=0x43b83a80, aCtx=<value optimized out>, aFlags=<value optimized out>)
    at layout/base/nsDisplayList.cpp:538
#18 0x3ba2b594 in nsLayoutUtils::PaintFrame (aRenderingContext=<value optimized out>, aFrame=0x4345c800, aDirtyRegion=<value optimized out>, aBackstop=<value optimized out>, aFlags=260)
    at layout/base/nsLayoutUtils.cpp:1697
**********************
(Reporter)

Comment 1

6 years ago
Created attachment 562674 [details] [diff] [review]
Stop using intermediate for special condition

Follow to roc suggestion about to not use intermediate for 3d transforms... and that seems to work, works smooth and fast...
Attachment #562674 - Flags: feedback?(roc)
Comment on attachment 562674 [details] [diff] [review]
Stop using intermediate for special condition

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

I don't think it can be quite this simple, but Matt should look at it...
Attachment #562674 - Flags: feedback?(roc) → review?(matt.woodrow)
(Reporter)

Updated

6 years ago
Blocks: 505115
Comment on attachment 562674 [details] [diff] [review]
Stop using intermediate for special condition

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

As Roc said, I believe this is wrong. If only because this still will not preserve 3D when there's group opacity I believe. But yeah, Matt's the better person to look at this.
(Assignee)

Comment 4

6 years ago
I don't think this will affect group opacity, this should already have it's own container layer (or the same container) and will still force a temporary surface to be allocated.

We don't need the checks for the PRESERVE_3D flag, this is just a hint for sorting. The way preserve-3d is handled, we have already multiplied the transforms together, so in terms of rendering, none of the containers want to preserve 3d as such.

This is the main reason we allocate a temporary surface, to project the 3d transform back into the screen plane.

We should be able to fix this instead by just modifying the matrix on these layers and clearing the Z components. Probably nsDisplayTransform::BuildLayer would be the best place for this?
(Assignee)

Comment 5

6 years ago
Created attachment 564449 [details] [diff] [review]
Stop using intermediate for special condition v2
Attachment #564449 - Flags: review?(roc)
(Reporter)

Comment 6

6 years ago
Comment on attachment 564449 [details] [diff] [review]
Stop using intermediate for special condition v2

> {
>   const gfx3DMatrix& newTransformMatrix = 
I believe this should not be const if you want to use newTransformMatrix.ProjectTo2D

>     GetTransform(mFrame->PresContext()->AppUnitsPerDevPixel());
>+  newTransformMatrix.ProjectTo2D();
>
Isn't this going to totally stuff up your layer-sorting patch?

Don't we need some D3D changes here too?

What happens if you just remove the intermediate-surface check? We'll render layers to the container buffer with a full 3D transform, but since we haven't enabled depth buffering, the Z will just get thrown away, won't it? What could possibly go wrong? :-)
(Assignee)

Comment 8

6 years ago
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #7)
> Isn't this going to totally stuff up your layer-sorting patch?

Well, that's awkward. Of all people, I probably should have spotted that :)

I think we can fix this by moving the projection into DefaultComputeEffectiveTransforms, and making sure sorting is using the Local transform instead of the Effective.

> 
> Don't we need some D3D changes here too?

There shouldn't be any explicit changes needed, D3D uses DefaultComputeEffectiveTransforms too? I might have missed an assertion though.

> 
> What happens if you just remove the intermediate-surface check? We'll render
> layers to the container buffer with a full 3D transform, but since we
> haven't enabled depth buffering, the Z will just get thrown away, won't it?
> What could possibly go wrong? :-)

The problem case is where we have nested 3d transforms that haven't (and shouldn't) be coalesced by the preserve-3d code in layout. Using the full matrix, and multiplying them together in DefaultComputeEffectiveTransforms will give us preserve-3d rendering, which is wrong.
(In reply to Matt Woodrow (:mattwoodrow) from comment #8)
> I think we can fix this by moving the projection into
> DefaultComputeEffectiveTransforms, and making sure sorting is using the
> Local transform instead of the Effective.

That should work.

> > Don't we need some D3D changes here too?
> 
> There shouldn't be any explicit changes needed, D3D uses
> DefaultComputeEffectiveTransforms too? I might have missed an assertion
> though.

Sorry, I hadn't noticed that your ContainerLayerOGL changes were cosmetic.

> > What happens if you just remove the intermediate-surface check? We'll render
> > layers to the container buffer with a full 3D transform, but since we
> > haven't enabled depth buffering, the Z will just get thrown away, won't it?
> > What could possibly go wrong? :-)
> 
> The problem case is where we have nested 3d transforms that haven't (and
> shouldn't) be coalesced by the preserve-3d code in layout. Using the full
> matrix, and multiplying them together in DefaultComputeEffectiveTransforms
> will give us preserve-3d rendering, which is wrong.

Right, I get it.
(Assignee)

Comment 10

6 years ago
Created attachment 565067 [details] [diff] [review]
Stop using intermediate layers for 3d transforms v3

Actually tested with sorting patches applied this time :)
Attachment #562674 - Attachment is obsolete: true
Attachment #564449 - Attachment is obsolete: true
Attachment #562674 - Flags: review?(matt.woodrow)
Attachment #564449 - Flags: review?(roc)
Attachment #565067 - Flags: review?(roc)
Attachment #565067 - Flags: review?(roc) → review+
Comment on attachment 565067 [details] [diff] [review]
Stop using intermediate layers for 3d transforms v3

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

Do we have a reftest for a 3D-transformed container with a 3D-transformed child without preserve-3d to ensure that flattening actually occurs correctly?
(Assignee)

Comment 12

6 years ago
Yes, we do. preserve3d-1b.html checks this.
(Assignee)

Comment 13

6 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/2465969e8c76
Whiteboard: [inbound]
https://hg.mozilla.org/mozilla-central/rev/2465969e8c76
Assignee: nobody → matt.woodrow
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla10

Updated

5 years ago
Depends on: 740789
You need to log in before you can comment on or make changes to this bug.