[GTK] Should check whether GDK can use GL before asking it to

• Component: WebKitGTK
• Status: RESOLVED FIXED

[Gustavo Noronha (kov)]

[GTK] Should check whether GDK can use GL before asking it to

[Gustavo Noronha (kov)]

Created attachment 289924 [details]
Patch

[Michael Catanzaro]

Comment on attachment 289924 [details]
Patch

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

Thanks. OK with a GTK+ bug report on bugzilla.gnome.org.

> Source/WebKit2/UIProcess/gtk/AcceleratedBackingStoreWayland.cpp:79
> +        g_warning("GDK is not able to create a GL context, falling back to glReadPixels: %s", error->message);

I would tweak this slightly to indicate that glReadPixels is slow:

"falling back to glReadPixels (slow!): %s"

> Source/WebKit2/UIProcess/gtk/AcceleratedBackingStoreWayland.h:46
> +    bool canGdkUseGL();

I think this function should probably be static, since it doesn't need any member variables. Otherwise, it should be const.

[Carlos Garcia Campos]

Comment on attachment 289924 [details]
Patch

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

>> Source/WebKit2/UIProcess/gtk/AcceleratedBackingStoreWayland.h:46
>> +    bool canGdkUseGL();
> 
> I think this function should probably be static, since it doesn't need any member variables. Otherwise, it should be const.

And we avoid including gtk.h in the header.

[Carlos Garcia Campos]

(In reply to comment #2)
> Comment on attachment 289924 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=289924&action=review
> 
> Thanks. OK with a GTK+ bug report on bugzilla.gnome.org.
> 
> > Source/WebKit2/UIProcess/gtk/AcceleratedBackingStoreWayland.cpp:79
> > +        g_warning("GDK is not able to create a GL context, falling back to glReadPixels: %s", error->message);
> 
> I would tweak this slightly to indicate that glReadPixels is slow:
> 
> "falling back to glReadPixels (slow!): %s"

I would not, first because it's not accurate, it's fast in most desktops, but also because there's nothing the user can do, I'm afraid. Or what's the reason why GL can't be used? missing libs the user needs to install? or drivers?

> > Source/WebKit2/UIProcess/gtk/AcceleratedBackingStoreWayland.h:46
> > +    bool canGdkUseGL();
> 
> I think this function should probably be static, since it doesn't need any
> member variables. Otherwise, it should be const.

[Gustavo Noronha (kov)]

Committed r206424: <http://trac.webkit.org/changeset/206424>