Hi, WebCore supports css-shaders. But currently, gtk build system does not support building it.
Created attachment 161758 [details] patch proposal With this patch, it's possible to build gtk port with css shaders: build-webkit --gtk --css-shaders --css-filters For the settings, I've taken example on the qt port, and enable css shaders when webgl is enabled
Comment on attachment 161758 [details] patch proposal View in context: https://bugs.webkit.org/attachment.cgi?id=161758&action=review I think that Zan should double-check this patch to see if it fits well with the work that he is doing to handle eliminating configuration arguments and make the feature list autogenerated. > Source/WebKit/gtk/webkit/webkitwebview.cpp:3436 > +#if ENABLE(CSS_SHADERS) > + // For now, enable CSS shaders when WebGL is enabled. > + coreSettings->setCSSCustomFilterEnabled(settingsPrivate->enableWebgl); > +#endif I think this doesn't necessarily make sense because: 1. CSS shaders are still an experimental features, so I think that people would probably like a way to turn on WebGL (which isn't experimental) without enabling CSS shaders. 2. CSS shaders really depend on the TextureMapperGL backend at the moment, not WebGL. > configure.ac:1174 > +# check whether to enable CSS Shaders support > +AC_MSG_CHECKING([whether to enable CSS Shaders]) > +AC_ARG_ENABLE(css_shaders, > + AC_HELP_STRING([--enable-css-shaders], > + [enable support for CSS Shaders [default=no]]), > + [],[enable_css_shaders="no"]) > +AC_MSG_RESULT([$enable_css_shaders]) > + I think what we want to do here instead is to enable them when experimental features are enabled and otherwise not. I'd like to have them turned on eventually, but probably behind WebKitWebSettings::enable-css-shaders. So instead of a configure option why not just enable them when experimental features are on and accelerated compositing is enabled.
CCing Zan as he's currently the person who knows the most about the direction the configuration options are moving.
Comment on attachment 161758 [details] patch proposal View in context: https://bugs.webkit.org/attachment.cgi?id=161758&action=review > Source/WebCore/GNUmakefile.am:517 > +feature_defines_overrides += ENABLE_CSS_SHADERS=1 You'll also need to add ENABLE_CSS_SHADERS entry to Source/WebCore/GNUmakefile.features.am. It's value depends on whether it should be enabled in developer builds or not. If yes, you'll also need to override the feature define with ENABLE_CSS_SHADERS=0 if unstable features are not to be enabled. That's done here: http://trac.webkit.org/browser/trunk/Source/WebCore/GNUmakefile.am#L771 There's a bug here though, duplicate entries can pop up in feature_defines and webcore_cppflags variables. I'll post a fix for that soon. Also, if the configuration flag in configure.ac is in the end not used, this overriding depending on the ENABLE_CSS_SHADERS conditional is useless and should be removed. >> configure.ac:1174 >> +AC_MSG_RESULT([$enable_css_shaders]) >> + > > I think what we want to do here instead is to enable them when experimental features are enabled and otherwise not. I'd like to have them turned on eventually, but probably behind WebKitWebSettings::enable-css-shaders. > > So instead of a configure option why not just enable them when experimental features are on and accelerated compositing is enabled. If I'm understanding things correctly, CSS Shaders rely on TextureMapperGL (i.e. the acceleration backend being OpenGL). In that regard it is the same as WebGL, which currently gets enabled by default if the acceleration backend is OpenGL and throws an error if not. Perhaps CSS Shaders should do the same, meaning they should be guarded by a configuration flag?
(In reply to comment #4) > If I'm understanding things correctly, CSS Shaders rely on TextureMapperGL (i.e. the acceleration backend being OpenGL). In that regard it is the same as WebGL, which currently gets enabled by default if the acceleration backend is OpenGL and throws an error if not. Perhaps CSS Shaders should do the same, meaning they should be guarded by a configuration flag? One potential difference from WebGL is that WebGL works even if accelerated compositing is turned off. I'm not sure if CSS shaders work if accelerated compositing is turned off, since I believe the implementation is in the actual TextureMapper itself.
(In reply to comment #2) > So instead of a configure option why not just enable them when experimental features are on and accelerated compositing is enabled. Do you mean having a --enable-experimental flags in configure.ac which would enable a few experimental flags ? > One potential difference from WebGL is that WebGL works even if accelerated compositing is turned off. I'm not sure if CSS shaders work if accelerated compositing is turned off, since I believe the implementation is in the actual TextureMapper itself. Indeed, css shaders cannot build without accelerated compositing, because it uses TilingData stuff.
(In reply to comment #6) > (In reply to comment #2) > > > So instead of a configure option why not just enable them when experimental features are on and accelerated compositing is enabled. > > Do you mean having a --enable-experimental flags in configure.ac which would enable a few experimental flags ? The flag is already there and currently named --enable-unstable-features.
(In reply to comment #4) > (From update of attachment 161758 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=161758&action=review > > > Source/WebCore/GNUmakefile.am:517 > > +feature_defines_overrides += ENABLE_CSS_SHADERS=1 > > You'll also need to add ENABLE_CSS_SHADERS entry to Source/WebCore/GNUmakefile.features.am. It's value depends on whether it should be enabled in developer builds or not. ENABLE_CSS_SHADERS was already defined in GNUmakefile.features.am > Also, if the configuration flag in configure.ac is in the end not used, this overriding depending on the ENABLE_CSS_SHADERS conditional is useless and should be removed. I'm not sure what you mean here.
Created attachment 162568 [details] updated patch updated patch: define enable_css_shaders in configure.ac if: experimental features and accelerated compositing and css filters are all enabled. define a enable-css-shaders property in webkitwebsettings. At runtime, css shaders can work even if accelerated compositing setting is disabled
(In reply to comment #9) > Created an attachment (id=162568) [details] > updated patch > > updated patch: > > define enable_css_shaders in configure.ac if: experimental features and accelerated compositing and css filters are all enabled. Unfortunately, css-filter support has just been removed in bug #87664
Created attachment 163422 [details] updated patch: build on top of #87664 and reintroduce css-filters
Comment on attachment 163422 [details] updated patch: build on top of #87664 and reintroduce css-filters View in context: https://bugs.webkit.org/attachment.cgi?id=163422&action=review > Source/WebKit/gtk/webkit/webkitwebsettings.cpp:948 > + * Enable or disable support for css shaders (css custom filters) It might be a good idea to link to the spec here as well as indicate whether or not accelerated compositing is required for this to work. A minor nit: you're missing a period here. > Source/WebKit/gtk/webkit/webkitwebsettings.cpp:950 > + * Since: 1.9.7 This should be 2.0 > configure.ac:967 > +# check whether to enable CSS Filters support You can just omit this comment. > configure.ac:973 > +AC_ARG_ENABLE(css_filters, > + AC_HELP_STRING([--enable-css-filters], > + [enable support for CSS Filters [default=no]]), > + [],[enable_css_filters="no"]) > +AC_MSG_RESULT([$enable_css_filters]) It still seems odd to expose a configure flag for a feature that isn't complete yet. Maybe you can just remove this. > configure.ac:976 > +# enable css shaders if unstable_features, css_filters and > +# accelerated_compositing are turned on Nit: Missing a capital letter and a period at the end. > configure.ac:983 > +AC_MSG_RESULT([$enable_css_shaders]) How about just doing this: if test "$enable_unstable_features" = "yes" && test "$enable_accelerated_compositing" = "yes" && test "$with_acceleration_backend" = "opengl"; then enable_css_shaders=yes enable_css_filters=yes fi Does that make sense? > configure.ac:1356 > + CSS Filters support : $enable_css_filters > + CSS Shaders support : $enable_css_shaders Probably best to not include this as well.
Created attachment 163434 [details] updated patch to address comments
Comment on attachment 163434 [details] updated patch to address comments Okay. Let's give this a shot.
Comment on attachment 163434 [details] updated patch to address comments Clearing flags on attachment: 163434 Committed r128231: <http://trac.webkit.org/changeset/128231>
All reviewed patches have been landed. Closing bug.