[prev in list] [next in list] [prev in thread] [next in thread] 

List:       webkit-reviews
Subject:    [webkit-reviews] review denied: [Bug 95603] [Gtk] allow building with css-shaders : [Attachment 1617
From:       bugzilla-daemon () webkit ! org
Date:       2012-08-31 21:43:52
Message-ID: 20120831214411.05C2F1589BB () lists ! macosforge ! org
[Download RAW message or body]

Martin Robinson <mrobinson@webkit.org> has denied arno. <arno@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@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.
_______________________________________________
webkit-reviews mailing list
webkit-reviews@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-reviews
[prev in list] [next in list] [prev in thread] [next in thread]