Bug 235376

Summary: [Cocoa] Accelerated filters are enabled by the wrong setting
Product: WebKit Reporter: Said Abou-Hallawa <sabouhallawa>
Component: Layout and RenderingAssignee: Said Abou-Hallawa <sabouhallawa>
Status: RESOLVED FIXED    
Severity: Normal CC: annulen, benjamin, bfulgham, cdumez, changseok, cmarcelo, ddkilzer, dino, esprehn+autocc, ews-watchlist, fmalita, fred.wang, glenn, gyuyoung.kim, kondapallykalyan, macpherson, menard, pdr, ryuan.choi, schenney, sergio, simon.fraser, thorton, 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=149424
https://bugs.webkit.org/show_bug.cgi?id=73842
https://bugs.webkit.org/show_bug.cgi?id=232831
Bug Depends on:    
Bug Blocks: 231253    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
ews-feeder: commit-queue-
Patch none

Description Said Abou-Hallawa 2022-01-19 12:48:30 PST
There has been a confusion about how CoreImage filters can be enabled. There are two settings/feature controls which have been used:

1. CoreImageAcceleratedFilterRenderEnabled
2. AcceleratedFiltersEnabled

The first was used to enable CoreImage filters which also requires to have accelerated filters. It is also linked with the mini-browser experimental feature "CoreImage-Accelerated Filter Rendering". The use of it was mistakenly dropped in r286193 and AcceleratedFiltersEnabled has been used since then.

The second one was introduced by r102088 for the Chromium port back in 2011. There is nothing in WebKit enables this setting right now. And for Apple ports, I do not think there is any current plans for using IOSurfaces for software filters.

So I think to use the correct setting for enabling the CoreImage filters and to clean this area we need to do the following:

1. Delete the setting AcceleratedFiltersEnabled
2. Use CoreImageAcceleratedFilterRenderEnabled to enable CoreImage filters and to force using IOSurfaces for the intermediate filter images. This setting will be respected only if the filter chain can be rendered via CoreImage.
3. Fix the GTK unused parameter warning from FEColorMatrix::createApplier(), FEComponentTransfer::createApplier() and SourceGraphic::createApplier().
Comment 1 Said Abou-Hallawa 2022-01-19 13:04:08 PST
Created attachment 449508 [details]
Patch
Comment 2 Said Abou-Hallawa 2022-01-19 18:11:39 PST
Created attachment 449539 [details]
Patch
Comment 3 Tim Horton 2022-01-20 02:34:24 PST
Comment on attachment 449539 [details]
Patch

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

> Source/WTF/Scripts/Preferences/WebPreferencesExperimental.yaml:414
>    webcoreOnChange: setNeedsRelayoutAllFrames

can we change the preference name to CoreImageAcceleratedFiltersEnabled or UseCoreImageForAcceleratedFilters? "filter render" is a weird term.

> Source/WebCore/css/CSSFilterImageValue.cpp:118
> +    auto renderingMode = renderer.page().acceleratedFilterRenderEnabled() ? RenderingMode::Accelerated : RenderingMode::Unaccelerated;

"acceleratedFilterRender*ing*Enabled" I think would read more correctly, but also it could be simplified as above.
Comment 4 Said Abou-Hallawa 2022-01-20 17:24:05 PST
Created attachment 449626 [details]
Patch
Comment 5 Said Abou-Hallawa 2022-01-20 17:58:45 PST
Created attachment 449627 [details]
Patch
Comment 6 EWS 2022-01-20 22:16:23 PST
Committed r288352 (246258@main): <https://commits.webkit.org/246258@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 449627 [details].
Comment 7 Radar WebKit Bug Importer 2022-01-20 22:17:17 PST
<rdar://problem/87868182>