Last Comment Bug 808838 - 3D CSS Periodic table demo is unusably slow in Firefox
: 3D CSS Periodic table demo is unusably slow in Firefox
Status: NEW
[sps][Snappy:p2]
:
Product: Core
Classification: Components
Component: Layout (show other bugs)
: unspecified
: x86 Windows 7
: -- normal with 4 votes (vote)
: ---
Assigned To: Nobody; OK to take it and work on it
:
Mentors:
Depends on:
Blocks: 806256
  Show dependency treegraph
 
Reported: 2012-11-05 16:14 PST by (dormant account)
Modified: 2012-11-29 07:24 PST (History)
28 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Force 3d transformed layers to be active (7.31 KB, patch)
2012-11-05 17:50 PST, Matt Woodrow (:mattwoodrow)
no flags Details | Diff | Review

Description (dormant account) 2012-11-05 16:14:36 PST
+++ This bug was initially created as a clone of Bug #806256 +++

STR:
http://mrdoob.com/lab/javascript/threejs/css3d/periodictable/

This is still unusable with D2D.

D2D profile:
http://people.mozilla.com/~bgirard/cleopatra/#report=737b3cf85835b2f648bb9798612038db3a9090be
Comment 1 Bas Schouten (:bas.schouten) 2012-11-05 16:18:52 PST
So this is -not- fixed with Direct2D.

And the problem is rather interesting, this site appears to be triggering a DrawWindow, which tries to render with BasicLayers to a D2D surface. That would be fine, except that it's doing 3D transforms, which BasicLayers does by doing a readback, if we were drawing in software, that would be fine, but when drawing with Direct2D it means loads and loads of readbacks.

I have no idea why this is doing so many DrawWindow calls though, what's triggering it? Roc or Matt, can you tell from the page?
Comment 2 Bas Schouten (:bas.schouten) 2012-11-05 16:30:10 PST
Something went wrong rebuilding, so I'm still seeing a lot of time in the DrawWindow, but it's not necessarily the cause. Doing a clean rebuild now to test.
Comment 4 Matt Woodrow (:mattwoodrow) 2012-11-05 16:42:27 PST
BasicLayers will be slow because we flatten the layers in order to get component alpha.

I doubt that does anything useful for 3d transformed layers though.
Comment 5 Oleg Romashin (:romaxa) 2012-11-05 17:43:25 PST
I've noticed that this demo on Android (HW accelerated) still about 2x slower comparing to any webkit browser, like chrome... 
it feels like we don't operate with pure OGL/D3D layers transformation during that animation and still doing some software stuff, which I guess we should not.

One profile is showing some PaintBoxShadowOuter for every rendered frame...
Comment 6 Matt Woodrow (:mattwoodrow) 2012-11-05 17:50:25 PST
Created attachment 678571 [details] [diff] [review]
Force 3d transformed layers to be active

This forces us to always have an active layer for 3d transformed content.

Unfortunately it makes ChildrenCanBeInactive quite a bit more expensive since we always have to walk all descendants.

Significant performance improvement on this demo with BasicLayers though.
Comment 7 Matt Woodrow (:mattwoodrow) 2012-11-05 18:31:37 PST
Note that we need to bubble the LAYER_ACTIVE_FORCE flag up because there is an outer transform wrapping the 3d transformed elements. We end up flattening both.

We could instead transform the 'forced active' flag onto the layers we create, and only flatten if there are layers that have component alpha, but don't have the forced flag set.
Comment 8 Bas Schouten (:bas.schouten) 2012-11-05 18:54:05 PST
So, I've now got a proper build going. There's very little graphics work in the profile, the relevant work is in AlphaBoxBlur, my patches in bug 509052 take care of that mostly. At the top of the profile then I find FrameLayerBuilder::ProcessRemovedDisplayItem, and a bunch of hashtable stuff. Time there seems to be mostly spent in memory access on a quick look, here's the bottom-up list of top contributions:

Function / Call Stack	CPU Time	Module	Function (Full)
mozilla::FrameLayerBuilder::ProcessRemovedDisplayItems	516.888ms	xul.dll	mozilla::FrameLayerBuilder::ProcessRemovedDisplayItems(class nsRefPtrHashKey<class mozilla::FrameLayerBuilder::DisplayItemData> *,void *)
malloc	297.916ms	MSVCR110.dll	malloc
PL_DHashTableEnumerate	273.049ms	xul.dll	PL_DHashTableEnumerate
PeekMessageW	218.629ms	USER32.dll	PeekMessageW
nsRuleNode::GetStyleDisplay	199.230ms	xul.dll	nsRuleNode::GetStyleDisplay(class nsStyleContext *,bool)
[d3d11.dll]	196.612ms	d3d11.dll	[d3d11.dll]
mozilla::gfx::AlphaBoxBlur::BoxBlur_SSE2	168.010ms	gkmedias.dll	mozilla::gfx::AlphaBoxBlur::BoxBlur_SSE2(int,int,int,int,unsigned int *,unsigned int)
SearchTable	156.743ms	xul.dll	SearchTable
nsTHashtable<struct mozilla::plugins::DeletingObjectEntry>::s_EnumStub	148.190ms	xul.dll	nsTHashtable<struct mozilla::plugins::DeletingObjectEntry>::s_EnumStub(struct PLDHashTable *,struct PLDHashEntryHdr *,unsigned int,void *)
Comment 9 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2012-11-05 23:59:23 PST
Comment on attachment 678571 [details] [diff] [review]
Force 3d transformed layers to be active

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

It's not clear from the bug comments so far what the actual problem is here. I'd like to understand that before we take a step like this.
Comment 10 Matt Woodrow (:mattwoodrow) 2012-11-06 13:44:36 PST
It's not clear what the reported problem is, but my patch fixes a real issue with BasicLayers.

The underlying issue is that we flatten layers that have component alpha, even if flattening them won't actually fix the issue.

With a 3d transform in BasicLayers, we're always drawing into a temporary surface (and then transforming it using pixman), so we don't get subpixel AA text regardless. Having us trigger component alpha flattening and forcing us to redraw the contents of every layer, every paint is undesirable.
Comment 11 David Rajchenbach-Teller [:Yoric] (please use "needinfo") 2012-11-09 01:06:04 PST
I may be pointing out at the obvious, but there seems to be the exact same issue on MacOS X, so this is most likely not DirectX-specific.
Comment 12 Romain DEP. 2012-11-24 10:27:44 PST
Maybe this is linux (with intel GPU) specific, but I'm pretty sure that since this commit I have:

- an impressive FPS boost on http://bartaz.github.com/impress.js/ (that's the good news :) )
- but also a lot of flickering too. Some elements even disappears once the transition is complete (@see for example slide 4, the String «then you should try» is displayed when switching from slide 3 to 4, but turns invisible once «the camera» isn't moving anymore).
I just tried with a fresh profile and it's the same. Playing with xrender or azure parameters doesn't seems to help there. Can anyone reproduce ?
Thanks.
Comment 13 Josh Matthews [:jdm] 2012-11-25 15:26:08 PST
I can reproduce the problems from comment 12 on OS X 10.6.8 in the latest nightly.
Comment 14 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2012-11-25 15:28:55 PST
What commit? Nothing has been committed here yet.
Comment 15 Romain DEP. 2012-11-25 15:45:49 PST
Something actually changed.
https://bugzilla.mozilla.org/show_bug.cgi?id=806256
Comment 16 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2012-11-25 18:19:19 PST
(In reply to Romain DEP. from comment #12)
> - but also a lot of flickering too. Some elements even disappears once the
> transition is complete (@see for example slide 4, the String «then you
> should try» is displayed when switching from slide 3 to 4, but turns
> invisible once «the camera» isn't moving anymore).

Probably a new bug should be filed for this.
Comment 17 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2012-11-25 18:20:40 PST
(In reply to Matt Woodrow (:mattwoodrow) from comment #10)
> The underlying issue is that we flatten layers that have component alpha,
> even if flattening them won't actually fix the issue.
> 
> With a 3d transform in BasicLayers, we're always drawing into a temporary
> surface (and then transforming it using pixman), so we don't get subpixel AA
> text regardless. Having us trigger component alpha flattening and forcing us
> to redraw the contents of every layer, every paint is undesirable.

I remember discussing this with you, but I don't remember where that discussion was written down or what the ultimate result was.

ChooseScaleAndSetTransform currently does:
  if (isRetained && (!canDraw2D || transform2d.HasNonIntegerTranslation())) {
    result.mDisableSubpixelAntialiasingInDescendants = true;
  }
Seems to me that if !canDraw2D we should be setting mDisableSubpixelAntialiasingInDescendants to true even if !isRetained. If we did that, then we shouldn't have to flatten in this case.

I remember you had a response to that, but I don't remember what it was :-).
Comment 18 Josh Matthews [:jdm] 2012-11-25 21:45:20 PST
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #16)
> (In reply to Romain DEP. from comment #12)
> > - but also a lot of flickering too. Some elements even disappears once the
> > transition is complete (@see for example slide 4, the String «then you
> > should try» is displayed when switching from slide 3 to 4, but turns
> > invisible once «the camera» isn't moving anymore).
> 
> Probably a new bug should be filed for this.

Filed bug 815040.
Comment 19 Matt Woodrow (:mattwoodrow) 2012-11-28 13:42:49 PST
> 
> I remember discussing this with you, but I don't remember where that
> discussion was written down or what the ultimate result was.
> 
> ChooseScaleAndSetTransform currently does:
>   if (isRetained && (!canDraw2D || transform2d.HasNonIntegerTranslation())) {
>     result.mDisableSubpixelAntialiasingInDescendants = true;
>   }
> Seems to me that if !canDraw2D we should be setting
> mDisableSubpixelAntialiasingInDescendants to true even if !isRetained. If we
> did that, then we shouldn't have to flatten in this case.
> 
> I remember you had a response to that, but I don't remember what it was :-).

I don't remember what we decided either.

An equivalent to the patch in this bug has already landed though, so this shouldn't be much of a problem any more.

That said, your change looks valid too.
Comment 20 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2012-11-29 03:36:36 PST
So this bug is fixed now?
Comment 21 Bas Schouten (:bas.schouten) 2012-11-29 07:24:32 PST
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #20)
> So this bug is fixed now?

Not really, on any non-super-fast CPU this demo is still fairly slow. And the vast majority of the time is spent in FrameLayerBuilder::ProcessRemovedDisplayItems. Which, from a quick look at the profile, seems to partially just be very cache unfriendly.

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