Bug 199513

Created attachment 373496 [details] Patch

Comment on attachment 373496 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=373496&action=review Wow :( > Source/cmake/OptionsGTK.cmake:152 > +WEBKIT_OPTION_DEFAULT_PORT_VALUE(ENABLE_CSS_COMPOSITING PRIVATE ON) r=me, but please enable this in WebKitFeatures.cmake instead since surely no port wants this off, right?

Given that no port-specific code is required, I think it should be on. Maybe it's even worth to remove this option entirely.

(In reply to Michael Catanzaro from comment #2) > Comment on attachment 373496 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=373496&action=review > > Wow :( > > > Source/cmake/OptionsGTK.cmake:152 > > +WEBKIT_OPTION_DEFAULT_PORT_VALUE(ENABLE_CSS_COMPOSITING PRIVATE ON) > > r=me, but please enable this in WebKitFeatures.cmake instead since surely no > port wants this off, right? I'm not sure. Let's try that first on the EWS to test it doesn't break other ports.

Actually I'd like to get my words about platform-specific code back. According to my old research [1] it requires implementation of setBlendMode [1] https://github.com/qtwebkit/qtwebkit/issues/410

Created attachment 373500 [details] Patch Enabling the feature globally. Testing EWS

(In reply to Carlos Alberto Lopez Perez from comment #6) > Created attachment 373500 [details] > Patch > > Enabling the feature globally. Testing EWS The build breaks on AppleWin because it needs an implementation of PlatformCALayer::setBlendMode(). However, it passes on WinCairo. So, lets' try to enable this on all the ports but AppleWin. (In reply to Konstantin Tokarev from comment #5) > Actually I'd like to get my words about platform-specific code back. > According to my old research [1] it requires implementation of setBlendMode > > [1] https://github.com/qtwebkit/qtwebkit/issues/410 Not sure if that is needed anymore. The feature is working for me without doing that implementation.

Created attachment 373723 [details] Patch Testing EWS

(In reply to Carlos Alberto Lopez Perez from comment #7) > Not sure if that is needed anymore. > The feature is working for me without doing that implementation. I'm not sure if it will work when subject transform is applied to element which makes its own layer

(In reply to Konstantin Tokarev from comment #9) > (In reply to Carlos Alberto Lopez Perez from comment #7) > > Not sure if that is needed anymore. > > The feature is working for me without doing that implementation. > > I'm not sure if it will work when subject transform is applied to element > which makes its own layer Do you happen to have any test for that? I have ran all the 113 layout tests under css3/blending and 76 are passing (67.3% pass rate).

>Do you happen to have any test for that? I don't have such test, but it should be easy to make one: apply "transform: translateZ(0);" to element with mix-blend-mode/isolation/background-blend-mode or whatever property is being tested. It should make that element a separate TextureMapper layer.

(In reply to Konstantin Tokarev from comment #11) > >Do you happen to have any test for that? > > I don't have such test, but it should be easy to make one: apply "transform: > translateZ(0);" to element with > mix-blend-mode/isolation/background-blend-mode or whatever property is being > tested. It should make that element a separate TextureMapper layer. Like this one https://svn.webkit.org/repository/webkit/trunk/LayoutTests/css3/blending/blend-mode-simple-composited.html right? Yes, it is one of the tests that are failing after enabling the feature. It is not doing the blending in that case. However, I think is worth enabling this feature now even if there are cases where it is still not working as it should.

(In reply to Carlos Alberto Lopez Perez from comment #12) > However, I think is worth enabling this feature now even if there are cases > where it is still not working as it should. If it does not introduce any visible artifacts anywhere, go ahead

(In reply to Konstantin Tokarev from comment #13) > (In reply to Carlos Alberto Lopez Perez from comment #12) > > However, I think is worth enabling this feature now even if there are cases > > where it is still not working as it should. > > If it does not introduce any visible artifacts anywhere, go ahead No, it doesn't. I have tested several examples as well as checked all the css3/blending layout tests (with pixel tests enabled) and it either renders the blending correctly or is as bad as now (doesn't renders any blending). So its an improvement IMHO.

Comment on attachment 373723 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=373723&action=review > Source/cmake/OptionsFTW.cmake:-144 Better to do > Source/cmake/OptionsWin.cmake:99 > + WEBKIT_OPTION_DEFAULT_PORT_VALUE(ENABLE_CSS_COMPOSITING PUBLIC OFF) Yeah, this is nicer. This way we minimize the number of ports diverging from the WebKit default settings in WebKitFeatures.cmake.

Committed r247297: <https://trac.webkit.org/changeset/247297>