Bug 114394 – [GTK] [WebKit2] Add a setting to control whether or not accelerated 2D canvas is enabled

• Component: WebKitGTK
• Status: RESOLVED FIXED

[Martin Robinson]

Accelerated 2D canvas controls whether or not 2D canvas is potentially drawn with hardware acceleration.

[Martin Robinson]

Created attachment 197451 [details]
Patch

[WebKit Commit Bot]

Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API

[Carlos Garcia Campos]

Comment on attachment 197451 [details]
Patch

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

Looks great, thanks! You need the approval of another GTK+ reviewer and a webkit2 owner. I also think we shouldn't add new API until we have trunk in sync with the stale branch, it would be weird to include API for 2.2 before the one already released in 2.0

> Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:1088
> +     * if WebKitGTK+ was compiled with a version of Cairo including the unstable CairoGL API.

What's the first version of cairo with a reasonable support of GL? Maybe it's worth mentioning a cairo version here.

> Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:1091
> +     */

Since: 2.2

> Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:1097
> +        g_param_spec_boolean("enable-accelerated-2d-canvas",
> +            _("Enable accelerated 2D canvas"),
> +            _("Whether to enable accelerated 2D canvas"),
> +            FALSE,
> +            readWriteConstructParamFlags));

I think we need to agree on a way to define properties and signals in a consistent way and making happy the style checker at the same time. I prefer to move the first argument to a new line to make sure all arguments are lined up, but I'm not opposed to this approach, we just need to decide.

> Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:2693
> + */

Since: 2.2

> Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:2707
> + */

Ditto.

[Martin Robinson]

Created attachment 197637 [details]
Patch

[Martin Robinson]

(In reply to comment #3)

> > Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:1088
> > +     * if WebKitGTK+ was compiled with a version of Cairo including the unstable CairoGL API.
> 
> What's the first version of cairo with a reasonable support of GL? Maybe it's worth mentioning a cairo version here.

There's no timetable for when CairoGL will be stable, so I cannot provide much more information sadly. One thing to consider is that in the future we may be able to accelerated canvas with XRender -- but I'm not entirely sure of the likelihood/usefulness of that either.

> 
> > Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:1091
> > +     */
> 
> Since: 2.2

Oh, right!

> 
> > Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:1097
> > +        g_param_spec_boolean("enable-accelerated-2d-canvas",
> > +            _("Enable accelerated 2D canvas"),
> > +            _("Whether to enable accelerated 2D canvas"),
> > +            FALSE,
> > +            readWriteConstructParamFlags));
> 
> I think we need to agree on a way to define properties and signals in a consistent way and making happy the style checker at the same time. I prefer to move the first argument to a new line to make sure all arguments are lined up, but I'm not opposed to this approach, we just need to decide.

I don't feel a personal necessity for this degree of consistency, but I certainly don't mind providing it. :)

[Xan Lopez]

Comment on attachment 197637 [details]
Patch

Looks good!

[Xan Lopez]

Comment on attachment 197637 [details]
Patch

Removing r+, since this is WebKit2, but looks perfect to me.

[Carlos Garcia Campos]

(In reply to comment #7)
> (From update of attachment 197637 [details])
> Removing r+, since this is WebKit2, but looks perfect to me.

I think we should also try to get trunk in sync with the stable branch before landing patches adding new api.

[Martin Robinson]

CCing some WebKit2 owners. Two GTK+ reviewers have approved this patch and it's deadly simple. It just connects a WebKit2 preference to the GTK+ API equivalent.

[Martin Robinson]

(In reply to comment #8)

> I think we should also try to get trunk in sync with the stable branch before landing patches adding new api.

I talked with cgarcia and he doesn't have have any direct objection to landing this patch now, but an aesthetic preference for landing pending patches earlier. The GTK+ port can work out the order of landing patches, so this is just waiting on owner review.

[WebKit Commit Bot]

Comment on attachment 197637 [details]
Patch

Clearing flags on attachment: 197637

Committed r149440: <http://trac.webkit.org/changeset/149440>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.