Skip to content

Commit

Permalink
[Filters] Do not clip the destination context when compositing the fi…
Browse files Browse the repository at this point in the history
…lter style transparency layers

https://bugs.webkit.org/show_bug.cgi?id=261926
rdar://115901634

Reviewed by Simon Fraser.

Clipping is not needed at that time because it has to be applied to the filter
style transparency layers before drawing the target element.

If this unneeded clipping is applied, removing this clipping will overlap with
ending the transparency layers. This will end up calling GraphicsContext::restore()
with the wrong purpose.

Rename FilterTargetSwitcher::needsRedrawSourceImage() to hasSourceImage() and
flip its meaning. This will make differentiating software filters from style
filters clearer. When hasSourceImage() is true, this means the target element is
drawn to an ImageBuffer.

* LayoutTests/css3/filters/drop-shadow-child-clipped-expected.html: Added.
* LayoutTests/css3/filters/drop-shadow-child-clipped.html: Added.
* LayoutTests/fast/filter-image/clipped-filter.html:
* Source/WebCore/platform/graphics/filters/FilterImageTargetSwitcher.h:
* Source/WebCore/platform/graphics/filters/FilterStyleTargetSwitcher.h:
(): Deleted.
* Source/WebCore/platform/graphics/filters/FilterTargetSwitcher.h:
(WebCore::FilterTargetSwitcher::hasSourceImage const):
(WebCore::FilterTargetSwitcher::needsRedrawSourceImage const): Deleted.
* Source/WebCore/rendering/RenderLayer.cpp:
(WebCore::RenderLayer::applyFilters):
* Source/WebCore/rendering/RenderLayerFilters.cpp:
(WebCore::RenderLayerFilters::hasSourceImage const):
(WebCore::RenderLayerFilters::beginFilterEffect):
(WebCore::RenderLayerFilters::needsRedrawSourceImage const): Deleted.
* Source/WebCore/rendering/RenderLayerFilters.h:
* Source/WebCore/rendering/svg/RenderSVGResourceFilter.cpp:
(WebCore::RenderSVGResourceFilter::applyResource):

Canonical link: https://commits.webkit.org/268341@main
  • Loading branch information
shallawa authored and Said Abou-Hallawa committed Sep 22, 2023
1 parent 4c8538a commit a73cf05
Show file tree
Hide file tree
Showing 10 changed files with 73 additions and 19 deletions.
28 changes: 28 additions & 0 deletions LayoutTests/css3/filters/drop-shadow-child-clipped-expected.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
<style>
.container {
width: 135px;
height: 100px;
border: gray 1px solid;
overflow: hidden;
}
.box {
width: 45px;
height: 100px;
position: relative;
}
.blue-background {
left: 0;
background: MediumBlue;
}
.red-background {
left: 90px;
top: -100px;
background: crimson;
}
</style>
<body>
<div class="container">
<div class="box blue-background"></div>
<div class="box red-background"></div>
</div>
</body>
22 changes: 22 additions & 0 deletions LayoutTests/css3/filters/drop-shadow-child-clipped.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
<!-- webkit-test-runner [ GraphicsContextFiltersEnabled=true ] -->
<style>
.container {
width: 135px;
height: 100px;
border: gray 1px solid;
overflow: hidden;
}
.box {
width: 45px;
height: 100px;
position: relative;
left: 90px;
background: crimson;
filter: drop-shadow(-90px 0 0 MediumBlue);
}
</style>
<body>
<div class="container">
<div class="box"></div>
</div>
</body>
2 changes: 1 addition & 1 deletion LayoutTests/fast/filter-image/clipped-filter.html
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@
<svg>
<defs>
<clipPath id="clipPath">
<path d="M18,38 v200 h90 a150,150 0 0,1 300,0 h50 v-200 z"
<path d="M18,38 v200 h90 a150,150 0 0,1 300,0 h50 v-200 z">
</clipPath>
</defs>
</svg>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (C) 2022 Apple Inc. All rights reserved.
* Copyright (C) 2022-2023 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
Expand Down Expand Up @@ -39,6 +39,8 @@ class FilterImageTargetSwitcher final : public FilterTargetSwitcher {
private:
GraphicsContext* drawingContext(GraphicsContext& destinationContext) const override;

bool hasSourceImage() const override { return m_sourceImage; }

void beginClipAndDrawSourceImage(GraphicsContext& destinationContext, const FloatRect& repaintRect) override;
void endClipAndDrawSourceImage(GraphicsContext& destinationContext) override;
void endDrawSourceImage(GraphicsContext& destinationContext) override;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (C) 2022 Apple Inc. All rights reserved.
* Copyright (C) 2022-2023 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
Expand Down Expand Up @@ -36,8 +36,6 @@ class FilterStyleTargetSwitcher : public FilterTargetSwitcher {
FilterStyleTargetSwitcher(Filter&, const FloatRect &sourceImageRect);

private:
bool needsRedrawSourceImage() const override { return true; }

void beginDrawSourceImage(GraphicsContext& destinationContext) override;
void endDrawSourceImage(GraphicsContext& destinationContext) override;

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (C) 2022 Apple Inc. All rights reserved.
* Copyright (C) 2022-2023 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
Expand Down Expand Up @@ -44,7 +44,7 @@ class FilterTargetSwitcher {

virtual GraphicsContext* drawingContext(GraphicsContext& destinationContext) const { return &destinationContext; }

virtual bool needsRedrawSourceImage() const { return false; }
virtual bool hasSourceImage() const { return false; }

virtual void beginClipAndDrawSourceImage(GraphicsContext& destinationContext, const FloatRect&) { beginDrawSourceImage(destinationContext); }
virtual void endClipAndDrawSourceImage(GraphicsContext& destinationContext) { endDrawSourceImage(destinationContext); }
Expand Down
15 changes: 10 additions & 5 deletions Source/WebCore/rendering/RenderLayer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3162,13 +3162,18 @@ GraphicsContext* RenderLayer::setupFilters(GraphicsContext& destinationContext,

void RenderLayer::applyFilters(GraphicsContext& originalContext, const LayerPaintingInfo& paintingInfo, OptionSet<PaintBehavior> behavior, const LayerFragments& layerFragments)
{
// FIXME: Handle more than one fragment.
ClipRect backgroundRect = layerFragments.isEmpty() ? ClipRect() : layerFragments[0].backgroundRect;

GraphicsContextStateSaver stateSaver(originalContext, false);
RegionContextStateSaver regionContextStateSaver(paintingInfo.regionContext);
bool needsClipping = m_filters->hasSourceImage();

if (needsClipping) {
// FIXME: Handle more than one fragment.
ClipRect backgroundRect = layerFragments.isEmpty() ? ClipRect() : layerFragments[0].backgroundRect;

RegionContextStateSaver regionContextStateSaver(paintingInfo.regionContext);

clipToRect(originalContext, stateSaver, regionContextStateSaver, paintingInfo, behavior, backgroundRect);
}

clipToRect(originalContext, stateSaver, regionContextStateSaver, paintingInfo, behavior, backgroundRect);
m_filters->applyFilterEffect(originalContext);
}

Expand Down
6 changes: 3 additions & 3 deletions Source/WebCore/rendering/RenderLayerFilters.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,9 @@ bool RenderLayerFilters::hasFilterThatShouldBeRestrictedBySecurityOrigin() const
return m_filter && m_filter->hasFilterThatShouldBeRestrictedBySecurityOrigin();
}

bool RenderLayerFilters::needsRedrawSourceImage() const
bool RenderLayerFilters::hasSourceImage() const
{
return m_targetSwitcher && m_targetSwitcher->needsRedrawSourceImage();
return m_targetSwitcher && m_targetSwitcher->hasSourceImage();
}

void RenderLayerFilters::notifyFinished(CachedResource&, const NetworkLoadMetrics&)
Expand Down Expand Up @@ -178,7 +178,7 @@ GraphicsContext* RenderLayerFilters::beginFilterEffect(RenderElement& renderer,

if (!filter.hasFilterThatMovesPixels())
m_repaintRect = dirtyRect;
else if (hasUpdatedBackingStore || needsRedrawSourceImage())
else if (hasUpdatedBackingStore || !hasSourceImage())
m_repaintRect = filterRegion;
else {
m_repaintRect = dirtyRect;
Expand Down
5 changes: 2 additions & 3 deletions Source/WebCore/rendering/RenderLayerFilters.h
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/*
* Copyright (C) 2012 Adobe Systems Incorporated. All rights reserved.
* Copyright (C) 2013-2022 Apple Inc. All rights reserved.
* Copyright (C) 2013-2023 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
Expand Down Expand Up @@ -56,6 +56,7 @@ class RenderLayerFilters final : private CachedSVGDocumentClient {

bool hasFilterThatMovesPixels() const;
bool hasFilterThatShouldBeRestrictedBySecurityOrigin() const;
bool hasSourceImage() const;

void updateReferenceFilterClients(const FilterOperations&);
void removeReferenceFilterClients();
Expand All @@ -73,8 +74,6 @@ class RenderLayerFilters final : private CachedSVGDocumentClient {
void applyFilterEffect(GraphicsContext& destinationContext);

private:
bool needsRedrawSourceImage() const;

void notifyFinished(CachedResource&, const NetworkLoadMetrics&) final;
void resetDirtySourceRect() { m_dirtySourceRect = LayoutRect(); }

Expand Down
2 changes: 1 addition & 1 deletion Source/WebCore/rendering/svg/RenderSVGResourceFilter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ bool RenderSVGResourceFilter::applyResource(RenderElement& renderer, const Rende
}

ASSERT(filterData->targetSwitcher);
if (!filterData->targetSwitcher->needsRedrawSourceImage())
if (filterData->targetSwitcher->hasSourceImage())
return false;

filterData->targetSwitcher->beginDrawSourceImage(*context);
Expand Down

0 comments on commit a73cf05

Please sign in to comment.