Bug 199513 - [GTK][WPE] Enable support for CSS_COMPOSITING
Summary: [GTK][WPE] Enable support for CSS_COMPOSITING
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Carlos Alberto Lopez Perez
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-07-05 05:45 PDT by Carlos Alberto Lopez Perez
Modified: 2019-07-10 04:59 PDT (History)
6 users (show)

See Also:


Attachments
Patch (2.27 MB, patch)
2019-07-05 08:31 PDT, Carlos Alberto Lopez Perez
no flags Details | Formatted Diff | Diff
Patch (2.27 MB, patch)
2019-07-05 09:07 PDT, Carlos Alberto Lopez Perez
no flags Details | Formatted Diff | Diff
Patch (2.27 MB, patch)
2019-07-09 09:35 PDT, Carlos Alberto Lopez Perez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Carlos Alberto Lopez Perez 2019-07-05 05:45:37 PDT
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
Comment 1 Carlos Alberto Lopez Perez 2019-07-05 08:31:08 PDT
Created attachment 373496 [details]
Patch
Comment 2 Michael Catanzaro 2019-07-05 08:57:10 PDT
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?
Comment 3 Konstantin Tokarev 2019-07-05 09:00:33 PDT
Given that no port-specific code is required, I think it should be on. Maybe it's even worth to remove this option entirely.
Comment 4 Carlos Alberto Lopez Perez 2019-07-05 09:02:25 PDT
(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.
Comment 5 Konstantin Tokarev 2019-07-05 09:05:08 PDT
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
Comment 6 Carlos Alberto Lopez Perez 2019-07-05 09:07:58 PDT
Created attachment 373500 [details]
Patch

Enabling the feature globally. Testing EWS
Comment 7 Carlos Alberto Lopez Perez 2019-07-09 09:29:46 PDT
(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.
Comment 8 Carlos Alberto Lopez Perez 2019-07-09 09:35:48 PDT
Created attachment 373723 [details]
Patch

Testing EWS
Comment 9 Konstantin Tokarev 2019-07-09 09:37:29 PDT
(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
Comment 10 Carlos Alberto Lopez Perez 2019-07-09 09:42:44 PDT
(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).
Comment 11 Konstantin Tokarev 2019-07-09 09:51:13 PDT
>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.
Comment 12 Carlos Alberto Lopez Perez 2019-07-09 10:09:39 PDT
(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.
Comment 13 Konstantin Tokarev 2019-07-09 10:13:24 PDT
(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
Comment 14 Carlos Alberto Lopez Perez 2019-07-09 10:18:29 PDT
(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 15 Michael Catanzaro 2019-07-09 13:23:09 PDT
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.
Comment 16 Carlos Alberto Lopez Perez 2019-07-10 04:59:10 PDT
Committed r247297: <https://trac.webkit.org/changeset/247297>