[webkit-reviews] review denied: [Bug 95603] [Gtk] allow building with css-shaders : [Attachment 161758] patch proposal

[bugzilla-daemon at webkit.org]

Martin Robinson <mrobinson at webkit.org> has denied arno. <arno at renevier.net>'s
request for review:
Bug 95603: [Gtk] allow building with css-shaders
https://bugs.webkit.org/show_bug.cgi?id=95603

Attachment 161758: patch proposal
https://bugs.webkit.org/attachment.cgi?id=161758&action=review

------- Additional Comments from Martin Robinson <mrobinson at webkit.org>
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.