Bug 232831

Summary: [GPU Process] [Filters 13/23] Implement FilterEffect CoreImage appliers
Product: WebKit Reporter: Said Abou-Hallawa <sabouhallawa>
Component: Layout and RenderingAssignee: Said Abou-Hallawa <sabouhallawa>
Status: RESOLVED FIXED    
Severity: Normal CC: bfulgham, changseok, dino, esprehn+autocc, ews-watchlist, fmalita, glenn, gyuyoung.kim, heycam, kondapallykalyan, pdr, schenney, sergio, simon.fraser, webkit-bug-importer, zalan
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=235376
Bug Depends on:    
Bug Blocks: 231253    
Attachments:
Description Flags
Patch
heycam: review+
Patch none

Description Said Abou-Hallawa 2021-11-08 12:22:38 PST
This will eliminate the branching we do in CSSFilter::apply(). This will also remove the classes FilterEffectRenderer and FilterEffectRendererCoreImage.
Comment 1 Radar WebKit Bug Importer 2021-11-15 12:23:23 PST
<rdar://problem/85424049>
Comment 2 Said Abou-Hallawa 2021-11-26 21:53:34 PST
Created attachment 445211 [details]
Patch
Comment 3 Cameron McCormack (:heycam) 2021-11-27 14:03:57 PST
Comment on attachment 445211 [details]
Patch

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

> Source/WebCore/platform/graphics/coreimage/FEColorMatrixCoreImageApplier.mm:3
> +/*
> +* Copyright (C) 2020-2021 Apple Inc. All rights reserved.
> +*

This (and in the other added files in this patch) is copied from another file, but the asterisks are misaligned.

> Source/WebCore/svg/graphics/filters/SVGFilter.cpp:91
> +bool SVGFilter::supportsCoreImageRendering() const
> +{

I notice that this and CSSFIlter::supportsCoreImageRendering() are very similar, and that CSSFilter::m_functions and SVGFilter::m_expression are almost the same type. Should we have a common implementation of supportsCoreImageRendering() on Filter (or maybe you will move some CSSFilter and SVGFilter commonality up to Filter in a later patch in your queue)?
Comment 4 Said Abou-Hallawa 2021-11-27 19:26:54 PST
Created attachment 445229 [details]
Patch
Comment 5 Said Abou-Hallawa 2021-11-27 19:28:39 PST
Comment on attachment 445211 [details]
Patch

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

>> Source/WebCore/platform/graphics/coreimage/FEColorMatrixCoreImageApplier.mm:3
>> +*
> 
> This (and in the other added files in this patch) is copied from another file, but the asterisks are misaligned.

Fixed.

>> Source/WebCore/svg/graphics/filters/SVGFilter.cpp:91
>> +{
> 
> I notice that this and CSSFIlter::supportsCoreImageRendering() are very similar, and that CSSFilter::m_functions and SVGFilter::m_expression are almost the same type. Should we have a common implementation of supportsCoreImageRendering() on Filter (or maybe you will move some CSSFilter and SVGFilter commonality up to Filter in a later patch in your queue)?

Yes this similarities in CSSFilter and SVGFilter should be removed once the result FilterImage is removed from FilterEffect.
Comment 6 EWS 2021-11-27 20:28:19 PST
Committed r286193 (244575@main): <https://commits.webkit.org/244575@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 445229 [details].