Bug 235376 - [Cocoa] Accelerated filters are enabled by the wrong setting
Summary: [Cocoa] Accelerated filters are enabled by the wrong setting
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Said Abou-Hallawa
URL:
Keywords: InRadar
Depends on:
Blocks: 231253
  Show dependency treegraph
 
Reported: 2022-01-19 12:48 PST by Said Abou-Hallawa
Modified: 2022-01-20 22:17 PST (History)
25 users (show)

See Also:


Attachments
Patch (54.35 KB, patch)
2022-01-19 13:04 PST, Said Abou-Hallawa
no flags Details | Formatted Diff | Diff
Patch (63.32 KB, patch)
2022-01-19 18:11 PST, Said Abou-Hallawa
no flags Details | Formatted Diff | Diff
Patch (68.19 KB, patch)
2022-01-20 17:24 PST, Said Abou-Hallawa
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (83.37 KB, patch)
2022-01-20 17:58 PST, Said Abou-Hallawa
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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>