485030 - RenderDocument builds nsDisplayList for each frame of flashplayer video.

Status: VERIFIED FIXED

(Core :: Layout, defect)

Description

[Oleg Romashin (:romaxa)]

Attachment: Plain profile for youtube video case

Flashplayer video rendering in windowless mode is very expensive, because for each Invalidate we are asking layout to draw mostly the same area... and Paint/RenderDocument implementation calculate display list from scratch for each frame.

I think it should be possible somehow optimize or cache this operation in such critical path as video rendering...

I have 2 ideas:
1) it is try to cache one or more display lists, and reuse them if RenderDocument/Paint event asking to repaint the same area again and again (flash video, gif animation)
  And drop this cache if we got some nsIFrame reflow/add/remove which is intersects with cached display list rectangle...
2) Try to group all frames by some regions, and do not touch in BuildDisplayList those frames which are not in requested region group/groups.

Robert, David do you have any ideas about this problem?

Comment 1

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I could certainly imagine something like caching the most recent display list (and invalidating that cache on reflow, frame tree change, or style-forced repaint -- since those can change which display items are present), although I worry that there are a bunch of other conditions that can change which display items are in the display list.  roc would know better, though.

Comment 2

[Oleg Romashin (:romaxa)]

I'm not sure, how ignore scroll frame flag can affect on this problem, because usually we are painting updates which are inside scroll port view, but I'm also rendering elements which are outside scroll port (to be able paint offscreen document), and I'm using this flag always as TRUE... 

Do we have any region optimizations already for paint requests which are inside scroll port?

Comment 3

[Oleg Romashin (:romaxa)]

I think it should be possible to create some certain implementation which will allow to set current rect for painting, and it will build "dirty display list", which will contain less elements than whole document, and will be used for RenderDocument requests with rects which are inside this "dirty disp list" region....

Comment 4

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Ignoring the scroll frame shouldn't make much difference if you're passing the right dirty rect for display list construction.

Can you set gDumpPaintList=1 in a debugger or in your (debug) build and then tell us what the before-and-after display list looks like?

Building display lists hardly ever shows up as a concern. I'm kinda surprised that it's significant when playing video. A hierarchical profile would be helpful here. If we can find a way to just speed up display list construction, that would be best.

Comment 5

[Oleg Romashin (:romaxa)]

Attachment: PaintFrame output for ahom.ru/rabota (normal mozilla painting)

Comment 6

[Oleg Romashin (:romaxa)]

Attachment: Draw almost the same area with RenderDocument function

Comment 7

[Oleg Romashin (:romaxa)]

I've tried to disable RENDER_IGNORE_VIEWPORT_SCROLLING option for RenderDocument, and I got the good output for DisplayList before optimization:
>>>>>>Func:RenderDocument::4984 dirtyR[80,105,640,298]
Painting --- before optimization (dirty 4800,6352,38400,17888):
Clip (nil)() (0,0,48000,21240)
    CanvasBackground 0x8a6dfb4() (0,0,48000,190312) opaque uniform
Clip (nil)() (0,0,48000,21240)
    Background 0x8a6e584() (480,1286,47040,188066) uniform
    Plugin 0x8a6e8f0() (4800,4972,38400,30000)
Painting --- after optimization:
Clip (nil)() (0,0,48000,21240)
    CanvasBackground 0x8a6dfb4() (0,0,48000,190312) opaque uniform
    Background 0x8a6e584() (480,1286,47040,188066) uniform
    Plugin 0x8a6e8f0() (4800,4972,38400,30000)

Comment 8

[Oleg Romashin (:romaxa)]

Sounds like Builder behave very different when ignore scroll frame available...

Comment 9

[Oleg Romashin (:romaxa)]

I think this bug fixed would help for general fennec painting, because ignorescrollframe option also used there...

Comment 10

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: fix?

I think I see the problem. Try this patch!

Comment 11

[Oleg Romashin (:romaxa)]

Comment on attachment 369406 [details] [diff] [review]
fix?

Excellent!, with this patch it works perfect....
At least on first check.
Thanks Robert.

Comment 12

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 369406 [details] [diff] [review]
fix?

This patch stops us restricting the dirty rect to the child's overflow-rect before we call BuildDisplayList on it. It simply isn't necessary and by removing that restriction, nsGfxScrollFrameInner::BuildDisplayList can use the passed-in dirty rect instead of having to guess the maximum-size dirty rect (which is causing this performance issue).

Comment 13

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 369406 [details] [diff] [review]
fix?

r=dbaron

Comment 14

[Robert O'Callahan (:roc) (email my personal email if necessary)]

http://hg.mozilla.org/mozilla-central/rev/34873fe83876

Comment 15

[Zack Weinberg (:zwol)]

has nothing to do with bug 486065 akshlly.

Comment 16

[Zack Weinberg (:zwol)]

sigh, apparently bugzilla does *not* lie when it says the dependency information will be overwritten.

Comment 17

[Zack Weinberg (:zwol)]

If this ever gets pushed to the 1.9.1 branch it will trigger bug 486065 there.  (I don't know what I was thinking when I said this has nothing to do with that.)