Bug 199513 – [GTK][WPE] Enable support for CSS_COMPOSITING

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[Carlos Alberto Lopez Perez]

Apple ports enabled it a lot of time ago: https://trac.webkit.org/r130460

Enabling this feature allow to support the CSS properties "mix-blend-mode" and "isolation"

Examples:

https://www.w3schools.com/cssref/tryit.asp?filename=trycss_mix-blend-mode
https://developer.mozilla.org/en-US/docs/Web/CSS/isolation

[Carlos Alberto Lopez Perez]

Created attachment 373496 [details]
Patch

[Michael Catanzaro]

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?

[Konstantin Tokarev]

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

[Carlos Alberto Lopez Perez]

(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.

[Konstantin Tokarev]

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

[Carlos Alberto Lopez Perez]

Created attachment 373500 [details]
Patch

Enabling the feature globally. Testing EWS

[Carlos Alberto Lopez Perez]

(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.

[Carlos Alberto Lopez Perez]

Created attachment 373723 [details]
Patch

Testing EWS

[Konstantin Tokarev]

(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

[Carlos Alberto Lopez Perez]

(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).

[Konstantin Tokarev]

>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.

[Carlos Alberto Lopez Perez]

(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.

[Konstantin Tokarev]

(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

[Carlos Alberto Lopez Perez]

(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.

[Michael Catanzaro]

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.

[Carlos Alberto Lopez Perez]

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