Bug 46208 - Implement --enable-accelerated-2d-canvas flag in DumpRenderTree
Summary: Implement --enable-accelerated-2d-canvas flag in DumpRenderTree
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Windows 7
: P2 Normal
Assignee: Stephen White
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-09-21 12:29 PDT by Stephen White
Modified: 2010-09-23 07:51 PDT (History)
5 users (show)

See Also:


Attachments
Patch (8.41 KB, patch)
2010-09-21 13:34 PDT, Stephen White
no flags Details | Formatted Diff | Diff
Patch (6.32 KB, patch)
2010-09-22 07:37 PDT, Stephen White
levin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Stephen White 2010-09-21 12:29:53 PDT
In order to run the layout tests both with and without accelerated 2D canvas, we should have a flag that controls its behaviour at runtime.
Comment 1 Stephen White 2010-09-21 13:34:51 PDT
Created attachment 68284 [details]
Patch
Comment 2 Dirk Pranke 2010-09-21 13:46:54 PDT
Comment on attachment 68284 [details]
Patch

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

Looks good to me other than the nits noted below.

I have a patch in progress that adds essentially the same line to port/chromium.py plus handles the Chromium DRT case and the new baseline search path we'll want. I should have it ready inside an hour or so (I would've finished it last night but I got derailed by another bug), so whichever of the two lands first can be coordinated with the other one.

> WebKitTools/Scripts/webkitpy/layout_tests/run_webkit_tests.py:577
> +        test_args.enable_accelerated_2d_canvas = self._options.enable_accelerated_2d_canvas

I don't think you need this.

> WebKitTools/Scripts/webkitpy/layout_tests/run_webkit_tests.py:1663
> +            dest="enable_accelerated_2d_canvas",

I don't think you need this, either.

> WebKitTools/Scripts/webkitpy/layout_tests/port/chromium.py:360
> +

Here you need to check if enable-accelerated-2d-canvas is not actually set (since there's no default value):

if hasattr(self._options, 'enable_accelerated_2d_canvas') and self._options.enable_accelerated_2d_canvas): ...
Comment 3 Dirk Pranke 2010-09-21 15:48:23 PDT
See also bug 46208 for my patch to new-run-webkit-tests for the full support of GPU testing.
Comment 4 David Levin 2010-09-21 16:28:18 PDT
Comment on attachment 68284 [details]
Patch

It looks like Dirk has some good points here. Other than those the patch looks fine.
Comment 5 Kenneth Russell 2010-09-21 16:30:03 PDT
(In reply to comment #3)
> See also bug 46208 for my patch to new-run-webkit-tests for the full support of GPU testing.

Wrong bug ID?
Comment 6 Kenneth Russell 2010-09-21 16:31:23 PDT
(In reply to comment #5)
> (In reply to comment #3)
> > See also bug 46208 for my patch to new-run-webkit-tests for the full support of GPU testing.
> 
> Wrong bug ID?

Looks like it should have been bug 46225.
Comment 7 Dirk Pranke 2010-09-21 16:37:48 PDT
(In reply to comment #6)
> (In reply to comment #5)
> > (In reply to comment #3)
> > > See also bug 46208 for my patch to new-run-webkit-tests for the full support of GPU testing.
> > 
> > Wrong bug ID?
> 
> Looks like it should have been bug 46225.

Right. Sorry!
Comment 8 Stephen White 2010-09-22 07:07:39 PDT
(In reply to comment #2)
> (From update of attachment 68284 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=68284&action=review
> 
> Looks good to me other than the nits noted below.
> 
> I have a patch in progress that adds essentially the same line to port/chromium.py plus handles the Chromium DRT case and the new baseline search path we'll want. I should have it ready inside an hour or so (I would've finished it last night but I got derailed by another bug), so whichever of the two lands first can be coordinated with the other one.

OK, your patch looks good to me, so I'm going to try your patch against the DRT changes in this patch, and if it all works I'll just revert all the python changes in this patch and resubmit.

> > WebKitTools/Scripts/webkitpy/layout_tests/run_webkit_tests.py:577
> > +        test_args.enable_accelerated_2d_canvas = self._options.enable_accelerated_2d_canvas
> 
> I don't think you need this.
>
> > WebKitTools/Scripts/webkitpy/layout_tests/run_webkit_tests.py:1663
> > +            dest="enable_accelerated_2d_canvas",
> 
> I don't think you need this, either.
> 
> > WebKitTools/Scripts/webkitpy/layout_tests/port/chromium.py:360
> > +
> 
> Here you need to check if enable-accelerated-2d-canvas is not actually set (since there's no default value):
> 
> if hasattr(self._options, 'enable_accelerated_2d_canvas') and self._options.enable_accelerated_2d_canvas): ...
Comment 9 Stephen White 2010-09-22 07:37:21 PDT
Created attachment 68370 [details]
Patch
Comment 10 Dirk Pranke 2010-09-22 14:55:27 PDT
Comment on attachment 68370 [details]
Patch

change looks good to me, but I'm not a reviewer.
Comment 11 Stephen White 2010-09-23 07:24:40 PDT
Committed r68136: <http://trac.webkit.org/changeset/68136>
Comment 12 WebKit Review Bot 2010-09-23 07:51:37 PDT
http://trac.webkit.org/changeset/68136 might have broken Chromium Linux Release
The following changes are on the blame list:
http://trac.webkit.org/changeset/68136
http://trac.webkit.org/changeset/68137
http://trac.webkit.org/changeset/68138