The accelerated drawing path in not ready for prime time. It has quite a few correctness and performance issues. So we cannot enable it for webkit_gpu_tests yet. While I am working on resolving the known issues, we need to add at least a single test for accelerated drawing to prevent it from breaking. I am planning on following the pattern for --force-compositing-mode and add setAcceleratedDrawing() to windows.internal.
Created attachment 112365 [details] proposed patch
Comment on attachment 112365 [details] proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=112365&action=review > LayoutTests/platform/chromium/compositing/accelerated-drawing/alpha-expected.txt:1 > +layer at (0,0) size 800x600 why would you want a render tree dump for this test?
Comment on attachment 112365 [details] proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=112365&action=review >> LayoutTests/platform/chromium/compositing/accelerated-drawing/alpha-expected.txt:1 >> +layer at (0,0) size 800x600 > > why would you want a render tree dump for this test? It is not required. I thought all tests have render-tree. How can I disable the test from checking the render-tree?
layoutTestController.dumpAsText(true). Read http://trac.webkit.org/wiki/Writing%20Layout%20Tests%20for%20DumpRenderTree please
Created attachment 112377 [details] proposed patch Removed render-tree.
Bikeshed: Is there a point where we can rename accelerated drawing => accelerated painting to match the WebKit paint vs. compositor draw language used elsewhere?
(In reply to comment #6) > Bikeshed: Is there a point where we can rename accelerated drawing => accelerated painting to match the WebKit paint vs. compositor draw language used elsewhere? We have had this discussion a few times before. WebKit has had WebCore::Settings::setAcceleratedDrawingEnabled() for a long time. When I started working on it, I named everything "drawing" to avoid confusion. I agree that "painting" is a better word. I could rename all instances in a different patch?
The mac port uses the accelerated drawing terminology so some cross-platform logic will use it. We shouldn't use it in our internal code, however.
(In reply to comment #8) > The mac port uses the accelerated drawing terminology so some cross-platform logic will use it. We shouldn't use it in our internal code, however. Thanks for rehashing this argument for me. I missed the point where the mac port was using that terminology.
Comment on attachment 112377 [details] proposed patch Attachment 112377 [details] did not pass gtk-ews (gtk): Output: http://queues.webkit.org/results/10212519
Comment on attachment 112377 [details] proposed patch Looks like the trybots have cycled, so please add the appropriate exports to make this build everywhere. I'm fairly surprised this compiled unmodified on mac.
Created attachment 112755 [details] proposed patch Moved the implementation of WebCore::Settings::setAcceleratedDrawingEnabled() to the header file to fix linking issues.
(In reply to comment #11) > (From update of attachment 112377 [details]) > Looks like the trybots have cycled, so please add the appropriate exports to make this build everywhere. I'm fairly surprised this compiled unmodified on mac. It compiled on mac because __ZN7WebCore8Settings28setAcceleratedDrawingEnabledEb is defined in Source/WebCore/WebCore.exp.in.
(In reply to comment #12) > Created an attachment (id=112755) [details] > proposed patch > > Moved the implementation of WebCore::Settings::setAcceleratedDrawingEnabled() to the header file to fix linking issues. Moving the implementation to header file now breaks the mac port because of the exported symbol in WebCore.exp.in. I see two options to fix the build on all bots: 1. Delete the exported symbol in WebCore.exp.in, or 2. Revert my changes in Settings, i.e. move the implementation back to Settings.cpp. Add the symbol in export files for gtk and win port. Which option is better?
Created attachment 112913 [details] proposed patch Removed __ZN7WebCore8Settings28setAcceleratedDrawingEnabledEb from WebCore.exp.in. Review not needed at this time. Just checking if it makes the mac bot green.
Comment on attachment 112913 [details] proposed patch Seems to pass EWS, so r=me
Alok et al, could you make sure to CC me on any current and future patches that involve accelerated painting/drawing? I'm not sure what others feel, but I think it'd be nice to have consistency here with respect to the exported symbols and locations of implementations. As in, I'd rather just have all the simple getter/setters in one place or the other. If there's a desire to change the name of the flag and has good reason, then let's decide what to rename it to and change it everywhere to have it be consistent. Why is this test platform specific?
(In reply to comment #17) > Alok et al, could you make sure to CC me on any current and future patches that involve accelerated painting/drawing? Will do. Although most of my patches are chromium specific. > I'm not sure what others feel, but I think it'd be nice to have consistency here with respect to the exported symbols and locations of implementations. As in, I'd rather just have all the simple getter/setters in one place or the other. I agree. I dont know why all the simple setters are implemented in the source file. Moving them to the header file will make it easier to access them from DRT. > If there's a desire to change the name of the flag and has good reason, then let's decide what to rename it to and change it everywhere to have it be consistent. We feel that "painting" is better than "drawing". > Why is this test platform specific? This is a temporary test while we fix obvious issues with accelerated-drawing path in chromium. We should not be testing every graphics port as part of layout tests. As soon as the accelerated drawing path is ready, I plan to delete this test and update all image baselines.
Comment on attachment 112913 [details] proposed patch Clearing flags on attachment: 112913 Committed r98990: <http://trac.webkit.org/changeset/98990>
All reviewed patches have been landed. Closing bug.