Bug 169988 - [GTK][WPE] Support for backdrop-filter
Summary: [GTK][WPE] Support for backdrop-filter
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks: 68469
  Show dependency treegraph
 
Reported: 2017-03-22 18:58 PDT by Carlos Alberto Lopez Perez
Modified: 2020-08-05 20:50 PDT (History)
18 users (show)

See Also:


Attachments
Patch (44.59 KB, patch)
2020-07-24 07:00 PDT, Carlos Garcia Campos
aperez: review+
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 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>