[Webkit-unassigned] [Bug 169988] [GTK][WPE] Support for backdrop-filter

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=169988

--- Comment #6 from Carlos Garcia Campos <cgarcia at igalia.com> ---
(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.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20200727/df21d409/attachment-0001.htm>