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] Top level SVGFilter should own its FilterResults #13652

Closed
wants to merge 1 commit into from

Conversation

shallawa
Copy link
Contributor

@shallawa shallawa commented May 9, 2023

d8a093a

[GPU Process][Filters] Top level SVGFilter should own its FilterResults
https://bugs.webkit.org/show_bug.cgi?id=256535
rdar://109107572

Reviewed by NOBODY (OOPS!).

This will allow caching the FilterResults along with the SVGFilter in RemoteResourceCache.

GraphicsContext::drawFilteredImageBuffer() has to take FilterResultsEnsurer which
returns FilterResults&. drawFilteredImageBuffer() will call it when FilterResults
is needed. SVGFilter::ensureResults() will return FilterResults& and it takes
FilterResultsCreator. FilterResultsCreator returns a std::unique_ptr<FilterResults>
which SVGFilter will maintain.

* Source/WebCore/platform/graphics/GraphicsContext.cpp:
(WebCore::GraphicsContext::drawFilteredImageBuffer):
* Source/WebCore/platform/graphics/GraphicsContext.h:
* Source/WebCore/platform/graphics/displaylists/DisplayListItems.cpp:
(WebCore::DisplayList::DrawFilteredImageBuffer::apply):
* Source/WebCore/platform/graphics/displaylists/DisplayListItems.h:
* Source/WebCore/platform/graphics/displaylists/DisplayListRecorder.cpp:
(WebCore::DisplayList::Recorder::drawFilteredImageBuffer):
* Source/WebCore/platform/graphics/displaylists/DisplayListRecorder.h:
* Source/WebCore/platform/graphics/filters/FilterImageTargetSwitcher.cpp:
(WebCore::FilterImageTargetSwitcher::FilterImageTargetSwitcher):
(WebCore::FilterImageTargetSwitcher::endClipAndDrawSourceImage):
(WebCore::FilterImageTargetSwitcher::endDrawSourceImage):
* Source/WebCore/platform/graphics/filters/FilterImageTargetSwitcher.h:
* Source/WebCore/platform/graphics/filters/FilterResults.h:
* Source/WebCore/platform/graphics/filters/FilterStyleTargetSwitcher.cpp:
(WebCore::FilterStyleTargetSwitcher::endDrawSourceImage):
* Source/WebCore/platform/graphics/filters/FilterStyleTargetSwitcher.h:
* Source/WebCore/platform/graphics/filters/FilterTargetSwitcher.cpp:
(WebCore::FilterTargetSwitcher::create):
* Source/WebCore/platform/graphics/filters/FilterTargetSwitcher.h:
(WebCore::FilterTargetSwitcher::endClipAndDrawSourceImage):
(WebCore::FilterTargetSwitcher::endDrawSourceImage):
* Source/WebCore/rendering/svg/RenderSVGResourceFilter.cpp:
(WebCore::RenderSVGResourceFilter::applyResource):
(WebCore::RenderSVGResourceFilter::postApplyResource):
(WebCore::RenderSVGResourceFilter::markFilterForRepaint):
* Source/WebCore/rendering/svg/RenderSVGResourceFilter.h:
* Source/WebCore/svg/graphics/filters/SVGFilter.cpp:
(WebCore::SVGFilter::ensureResults):
(WebCore::SVGFilter::clearEffectResult):
* Source/WebCore/svg/graphics/filters/SVGFilter.h:
* Source/WebKit/GPUProcess/graphics/RemoteDisplayListRecorder.cpp:
(WebKit::RemoteDisplayListRecorder::drawFilteredImageBuffer):

d8a093a

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
βœ… πŸ›  watch
βœ… πŸ›  watch-sim

https://bugs.webkit.org/show_bug.cgi?id=256535
rdar://109107572

Reviewed by NOBODY (OOPS!).

This will allow caching the FilterResults along with the SVGFilter in RemoteResourceCache.

GraphicsContext::drawFilteredImageBuffer() has to take FilterResultsEnsurer which
returns FilterResults&. drawFilteredImageBuffer() will call it when FilterResults
is needed. SVGFilter::ensureResults() will return FilterResults& and it takes
FilterResultsCreator. FilterResultsCreator returns a std::unique_ptr<FilterResults>
which SVGFilter will maintain.

* Source/WebCore/platform/graphics/GraphicsContext.cpp:
(WebCore::GraphicsContext::drawFilteredImageBuffer):
* Source/WebCore/platform/graphics/GraphicsContext.h:
* Source/WebCore/platform/graphics/displaylists/DisplayListItems.cpp:
(WebCore::DisplayList::DrawFilteredImageBuffer::apply):
* Source/WebCore/platform/graphics/displaylists/DisplayListItems.h:
* Source/WebCore/platform/graphics/displaylists/DisplayListRecorder.cpp:
(WebCore::DisplayList::Recorder::drawFilteredImageBuffer):
* Source/WebCore/platform/graphics/displaylists/DisplayListRecorder.h:
* Source/WebCore/platform/graphics/filters/FilterImageTargetSwitcher.cpp:
(WebCore::FilterImageTargetSwitcher::FilterImageTargetSwitcher):
(WebCore::FilterImageTargetSwitcher::endClipAndDrawSourceImage):
(WebCore::FilterImageTargetSwitcher::endDrawSourceImage):
* Source/WebCore/platform/graphics/filters/FilterImageTargetSwitcher.h:
* Source/WebCore/platform/graphics/filters/FilterResults.h:
* Source/WebCore/platform/graphics/filters/FilterStyleTargetSwitcher.cpp:
(WebCore::FilterStyleTargetSwitcher::endDrawSourceImage):
* Source/WebCore/platform/graphics/filters/FilterStyleTargetSwitcher.h:
* Source/WebCore/platform/graphics/filters/FilterTargetSwitcher.cpp:
(WebCore::FilterTargetSwitcher::create):
* Source/WebCore/platform/graphics/filters/FilterTargetSwitcher.h:
(WebCore::FilterTargetSwitcher::endClipAndDrawSourceImage):
(WebCore::FilterTargetSwitcher::endDrawSourceImage):
* Source/WebCore/rendering/svg/RenderSVGResourceFilter.cpp:
(WebCore::RenderSVGResourceFilter::applyResource):
(WebCore::RenderSVGResourceFilter::postApplyResource):
(WebCore::RenderSVGResourceFilter::markFilterForRepaint):
* Source/WebCore/rendering/svg/RenderSVGResourceFilter.h:
* Source/WebCore/svg/graphics/filters/SVGFilter.cpp:
(WebCore::SVGFilter::ensureResults):
(WebCore::SVGFilter::clearEffectResult):
* Source/WebCore/svg/graphics/filters/SVGFilter.h:
* Source/WebKit/GPUProcess/graphics/RemoteDisplayListRecorder.cpp:
(WebKit::RemoteDisplayListRecorder::drawFilteredImageBuffer):
@shallawa shallawa self-assigned this May 9, 2023
@shallawa shallawa added the Layout and Rendering For bugs with layout and rendering of Web pages. label May 9, 2023
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label May 9, 2023
@@ -359,12 +359,13 @@ void GraphicsContext::drawConsumingImageBuffer(RefPtr<ImageBuffer> image, const
ImageBuffer::drawConsuming(WTFMove(image), *this, destination, source, options);
}

void GraphicsContext::drawFilteredImageBuffer(ImageBuffer* sourceImage, const FloatRect& sourceImageRect, Filter& filter, FilterResults& results)
void GraphicsContext::drawFilteredImageBuffer(ImageBuffer* sourceImage, const FloatRect& sourceImageRect, Filter& filter, const FilterResultsEnsurer& ensureFilterResults)
Copy link
Contributor

Choose a reason for hiding this comment

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

Not a fan of β€œensurer"

@@ -273,7 +273,7 @@ class GraphicsContext {
WEBCORE_EXPORT void drawConsumingImageBuffer(RefPtr<ImageBuffer>, const FloatRect& destination, const ImagePaintingOptions& = { });
WEBCORE_EXPORT void drawConsumingImageBuffer(RefPtr<ImageBuffer>, const FloatRect& destination, const FloatRect& source, const ImagePaintingOptions& = { });

WEBCORE_EXPORT virtual void drawFilteredImageBuffer(ImageBuffer* sourceImage, const FloatRect& sourceImageRect, Filter&, FilterResults&);
WEBCORE_EXPORT virtual void drawFilteredImageBuffer(ImageBuffer* sourceImage, const FloatRect& sourceImageRect, Filter&, const FilterResultsEnsurer& = nullptr);
Copy link
Contributor

Choose a reason for hiding this comment

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

Initializing a reference to nullptr is pretty sketchy.

@@ -49,6 +50,10 @@ class SVGFilter final : public Filter {

FilterEffectVector effectsOfType(FilterFunction::Type) const final;

FilterResults* results() { return m_results.get(); }
WEBCORE_EXPORT FilterResults& ensureResults(const FilterResultsCreator& createFilterResults = [] { return makeUnique<FilterResults>(); });
Copy link
Contributor

Choose a reason for hiding this comment

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

Either this doesn’t compile, or it does somehow and returns a reference to a deleted object.

@shallawa
Copy link
Contributor Author

The same PR is opened here #13676. So close this one.

@shallawa shallawa closed this May 10, 2023
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. merging-blocked Applied to prevent a change from being merged
Projects
None yet
4 participants