Bug 169988

Summary: [GTK][WPE] Support for backdrop-filter
Product: WebKit Reporter: Carlos Alberto Lopez Perez <clopez>
Component: CSSAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: annulen, aperez, bugs-noreply, cgarcia, cmarcelo, ews-watchlist, gyuyoung.kim, kondapallykalyan, luiz, magomez, noam, pnormand, ryuan.choi, sergio, webkit-bug-importer, yoshiaki.jitsukawa, zan, zeno
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=215209
Bug Depends on:    
Bug Blocks: 68469    
Attachments:
Description Flags
Patch aperez: review+

Description Carlos Alberto Lopez Perez 2017-03-22 18:58:29 PDT
The GTK port misses support for CSS backdrop filters.

This was implemented in bug 138384 for Mac/iOS.

More info: https://webkit.org/blog/3632/introducing-backdrop-filters/
Comment 1 Carlos Alberto Lopez Perez 2017-03-22 20:27:56 PDT
Status of support for the feature in other browsers: http://caniuse.com/#feat=css-backdrop-filter
Comment 2 Adrian Perez 2017-10-19 15:22:40 PDT
On the surface it looks like the feature could be enabled by
adding the following in “PlatformGTK.cmake”:

  WEBKIT_OPTION_DEFAULT_PORT_VALUE(ENABLE_FILTERS_LEVEL_2 PRIVATE ON)

but this alone is not enough for backdrop-filter to work. After
looking at how this was implemented in Cocoa, I think the crux of
the matter is that we have somehow to render normally the content
that falls behind the element to which backdrop filters are applied,
so then the result can be used to apply them. Cocoa seems to do
this using CoreAnimation, so with my (extremely) limited knowledge
of the graphics stack, we would need to implement the needed bits
in the texture mapper, right?
Comment 3 Philippe Normand 2020-07-20 12:43:37 PDT
Would be nice to support this at some point. results.webkit.org relies on this feature.
Comment 4 Carlos Garcia Campos 2020-07-24 07:00:23 PDT
Created attachment 405139 [details]
Patch

There are still a few tests failing (some of them due to other bugs like -webkit-box-reflect not working on compositing), but the examples I've been trying are all working (https://webkit.org/demos/backdrop-filter and https://results.webkit.org).
Comment 5 Adrian Perez 2020-07-24 15:26:09 PDT
Comment on attachment 405139 [details]
Patch

Wow, this is quite a neat patch overall, somehow before starting reading
it I thought it would end up being much more complicated 💪️

I made just one comment below about the isFilterProperty lambda.

View in context: https://bugs.webkit.org/attachment.cgi?id=405139&action=review

> Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedGraphicsLayer.cpp:1358
> +    auto isFilterProperty = [&]() -> bool {

The lambda here only uses “valueList.property()” so I would only capture
that explicitly 

  auto isFilterProperty = [property = valueList.property()]() -> bool { /* ... */ };

Though, honestly I do not see the need to use a lambda here, as it is used
only once immediately and it calculates a simple value. Why not calculate
the value directly? One possible way:

      bool isFilterProperty;
      switch (valueList.property()) {
  #if ENABLE(FILTERS_LEVEL_2)
      case AnimatedPropertyWebkitBackdropFilter:
  #endif
      case AnimatedPropertyFilter:
          isFilterProperty = true;
          break;
      default:
          isFilterProperty = false;
      }

      if (isFilterProperty) {
          /* ... */
      }
Comment 6 Carlos Garcia Campos 2020-07-27 00:02:46 PDT
(In reply to Adrian Perez from comment #5)
> Comment on attachment 405139 [details]
> Patch
> 
> Wow, this is quite a neat patch overall, somehow before starting reading
> it I thought it would end up being much more complicated 💪️
> 
> I made just one comment below about the isFilterProperty lambda.
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=405139&action=review
> 
> > Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedGraphicsLayer.cpp:1358
> > +    auto isFilterProperty = [&]() -> bool {
> 
> The lambda here only uses “valueList.property()” so I would only capture
> that explicitly 
> 
>   auto isFilterProperty = [property = valueList.property()]() -> bool { /*
> ... */ };
> 
> Though, honestly I do not see the need to use a lambda here, as it is used
> only once immediately and it calculates a simple value. Why not calculate
> the value directly? One possible way:
> 
>       bool isFilterProperty;
>       switch (valueList.property()) {
>   #if ENABLE(FILTERS_LEVEL_2)
>       case AnimatedPropertyWebkitBackdropFilter:
>   #endif
>       case AnimatedPropertyFilter:
>           isFilterProperty = true;
>           break;
>       default:
>           isFilterProperty = false;
>       }
> 
>       if (isFilterProperty) {
>           /* ... */
>       }

Ended up adding the lambda because with the if and the #ifdef it looked bad and the style checker always complained. The switch is a good idea and then we don't even need the isFilterProperty variable, I think.
Comment 7 Carlos Garcia Campos 2020-07-28 02:03:39 PDT
Committed r264968: <https://trac.webkit.org/changeset/264968>
Comment 8 Radar WebKit Bug Importer 2020-07-28 02:04:18 PDT
<rdar://problem/66207065>