Summary: | [WinCairo] Accelerated compositing should be disabled when graphics card does not support it. | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | peavo | ||||||||
Component: | WebKit Misc. | Assignee: | Nobody <webkit-unassigned> | ||||||||
Status: | REOPENED --- | ||||||||||
Severity: | Normal | CC: | achristensen, alex.christensen, bfulgham, commit-queue, Hironori.Fujii | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | PC | ||||||||||
OS: | Unspecified | ||||||||||
See Also: | https://bugs.webkit.org/show_bug.cgi?id=200493 | ||||||||||
Attachments: |
|
Description
peavo
2015-01-20 00:40:31 PST
Created attachment 244974 [details]
Patch
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. 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) 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. Comment on attachment 246413 [details] Patch Clearing flags on attachment: 246413 Committed r179962: <http://trac.webkit.org/changeset/179962> All reviewed patches have been landed. Closing bug. Reverted r179962 in r248885 for Bug 200493 and Bug 200563. Reopened. 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 |