Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[GPU Process] [Filters] Cache the SVGFilter applying results in RemoteResourceCache #13814

Conversation

shallawa
Copy link
Contributor

@shallawa shallawa commented May 12, 2023

b686ba4

[GPU Process] [Filters] Cache the SVGFilter applying results in RemoteResourceCache
https://bugs.webkit.org/show_bug.cgi?id=232845
rdar://85426641

Reviewed by Simon Fraser.

This allows caching the results of applying an SVGFilter to a source ImageBuffer
in GPU Process. We should be able to use the cached result if the Filter was not
changed. We should be also able to clear some of the result FilterImages if a
FilterEffect was changed.

* Source/WebCore/rendering/svg/RenderSVGResourceFilter.cpp:
(WebCore::RenderSVGResourceFilter::applyResource):
* Source/WebCore/svg/graphics/filters/SVGFilter.cpp:
(WebCore::SVGFilter::mergeEffects):
* Source/WebCore/svg/graphics/filters/SVGFilter.h:
* Source/WebKit/GPUProcess/graphics/RemoteDisplayListRecorder.cpp:
(WebKit::RemoteDisplayListRecorder::drawFilteredImageBufferInternal):
(WebKit::RemoteDisplayListRecorder::drawFilteredImageBuffer):
* Source/WebKit/GPUProcess/graphics/RemoteDisplayListRecorder.h:
* Source/WebKit/WebProcess/GPU/graphics/RemoteDisplayListRecorderProxy.cpp:
(WebKit::RemoteDisplayListRecorderProxy::recordDrawFilteredImageBuffer):

Canonical link: https://commits.webkit.org/264087@main

48a0df2

Misc iOS, tvOS & watchOS macOS Linux Windows
βœ… πŸ§ͺ style βœ… πŸ›  ios βœ… πŸ›  mac βœ… πŸ›  wpe   πŸ›  wincairo
βœ… πŸ§ͺ bindings βœ… πŸ›  ios-sim βœ… πŸ›  mac-AS-debug   πŸ§ͺ wpe-wk2
βœ… πŸ§ͺ webkitperl βœ… πŸ§ͺ ios-wk2 βœ… πŸ§ͺ api-mac βœ… πŸ›  gtk
  πŸ§ͺ ios-wk2-wpt βœ… πŸ§ͺ mac-wk1   πŸ§ͺ gtk-wk2
  πŸ§ͺ api-ios βœ… πŸ§ͺ mac-wk2 βœ… πŸ§ͺ api-gtk
βœ… πŸ›  tv βœ… πŸ§ͺ mac-AS-debug-wk2
βœ… πŸ›  tv-sim
❌ πŸ›  πŸ§ͺ merge βœ… πŸ›  watch
  πŸ›  watch-sim

@shallawa shallawa requested a review from cdumez as a code owner May 12, 2023 17:22
@shallawa shallawa self-assigned this May 12, 2023
@shallawa shallawa force-pushed the eng/GPU-Process-Filters-2223-Cache-the-filter-applying-results-in-RemoteResourceCache branch from a59ad65 to cb8d4f1 Compare May 12, 2023 17:22
@shallawa shallawa added the Layout and Rendering For bugs with layout and rendering of Web pages. label May 12, 2023
@smfr
Copy link
Contributor

smfr commented May 12, 2023

[GPU Process] [Filters 22/23] Cache the SVGFilter applying results in RemoteResourceCache

I don't think the "22/23" is useful.

@shallawa shallawa force-pushed the eng/GPU-Process-Filters-2223-Cache-the-filter-applying-results-in-RemoteResourceCache branch from cb8d4f1 to 3e2cdfb Compare May 12, 2023 17:25
@shallawa shallawa changed the title [GPU Process] [Filters 22/23] Cache the SVGFilter applying results in RemoteResourceCache [GPU Process] [Filters] Cache the SVGFilter applying results in RemoteResourceCache May 12, 2023
@shallawa shallawa force-pushed the eng/GPU-Process-Filters-2223-Cache-the-filter-applying-results-in-RemoteResourceCache branch from 3e2cdfb to 0df44f4 Compare May 12, 2023 17:26
@shallawa shallawa requested a review from smfr May 12, 2023 17:26
class FilterEffect;

struct SVGFilterTransactionItem {
unsigned index;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Index into what? This isn't the index in this Vector<>, right?

It's a bit confusing to use these vectors for SVG filters, where the filter structure is a graph, and more confusing to have this standalone structure that contains indexes into something else.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is an index to the SVGFilter::m_effects which is a Vector of the unique effects in the SVGFilter graph. The order of the effects in WebProcess and GPUProcess is the same. This is why when a certain FilterEffect changes in WebProcess we lookup its index in SVGFilter::m_effects and send it with this index to GPUProcess in SVGFilterTransaction. Using this index, GPUProcess will replace this effect in SVGFilter::m_effects and delete its FilterImage along with the result of all the dependent FilterEffects.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SVGFilterExpression represents the graph of the SVGFilter. It is a Vector of SVGFilterExpressionTerm. Each SVGFilterExpressionTerm has an index pointing to the SVGFilter::m_effects.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a way we can package up SVGFilterExpression and SVGFilterTransaction into one struct, so we don't have these dangling indexes?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes we can send the whole SVGFilter to GPUP and merge the new effects with the old cached one. If an effect does not match its cached corresponding one, we replace the old with the new and we clear the result and all the dependent results of this effect.

@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label May 12, 2023
@shallawa shallawa removed the merging-blocked Applied to prevent a change from being merged label May 13, 2023
@shallawa shallawa force-pushed the eng/GPU-Process-Filters-2223-Cache-the-filter-applying-results-in-RemoteResourceCache branch from 0df44f4 to 35b38f0 Compare May 13, 2023 09:06
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label May 13, 2023
@shallawa shallawa removed the merging-blocked Applied to prevent a change from being merged label May 14, 2023
@shallawa shallawa changed the title [GPU Process] [Filters] Cache the SVGFilter applying results in RemoteResourceCache `[GPU Process] [Filters] Cache the SVGFilter applying results in RemoteResourceCache May 14, 2023
@shallawa shallawa force-pushed the eng/GPU-Process-Filters-2223-Cache-the-filter-applying-results-in-RemoteResourceCache branch from 35b38f0 to cd149b1 Compare May 14, 2023 17:19
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label May 14, 2023
@shallawa shallawa changed the title `[GPU Process] [Filters] Cache the SVGFilter applying results in RemoteResourceCache [GPU Process] [Filters] Cache the SVGFilter applying results in RemoteResourceCache May 14, 2023
return;
}

RefPtr cahcedFilter = resourceCache().cachedFilter({ filter->renderingResourceIdentifier(), m_webProcessIdentifier });
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"cahcedFilter"

}

RefPtr cahcedFilter = resourceCache().cachedFilter({ filter->renderingResourceIdentifier(), m_webProcessIdentifier });
if (!cahcedFilter || !is<SVGFilter>(*cahcedFilter)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could be a single dynamicDowncast<>

@shallawa shallawa removed the merging-blocked Applied to prevent a change from being merged label May 15, 2023
@shallawa shallawa force-pushed the eng/GPU-Process-Filters-2223-Cache-the-filter-applying-results-in-RemoteResourceCache branch from cd149b1 to 48a0df2 Compare May 15, 2023 22:11
@shallawa shallawa added the merge-queue Applied to send a pull request to merge-queue label May 15, 2023
@webkit-commit-queue webkit-commit-queue force-pushed the eng/GPU-Process-Filters-2223-Cache-the-filter-applying-results-in-RemoteResourceCache branch from 48a0df2 to 8e47cee Compare May 15, 2023 23:07
…eResourceCache

https://bugs.webkit.org/show_bug.cgi?id=232845
rdar://85426641

Reviewed by Simon Fraser.

This allows caching the results of applying an SVGFilter to a source ImageBuffer
in GPU Process. We should be able to use the cached result if the Filter was not
changed. We should be also able to clear some of the result FilterImages if a
FilterEffect was changed.

* Source/WebCore/rendering/svg/RenderSVGResourceFilter.cpp:
(WebCore::RenderSVGResourceFilter::applyResource):
* Source/WebCore/svg/graphics/filters/SVGFilter.cpp:
(WebCore::SVGFilter::mergeEffects):
* Source/WebCore/svg/graphics/filters/SVGFilter.h:
* Source/WebKit/GPUProcess/graphics/RemoteDisplayListRecorder.cpp:
(WebKit::RemoteDisplayListRecorder::drawFilteredImageBufferInternal):
(WebKit::RemoteDisplayListRecorder::drawFilteredImageBuffer):
* Source/WebKit/GPUProcess/graphics/RemoteDisplayListRecorder.h:
* Source/WebKit/WebProcess/GPU/graphics/RemoteDisplayListRecorderProxy.cpp:
(WebKit::RemoteDisplayListRecorderProxy::recordDrawFilteredImageBuffer):

Canonical link: https://commits.webkit.org/264087@main
@webkit-commit-queue webkit-commit-queue force-pushed the eng/GPU-Process-Filters-2223-Cache-the-filter-applying-results-in-RemoteResourceCache branch from 8e47cee to b686ba4 Compare May 15, 2023 23:10
@webkit-commit-queue
Copy link
Collaborator

Committed 264087@main (b686ba4): https://commits.webkit.org/264087@main

Reviewed commits have been landed. Closing PR #13814 and removing active labels.

@webkit-commit-queue webkit-commit-queue merged commit b686ba4 into WebKit:main May 15, 2023
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label May 15, 2023
@shallawa shallawa deleted the eng/GPU-Process-Filters-2223-Cache-the-filter-applying-results-in-RemoteResourceCache branch May 17, 2023 01:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Layout and Rendering For bugs with layout and rendering of Web pages.
Projects
None yet
5 participants