Bug 75062 – [chromium] Try to create graphics context thrice before giving up on acceleratedCompositing

• Status: RESOLVED
• Resolution: WONTFIX
• Component: New Bugs

[Nat Duca]

[chromium] Try to create graphics context twice before giving up on acceleratedCompositing

[Nat Duca]

Created attachment 120264 [details]
Patch

[James Robinson]

Comment on attachment 120264 [details]
Patch

so we're relying on the style recalc timer to retry context creation?

should we just always be creating the compositor context off a timer? IOW in setRootLayer() just store the layer and set a timer, then when the timer fires go ahead and attempt to initialize the CCLayerTreeHost. if that fails, then we have to set accelerated compositing to false and force a style recalc to drop all the RenderLayerBackings

[James Robinson]

On second thought that might add too much latency in the normal case. Hmmmm

[Nat Duca]

(In reply to comment #3)
> On second thought that might add too much latency in the normal case. Hmmmm

OOh, that is a neat idea... lemme think.....

Suppose for a minute we had DoDeferredUpdate inside WebViewImpl (which we're always threatening to do). At that point, you'd have a central "DoDeferredUpdate" that would vector off to accelerated or software paint mode. So, when you got this, you'd just make sure a ddu was scheduled. And you'd stash a note saying "hey, this is the goal state for accelerated compositing."

Then, in DDU, the first thing you'd do is switch fully over to the other mode....

[Nat Duca]

Created attachment 121940 [details]
Patch

[James Robinson]

Comment on attachment 121940 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=121940&action=review

> Source/WebKit/chromium/src/WebViewImpl.cpp:2996
> +            // If the context creaiton failed, there is a very good chance that

typo creation

> Source/WebKit/chromium/src/WebViewImpl.cpp:3003
> +            m_page->mainFrame()->document()->scheduleForcedStyleRecalc();

this will force every element in the document to get styles re-resolved, regardless of whether they were dirty. do we need to go this heavy?

it'll also work on the main frame's document and not any subdocuments, so if there's a compositing trigger in an iframe this won't hit it - will it still trigger the compositor? easy (hah) way to test would be window.open() a page that has an iframe with a 3d-transformed element inside the iframe

> Source/WebKit/chromium/src/WebViewImpl.h:659
> +        kMaxCompositorCreationAttempts = 2

i don't think we normally use the k prefix for constants in WebKit.

can you put this in the .cpp? it's not API

> Source/WebKit/chromium/src/WebViewImpl.h:661
> +    bool m_numCompositorCreationAttempts;

bool number wut

[Nat Duca]

Created attachment 122054 [details]
Patch

[Nat Duca]

(In reply to comment #6)
> > Source/WebKit/chromium/src/WebViewImpl.cpp:3003
> > +            m_page->mainFrame()->document()->scheduleForcedStyleRecalc();
> 
> this will force every element in the document to get styles re-resolved, regardless of whether they were dirty. do we need to go this heavy?
> 
> it'll also work on the main frame's document and not any subdocuments, so if there's a compositing trigger in an iframe this won't hit it - will it still trigger the compositor? easy (hah) way to test would be window.open() a page that has an iframe with a 3d-transformed element inside the iframe

Not sure what you had in mind instead of scheduleForced... This is what we've typically used for forcing ourselves into.

Checked the window.open case with the following:
http://www.corp.google.com/~nduca/test/window_open_iframe_with_css.html

Seems to work... might be good to learn from you "why" :)

[Nat Duca]

(In reply to comment #8)
@jamesr: ping?

[James Robinson]

(In reply to comment #8)
> (In reply to comment #6)
> > > Source/WebKit/chromium/src/WebViewImpl.cpp:3003
> > > +            m_page->mainFrame()->document()->scheduleForcedStyleRecalc();
> > 
> > this will force every element in the document to get styles re-resolved, regardless of whether they were dirty. do we need to go this heavy?
> > 
> > it'll also work on the main frame's document and not any subdocuments, so if there's a compositing trigger in an iframe this won't hit it - will it still trigger the compositor? easy (hah) way to test would be window.open() a page that has an iframe with a 3d-transformed element inside the iframe
> 
> Not sure what you had in mind instead of scheduleForced... This is what we've typically used for forcing ourselves into.
> 
> Checked the window.open case with the following:
> http://www.corp.google.com/~nduca/test/window_open_iframe_with_css.html
> 
> Seems to work... might be good to learn from you "why" :)

That testcase seems to work without any local patches. Looking in more detail...

[Nat Duca]

(In reply to comment #10)
> 
> That testcase seems to work without any local patches. Looking in more detail...

F! My bad! I can look if you've got other stuff to do...

[James Robinson]

Completely different approach here: https://chromiumcodereview.appspot.com/9141035/

If you don't mind let's take discussion to http://code.google.com/p/chromium/issues/detail?id=106815 for a bit and see if we can get consensus there.

[James Robinson]

For anyone curious, the way to repro this problem is to pass --force-compositing-mode. The window.open()'d window doesn't get compositing when it should.

[James Robinson]

Comment on attachment 122054 [details]
Patch

I have a chrome-side patch underway to make the compositor initialization succeed the first time, see crbug.com/106815. While that's not done I would prefer to pursue that avenue rather than adding more retry stuff here.

[Nat Duca]

For the record, I agree wholeheartedly. This patch was designed to be a fallback position in case we didn't have time to pursue a real fix.