Bug 140667 – [WinCairo] Accelerated compositing should be disabled when graphics card does not support it.

• Status: REOPENED
• Component: WebKit Misc.

When the graphics card is not able to do accelerated compositing, we should set the relevant preference, so WebCore knows that acceleration is not possible.

Created attachment 244974 [details]
Patch

[Alex Christensen]

What happens when we try to use accelerated compositing when it's not available?  Does it not draw at all, or crash, or draw slowly?

(In reply to comment #2)
> What happens when we try to use accelerated compositing when it's not
> available?  Does it not draw at all, or crash, or draw slowly?

It does not draw at all. When disabling accelerated compositing it will draw, but slowly.

[Alex Christensen]

Comment on attachment 244974 [details]
Patch

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

I could see this making things a little harder to debug.  If we run into a problem in the future, we would also have to ask ourselves if it is a problem with the accelerated path or not.  Doesn't DirectX have a non-graphics-card implementation that ideally behaves the same, but slower?  Couldn't we use this through ANGLE instead of OpenGL?  If that's infeasible, then this is definitely the way to go.

> Source/WebKit/win/WebCoreSupport/AcceleratedCompositingContext.cpp:248
> +bool AcceleratedCompositingContext::acceleratedCompositingAvailable()

This and the declaration should probably also be inside #if USE(TEXTURE_MAPPER_GL)

> Source/WebKit/win/WebCoreSupport/AcceleratedCompositingContext.cpp:263
> +        DestroyWindow(testWindow);

::DestroyWindow here and elsewhere

Created attachment 246402 [details]
Patch

(In reply to comment #4)
> Comment on attachment 244974 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=244974&action=review
> 

Thanks for reviewing :)

> I could see this making things a little harder to debug.  If we run into a
> problem in the future, we would also have to ask ourselves if it is a
> problem with the accelerated path or not.  Doesn't DirectX have a
> non-graphics-card implementation that ideally behaves the same, but slower? 
> Couldn't we use this through ANGLE instead of OpenGL?  If that's infeasible,
> then this is definitely the way to go.
>

That might be possible, but I'm not sure :)
I think ANGLE examines the presence of some capabilities, and will not create a renderer if the requirements are not met.
 
> > Source/WebKit/win/WebCoreSupport/AcceleratedCompositingContext.cpp:248
> > +bool AcceleratedCompositingContext::acceleratedCompositingAvailable()
> 
> This and the declaration should probably also be inside #if
> USE(TEXTURE_MAPPER_GL)
> 

I think the whole file is inside #if USE(TEXTURE_MAPPER_GL)

[Alex Christensen]

Comment on attachment 246402 [details]
Patch

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

> Source/WebKit/win/WebCoreSupport/AcceleratedCompositingContext.cpp:254
> +    HWND testWindow = ::CreateWindowEx(WS_EX_NOACTIVATE, defWndProcWindowClassName(), L"AcceleratedCompositingTesterWindow", WS_POPUP | WS_VISIBLE | WS_DISABLED, -width, -height, width, height, 0, 0, 0, 0);

Is this visible?  Does it make a flash on the screen?  Does an invisible test work?

> Source/WebKit/win/WebCoreSupport/AcceleratedCompositingContext.cpp:307
> +    COLORREF pixelColor = ::GetPixel(hdc, 0, 0);

We should probably call ::SetPixel to red, swapBuffers with colorComponent as green, and then check to see if pixelColor is green.  It is unclear what the pixel would be if the test fails.

Created attachment 246413 [details]
Patch

(In reply to comment #7)
> Comment on attachment 246402 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=246402&action=review
> 

Thanks for the review :)

> > Source/WebKit/win/WebCoreSupport/AcceleratedCompositingContext.cpp:254
> > +    HWND testWindow = ::CreateWindowEx(WS_EX_NOACTIVATE, defWndProcWindowClassName(), L"AcceleratedCompositingTesterWindow", WS_POPUP | WS_VISIBLE | WS_DISABLED, -width, -height, width, height, 0, 0, 0, 0);
> 
> Is this visible?  Does it make a flash on the screen?  Does an invisible
> test work?
> 

It's not visible. I'm not sure how it would be with multiple monitors, though. I tested with an invisible window, but that didn't work.

[WebKit Commit Bot]

Comment on attachment 246413 [details]
Patch

Clearing flags on attachment: 246413

Committed r179962: <http://trac.webkit.org/changeset/179962>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Fujii Hironori]

Reverted r179962 in r248885 for Bug 200493 and Bug 200563. Reopened.

[Fujii Hironori]

Chrome has Software rendering list and Driver bug list.

https://chromium.googlesource.com/chromium/src/+/refs/heads/master/gpu/config/gpu_driver_bug_list.json
https://chromium.googlesource.com/chromium/src/+/refs/heads/master/gpu/config/software_rendering_list.json