Bug 46208 – Implement --enable-accelerated-2d-canvas flag in DumpRenderTree

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Stephen White]

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.

[Stephen White]

Created attachment 68284 [details]
Patch

[Dirk Pranke]

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): ...

[Dirk Pranke]

See also bug 46208 for my patch to new-run-webkit-tests for the full support of GPU testing.

[David Levin]

Comment on attachment 68284 [details]
Patch

It looks like Dirk has some good points here. Other than those the patch looks fine.

[Kenneth Russell]

(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?

[Kenneth Russell]

(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.

[Dirk Pranke]

(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!

[Stephen White]

(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): ...

[Stephen White]

Created attachment 68370 [details]
Patch

[Dirk Pranke]

Comment on attachment 68370 [details]
Patch

change looks good to me, but I'm not a reviewer.

[Stephen White]

Committed r68136: <http://trac.webkit.org/changeset/68136>

[WebKit Review Bot]

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