Last Comment Bug 689416 - CSS 3d Cube is very slow in OGL backend
: CSS 3d Cube is very slow in OGL backend
Status: RESOLVED FIXED
[inbound]
:
Product: Core
Classification: Components
Component: Graphics (show other bugs)
: Trunk
: x86 Linux
: -- normal (vote)
: mozilla10
Assigned To: Matt Woodrow (:mattwoodrow)
:
Mentors:
Depends on: 740789
Blocks: 505115
  Show dependency treegraph
 
Reported: 2011-09-26 18:25 PDT by Oleg Romashin (:romaxa)
Modified: 2016-06-03 02:36 PDT (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Test case (5.51 KB, text/html)
2011-09-26 18:25 PDT, Oleg Romashin (:romaxa)
no flags Details
Stop using intermediate for special condition (1.79 KB, patch)
2011-09-27 00:41 PDT, Oleg Romashin (:romaxa)
no flags Details | Diff | Splinter Review
Stop using intermediate for special condition v2 (5.50 KB, patch)
2011-10-03 20:35 PDT, Matt Woodrow (:mattwoodrow)
no flags Details | Diff | Splinter Review
Stop using intermediate layers for 3d transforms v3 (4.88 KB, patch)
2011-10-05 16:34 PDT, Matt Woodrow (:mattwoodrow)
roc: review+
Details | Diff | Splinter Review

Description Oleg Romashin (:romaxa) 2011-09-26 18:25:51 PDT
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
**********************
Comment 1 Oleg Romashin (:romaxa) 2011-09-27 00:41:12 PDT
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...
Comment 2 Robert O'Callahan (:roc) (email my personal email if necessary) 2011-09-27 01:59:32 PDT
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...
Comment 3 Bas Schouten (:bas.schouten) 2011-09-27 07:53:30 PDT
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.
Comment 4 Matt Woodrow (:mattwoodrow) 2011-10-01 14:22:55 PDT
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?
Comment 5 Matt Woodrow (:mattwoodrow) 2011-10-03 20:35:35 PDT
Created attachment 564449 [details] [diff] [review]
Stop using intermediate for special condition v2
Comment 6 Oleg Romashin (:romaxa) 2011-10-03 20:57:29 PDT
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();
>
Comment 7 Robert O'Callahan (:roc) (email my personal email if necessary) 2011-10-04 13:43:51 PDT
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? :-)
Comment 8 Matt Woodrow (:mattwoodrow) 2011-10-04 18:10:00 PDT
(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.
Comment 9 Robert O'Callahan (:roc) (email my personal email if necessary) 2011-10-04 18:37:29 PDT
(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.
Comment 10 Matt Woodrow (:mattwoodrow) 2011-10-05 16:34:11 PDT
Created attachment 565067 [details] [diff] [review]
Stop using intermediate layers for 3d transforms v3

Actually tested with sorting patches applied this time :)
Comment 11 Robert O'Callahan (:roc) (email my personal email if necessary) 2011-10-05 16:51:10 PDT
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?
Comment 12 Matt Woodrow (:mattwoodrow) 2011-10-05 17:24:00 PDT
Yes, we do. preserve3d-1b.html checks this.
Comment 13 Matt Woodrow (:mattwoodrow) 2011-10-06 14:30:40 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/2465969e8c76
Comment 14 Ed Morley [:emorley] 2011-10-07 04:08:32 PDT
https://hg.mozilla.org/mozilla-central/rev/2465969e8c76

Note You need to log in before you can comment on or make changes to this bug.