Bug 232844

Summary: [GPU Process] [Filters 21/23] Add the encoding/decoding for Filter and FilterEffect
Product: WebKit Reporter: Said Abou-Hallawa <sabouhallawa>
Component: Layout and RenderingAssignee: Said Abou-Hallawa <sabouhallawa>
Status: RESOLVED FIXED    
Severity: Normal CC: annulen, bfulgham, changseok, dino, esprehn+autocc, ews-watchlist, fmalita, fred.wang, glenn, gyuyoung.kim, kondapallykalyan, pdr, ryuan.choi, schenney, sergio, simon.fraser, ujwal.koneru, webkit-bug-importer, wenson_hsieh, zalan
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 231253    
Attachments:
Description Flags
Patch
ews-feeder: commit-queue-
Patch
ews-feeder: commit-queue-
Patch
ews-feeder: commit-queue-
Patch
ews-feeder: commit-queue-
Patch
wenson_hsieh: review+
Patch none

Said Abou-Hallawa
Reported 2021-11-08 13:21:21 PST
This will make it possible to transfer the Filter and FilterEffects data from WebProcess to GPUProcess.
Attachments
Patch (119.51 KB, patch)
2021-12-04 22:45 PST, Said Abou-Hallawa 		ews-feeder: commit-queue-
DetailsFormatted DiffDiff
Patch (121.35 KB, patch)
2021-12-04 23:33 PST, Said Abou-Hallawa 		ews-feeder: commit-queue-
DetailsFormatted DiffDiff
Patch (123.16 KB, patch)
2021-12-05 00:20 PST, Said Abou-Hallawa 		ews-feeder: commit-queue-
DetailsFormatted DiffDiff
Patch (121.21 KB, patch)
2021-12-05 01:00 PST, Said Abou-Hallawa 		ews-feeder: commit-queue-
DetailsFormatted DiffDiff
Patch (121.26 KB, patch)
2021-12-05 01:18 PST, Said Abou-Hallawa 		wenson_hsieh: review+
DetailsFormatted DiffDiff
Patch (121.19 KB, patch)
2021-12-05 20:27 PST, Said Abou-Hallawa 		no flags
DetailsFormatted DiffDiff
Show Obsolete (4) View All Add attachment proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1 2021-11-15 13:24:38 PST
<rdar://problem/85426351>
Said Abou-Hallawa
Comment 2 2021-12-04 22:45:59 PST
Created attachment 445978 [details] Patch
Said Abou-Hallawa
Comment 3 2021-12-04 23:33:19 PST
Created attachment 445979 [details] Patch
Said Abou-Hallawa
Comment 4 2021-12-05 00:20:42 PST
Created attachment 445981 [details] Patch
Said Abou-Hallawa
Comment 5 2021-12-05 01:00:21 PST
Created attachment 445982 [details] Patch
Said Abou-Hallawa
Comment 6 2021-12-05 01:18:38 PST
Created attachment 445983 [details] Patch
Wenson Hsieh
Comment 7 2021-12-05 11:32:08 PST
Comment on attachment 445983 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=445983&action=review > Source/WebCore/platform/graphics/filters/FEColorMatrix.h:99 > +template<> struct EnumTraits<WebCore::ColorMatrixType> { Nit - we should consider making ColorMatrixType an `enum class : uint8_t`, perhaps in a separate patch. > Source/WebCore/platform/graphics/filters/FEComponentTransfer.h:179 > + WebCore::ComponentTransferType, We should consider making ComponentTransferType an enum class as well (in a followup). > Source/WebCore/platform/graphics/filters/FEComposite.h:137 > + WebCore::CompositeOperationType, (Ditto for CompositeOperationType) > Source/WebCore/platform/graphics/filters/FEMerge.h:43 > + unsigned m_numberOfEffectInputs; Nit - let's make this initialize to `{ 0 };` > Source/WebCore/platform/graphics/filters/Filter.h:79 > + using FilterFunction::FilterFunction; Is this necessary? I don't see any nested `FilterFunction` symbol defined in the `FilterFunction` class (or a separate namespace called `FilterFunction`) > Source/WebKit/WebProcess/GPU/graphics/RemoteRenderingBackendProxy.cpp:229 > + auto sendResult = sendSync(Messages::RemoteRenderingBackend::GetFilteredImageForImageBuffer(imageBuffer, IPC::FilterReference { filter }), Messages::RemoteRenderingBackend::GetFilteredImageForImageBuffer::Reply(handle), renderingBackendIdentifier(), 1_s); I'm a little surprised that this actually works, given that RRB only receives stream messages. Did you mean to use sendSyncToStream() instead?
Said Abou-Hallawa
Comment 8 2021-12-05 20:14:00 PST
Comment on attachment 445983 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=445983&action=review >> Source/WebCore/platform/graphics/filters/FEMerge.h:43 >> + unsigned m_numberOfEffectInputs; > > Nit - let's make this initialize to `{ 0 };` Fixed. >> Source/WebCore/platform/graphics/filters/Filter.h:79 >> + using FilterFunction::FilterFunction; > > Is this necessary? I don't see any nested `FilterFunction` symbol defined in the `FilterFunction` class (or a separate namespace called `FilterFunction`) This defines a new constructor for the Filter class which will be called from the new CSSFilter constructor: CSSFilter::CSSFilter(Vector<Ref<FilterFunction>>&& functions) : Filter(Type::CSSFilter) , m_functions(WTFMove(functions)) { } Instead of writing Filter::Filter(Type filterType) : FilterFunction(filterType) { } we can just write using FilterFunction::FilterFunction; >> Source/WebKit/WebProcess/GPU/graphics/RemoteRenderingBackendProxy.cpp:229 >> + auto sendResult = sendSync(Messages::RemoteRenderingBackend::GetFilteredImageForImageBuffer(imageBuffer, IPC::FilterReference { filter }), Messages::RemoteRenderingBackend::GetFilteredImageForImageBuffer::Reply(handle), renderingBackendIdentifier(), 1_s); > > I'm a little surprised that this actually works, given that RRB only receives stream messages. Did you mean to use sendSyncToStream() instead? You are right. Fixed.
Said Abou-Hallawa
Comment 9 2021-12-05 20:27:30 PST
Created attachment 445992 [details] Patch
EWS
Comment 10 2021-12-05 21:49:59 PST
Committed r286538 (244870@main): <https://commits.webkit.org/244870@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 445992 [details].
Note You need to log in before you can comment on or make changes to this bug.