Bug 52333 – Add support to DumpRenderTree to use the GPU rather than software renderer

• Component: New Bugs
• Status: RESOLVED FIXED

[Vincent Scheib]

Add support to DumpRenderTree to use the GPU rather than software renderer

[Vincent Scheib]

Created attachment 78754 [details]
Patch

[WebKit Review Bot]

Attachment 78754 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/7571001

[Vincent Scheib]

Initial review for work in progress, The chromium changes will need to land before the webkit changes.
http://codereview.chromium.org/6258001
https://bugs.webkit.org/show_bug.cgi?id=52333

[Vincent Scheib]

Dirk, could you check on the simple switch additions to Tools/Scripts/webkitpy/layout_tests/...

[Dirk Pranke]

Comment on attachment 78754 [details]
Patch

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

The change looks fine. Semi-unrelated note: will we be able to remove the --accelerated-* flags (to new-run-webkit-tests, not to DRT/TestShell) at some point soon because those code paths should always be on?

> Tools/Scripts/webkitpy/layout_tests/port/chromium.py:396
> +                cmd.append('--test-on-GPU-hardware')

Typically we wouldn't use mixed-case for this. I might call the switch something shorter like --use-hardware-gpu .

> Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py:249
> +            help="Run OpenGL commands on real GPU hardware vs software"),

For now, this way of declaring things is fine. 

If we do something like what I discussed earlier, where default should vary depending on whether you use --platform chromium-win or --platform chromium-gpu-win, then you would want a default=None (not setting default= as an argument has the same effect) and then test for None explicitly in the Port code to see if the flag was set on the command line or if we need to specify the default. This is a result of the unfortunate fact of argument parsing happening before we get to execute any Port code (which I'll fix at some point, but not now).

[Dirk Pranke]

Mihai, you want to take a look at / review this?

[Mihai Parparita]

Comment on attachment 78754 [details]
Patch

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

> Tools/DumpRenderTree/chromium/DumpRenderTree.cpp:54
> +static const char optionHardwareAcceleratedGL[] = "--test-on-GPU-hardware";

I agree with Dirk, the flag name should be all lowercase.

[Vincent Scheib]

Created attachment 78818 [details]
Patch

[Vincent Scheib]

Command line switch changed to --enable-hardware-gpu

[WebKit Review Bot]

Attachment 78818 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/7567011

[Kenneth Russell]

Comment on attachment 78818 [details]
Patch

The code changes look good. You need to roll forward the Chromium revision in WebKit/chromium/DEPS to the revision where you checked in the new webkit_support API in order for this to compile on the webkit.org Chromium builders.

[Vincent Scheib]

Created attachment 78834 [details]
Patch

[Mihai Parparita]

Comment on attachment 78834 [details]
Patch

Please wait for the cr-linux EWS bot to process this patch before landing.

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

> WebKit/chromium/ChangeLog:8
> +        * WebKit.gyp: DumpRenderTree depends on Angle on windows

Should update the ChangeLog to include the DEPS file.

[Vincent Scheib]

Created attachment 78837 [details]
Patch

[WebKit Review Bot]

Attachment 78837 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/7579020

[Mihai Parparita]

(In reply to comment #15)
> Attachment 78837 [details] did not build on chromium:
> Build output: http://queues.webkit.org/results/7579020

Not sure I understand why this is happening:

In file included from WebKit/chromium/webkit/tools/test_shell/mock_webclipboard_impl.cc:5:
WebKit/chromium/webkit/tools/test_shell/mock_webclipboard_impl.h:13: fatal error: third_party/WebKit/WebKit/chromium/public/WebClipboard.h: No such file or directory

If I roll to 71332 locally, things build fine. Perhaps the EWS bot is sick (if it's missing a file)?

[Kent Tamura]

(In reply to comment #16)
> (In reply to comment #15)
> > Attachment 78837 [details] [details] did not build on chromium:
> > Build output: http://queues.webkit.org/results/7579020
> 
> Not sure I understand why this is happening:
> 
> In file included from WebKit/chromium/webkit/tools/test_shell/mock_webclipboard_impl.cc:5:
> WebKit/chromium/webkit/tools/test_shell/mock_webclipboard_impl.h:13: fatal error: third_party/WebKit/WebKit/chromium/public/WebClipboard.h: No such file or directory

It looks my failure.
webkit_support_common library should depend on setup_third_party.gyp:third_party_headers.
I'll check it and fix.

[Kent Tamura]

> webkit_support_common library should depend on setup_third_party.gyp:third_party_headers.
> I'll check it and fix.

Fixed.  Please use Chromium r71405 or later.

[Vincent Scheib]

Created attachment 78947 [details]
Patch

[Vincent Scheib]

Chromium revision bumped to 71405

[WebKit Commit Bot]

Comment on attachment 78947 [details]
Patch

Clearing flags on attachment: 78947

Committed r75800: <http://trac.webkit.org/changeset/75800>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.