WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
114394
[GTK] [WebKit2] Add a setting to control whether or not accelerated 2D canvas is enabled
https://bugs.webkit.org/show_bug.cgi?id=114394
Summary
[GTK] [WebKit2] Add a setting to control whether or not accelerated 2D canvas...
Martin Robinson
Reported
2013-04-10 16:58:57 PDT
Accelerated 2D canvas controls whether or not 2D canvas is potentially drawn with hardware acceleration.
Attachments
Patch
(8.11 KB, patch)
2013-04-10 17:04 PDT
,
Martin Robinson
no flags
Details
Formatted Diff
Diff
Patch
(8.15 KB, patch)
2013-04-11 10:22 PDT
,
Martin Robinson
no flags
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Martin Robinson
Comment 1
2013-04-10 17:04:10 PDT
Created
attachment 197451
[details]
Patch
WebKit Commit Bot
Comment 2
2013-04-10 17:06:42 PDT
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 3
2013-04-10 23:40:05 PDT
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
Comment 4
2013-04-11 10:22:43 PDT
Created
attachment 197637
[details]
Patch
Martin Robinson
Comment 5
2013-04-11 10:25:23 PDT
(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 6
2013-04-16 09:15:31 PDT
Comment on
attachment 197637
[details]
Patch Looks good!
Xan Lopez
Comment 7
2013-04-16 09:18:12 PDT
Comment on
attachment 197637
[details]
Patch Removing r+, since this is WebKit2, but looks perfect to me.
Carlos Garcia Campos
Comment 8
2013-04-16 09:19:20 PDT
(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
Comment 9
2013-04-16 09:23:56 PDT
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
Comment 10
2013-04-16 09:43:19 PDT
(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 11
2013-05-01 11:10:02 PDT
Comment on
attachment 197637
[details]
Patch Clearing flags on attachment: 197637 Committed
r149440
: <
http://trac.webkit.org/changeset/149440
>
WebKit Commit Bot
Comment 12
2013-05-01 11:10:07 PDT
All reviewed patches have been landed. Closing bug.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug