Skip to content

Commit

Permalink
Cherry-pick 275091@main (cfa8125). https://bugs.webkit.org/show_bug.c…
Browse files Browse the repository at this point in the history
…gi?id=268537

    Regression(251234@main): [Cairo][GTK][WPE] Darker output of SVG with filters fediffuselighting
    https://bugs.webkit.org/show_bug.cgi?id=268537

    Reviewed by Said Abou-Hallawa and Nikolas Zimmermann.

    Turn off the support for linearRGB color space in case of Cairo (251234@main) causes, that SVG
    with filter, where linearRGB as inputs is demanded, generates darker output.

    The SVG spec says that SourceGraphic has to be in linearRGB color space:
    https://www.w3.org/TR/filter-effects-1/#attr-valuedef-in-sourcegraphic

    In case of Cairo (which operates in SRGB color space) the image source (SourceGraphic)
    should be created in SRGB and before passing it to filters, it should be transformed to
    linearRGB color space.

    * LayoutTests/TestExpectations:
    * LayoutTests/platform/glib/TestExpectations:
    Remove some tests which pass with this change

    * LayoutTests/svg/filters/feConvolveMatrix-clipped.svg:
    * LayoutTests/svg/filters/feGaussianBlur-clipped.svg:
    * LayoutTests/fast/gradients/conic-gradient-extended-stops.html:
    Add possible differences in outputs

    * Source/WebCore/platform/graphics/ImageBuffer.cpp:
    (WebCore::ImageBuffer::filteredNativeImage):
    Pass color space to endDrawSourceImage

    * Source/WebCore/platform/graphics/filters/FEDropShadow.h:
    Force to use SRGB as operating color space for DropShadow in case of Cairo.

    * Source/WebCore/platform/graphics/filters/FilterImageTargetSwitcher.cpp:
    (WebCore::FilterImageTargetSwitcher::endClipAndDrawSourceImage):
    (WebCore::FilterImageTargetSwitcher::endDrawSourceImage):
    Before applying all filter, the source image has to be translated to requested
    color space (only valid for Cairo).

    * Source/WebCore/platform/graphics/filters/FilterTargetSwitcher.h:
    * Source/WebCore/platform/graphics/filters/FilterImageTargetSwitcher.h:
    * Source/WebCore/platform/graphics/filters/FilterStyleTargetSwitcher.h:
    Added color space as additional parameter.

    * Source/WebCore/platform/graphics/filters/FilterStyleTargetSwitcher.cpp:
    (WebCore::FilterStyleTargetSwitcher::endDrawSourceImage):
    WebCore::FilterStyleTargetSwitcher::endDrawSourceImage does not use color space so just ignore it

    * Source/WebCore/rendering/RenderLayerFilters.cpp:
    (WebCore::RenderLayerFilters::applyFilterEffect):
    Just pass DestinationColorSpace::SRGB() to endClipAndDrawSourceImage.

    * Source/WebCore/rendering/svg/legacy/LegacyRenderSVGResourceFilter.cpp:
    (WebCore::LegacyRenderSVGResourceFilter::applyResource):
    In case of Cairo, the sourceImage has to be created with sRGB color space, because
    Cairo operates in sRGB. The linearRGB color space will be passed to TargetSwitcher
    in WebCore::LegacyRenderSVGResourceFilter::postApplyResource and the sourceImage will
    be transformed to linearRGB before all filter applying.
    (WebCore::LegacyRenderSVGResourceFilter::postApplyResource):
    Before applying all filter, the source image has to be translated to requested
    color space.

    * Source/WebCore/svg/graphics/filters/SVGFilter.cpp:
    (WebCore::buildFilterEffectsGraph):
    Create SourceGraphic and SourceAlpha with LinearRGB color space if
    color-interpolation_filter attribute of the Filter has LinearRGB value (only in case of Cairo).

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

Canonical link: https://commits.webkit.org/274313.44@webkitglib/2.44
  • Loading branch information
pgorszkowski-igalia authored and aperezdc committed Mar 10, 2024
1 parent e73c807 commit 5eb5ab6
Show file tree
Hide file tree
Showing 16 changed files with 35 additions and 25 deletions.
1 change: 0 additions & 1 deletion LayoutTests/TestExpectations
Original file line number Diff line number Diff line change
Expand Up @@ -5333,7 +5333,6 @@ imported/w3c/web-platform-tests/css/filter-effects/tainting-feoffset-002.html [
imported/w3c/web-platform-tests/css/filter-effects/tainting-fespecularlighting-002.html [ ImageOnlyFailure ]
imported/w3c/web-platform-tests/css/filter-effects/tainting-fespecularlighting-003.html [ ImageOnlyFailure ]
imported/w3c/web-platform-tests/css/filter-effects/tainting-fetile-002.html [ ImageOnlyFailure ]
imported/w3c/web-platform-tests/css/filter-effects/backdrop-filter-edge-behavior.html [ ImageOnlyFailure ]
imported/w3c/web-platform-tests/css/filter-effects/backdrop-filter-reference-filter.html [ ImageOnlyFailure ]
imported/w3c/web-platform-tests/css/filter-effects/effect-reference-feimage-004.html [ ImageOnlyFailure ]
imported/w3c/web-platform-tests/css/filter-effects/filter-function/filter-function-001.html [ ImageOnlyFailure ]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@

<body>
<svg viewBox="0 0 700 100">
<meta name="fuzzy" content="maxDifference=0-16; totalPixels=0-30000" />
<defs>
<filter id="posterize" filterUnits="objectBoundingBox" primitiveUnits="objectBoundingBox">
<feComponentTransfer color-interpolation-filters="sRGB">
Expand Down
6 changes: 0 additions & 6 deletions LayoutTests/platform/glib/TestExpectations
Original file line number Diff line number Diff line change
Expand Up @@ -1675,7 +1675,6 @@ webkit.org/b/240934 imported/w3c/web-platform-tests/css/filter-effects/effect-re
webkit.org/b/240934 imported/w3c/web-platform-tests/css/filter-effects/root-element-with-opacity-filter-001.html [ ImageOnlyFailure ]

webkit.org/b/264583 imported/w3c/web-platform-tests/css/filter-effects/css-filters-animation-saturate.html [ ImageOnlyFailure ]
webkit.org/b/264583 imported/w3c/web-platform-tests/css/filter-effects/repaint-added-backdrop-filter.html [ ImageOnlyFailure ]
webkit.org/b/264584 imported/w3c/web-platform-tests/fetch/api/request/request-keepalive-quota.html?include=slow-2 [ Failure ]
webkit.org/b/264584 imported/w3c/web-platform-tests/fetch/metadata/generated/css-font-face.sub.tentative.html [ Failure ]
webkit.org/b/264584 imported/w3c/web-platform-tests/fetch/private-network-access/fetch.https.window.html [ Failure ]
Expand Down Expand Up @@ -3290,16 +3289,11 @@ fast/text/line-break-with-locale.html [ ImageOnlyFailure ]
webkit.org/b/225423 fast/canvas/canvas-composite-text-alpha.html [ Failure ]

# Non-sRGB color spaces are not currently supported by the glib ports.
css3/color-filters/svg/color-filter-inline-svg.html [ ImageOnlyFailure ]
css3/filters/reference-filter-color-space.html [ ImageOnlyFailure ]
fast/canvas/canvas-color-space-display-p3.html [ Skip ]
fast/canvas/canvas-color-space-display-p3-ImageData.html [ Skip ]
imported/w3c/web-platform-tests/html/canvas/element/wide-gamut-canvas/ [ Skip ]
imported/w3c/web-platform-tests/html/canvas/element/manual/wide-gamut-canvas/ [ Skip ]
imported/w3c/web-platform-tests/html/canvas/offscreen/wide-gamut-canvas/ [ Skip ]
svg/filters/feCompositeOpaque.html [ ImageOnlyFailure ]
svg/filters/feDiffuseLighting-bottomRightPixel.html [ ImageOnlyFailure ]
svg/filters/feDisplacementMap-filterUnits.svg [ ImageOnlyFailure ]

webkit.org/b/254415 fast/canvas/canvas-drawImage-hdr-video.html [ Failure ]

Expand Down
1 change: 1 addition & 0 deletions LayoutTests/svg/filters/feConvolveMatrix-clipped.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
1 change: 1 addition & 0 deletions LayoutTests/svg/filters/feGaussianBlur-clipped.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
2 changes: 1 addition & 1 deletion Source/WTF/wtf/PlatformEnable.h
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@
#endif

#if !defined(ENABLE_DESTINATION_COLOR_SPACE_LINEAR_SRGB)
#define ENABLE_DESTINATION_COLOR_SPACE_LINEAR_SRGB 0
#define ENABLE_DESTINATION_COLOR_SPACE_LINEAR_SRGB 1
#endif

#if !defined(ENABLE_DRAG_SUPPORT)
Expand Down
2 changes: 1 addition & 1 deletion Source/WebCore/platform/graphics/ImageBuffer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -350,7 +350,7 @@ RefPtr<NativeImage> ImageBuffer::filteredNativeImage(Filter& filter, Function<vo

if (filter.filterRenderingModes().contains(FilterRenderingMode::GraphicsContext)) {
ASSERT(targetSwitcher);
targetSwitcher->endDrawSourceImage(context());
targetSwitcher->endDrawSourceImage(context(), colorSpace());
return copyImageBufferToNativeImage(*this, CopyBackingStore, PreserveResolution::No);
}

Expand Down
4 changes: 4 additions & 0 deletions Source/WebCore/platform/graphics/filters/FEDropShadow.h
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,10 @@ class FEDropShadow : public FilterEffect {

static IntOutsets calculateOutsets(const FloatSize& offset, const FloatSize& stdDeviation);

#if USE(CAIRO)
void setOperatingColorSpace(const DestinationColorSpace&) override { }
#endif

private:
FEDropShadow(float stdX, float stdY, float dx, float dy, const Color& shadowColor, float shadowOpacity, DestinationColorSpace);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,20 +65,29 @@ void FilterImageTargetSwitcher::beginClipAndDrawSourceImage(GraphicsContext& des
}
}

void FilterImageTargetSwitcher::endClipAndDrawSourceImage(GraphicsContext& destinationContext)
void FilterImageTargetSwitcher::endClipAndDrawSourceImage(GraphicsContext& destinationContext, const DestinationColorSpace& colorSpace)
{
if (auto* context = drawingContext(destinationContext))
context->restore();

endDrawSourceImage(destinationContext);
endDrawSourceImage(destinationContext, colorSpace);
}

void FilterImageTargetSwitcher::endDrawSourceImage(GraphicsContext& destinationContext)
void FilterImageTargetSwitcher::endDrawSourceImage(GraphicsContext& destinationContext, const DestinationColorSpace& colorSpace)
{
if (!m_filter)
return;

FilterResults results;
#if USE(CAIRO)
// Cairo operates in SRGB which is why the SourceImage initially is in SRGB color space,
// but before applying all filters it has to be transformed to LinearRGB to comply with
// specification (https://www.w3.org/TR/filter-effects-1/#attr-valuedef-in-sourcegraphic).
if (m_sourceImage)
m_sourceImage->transformToColorSpace(colorSpace);
#else
UNUSED_PARAM(colorSpace);
#endif
destinationContext.drawFilteredImageBuffer(m_sourceImage.get(), m_sourceImageRect, Ref { *m_filter }, m_results ? *m_results : results);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,10 @@ class FilterImageTargetSwitcher final : public FilterTargetSwitcher {
bool hasSourceImage() const override { return m_sourceImage; }

void beginClipAndDrawSourceImage(GraphicsContext& destinationContext, const FloatRect& repaintRect, const FloatRect& clipRect) override;
void endClipAndDrawSourceImage(GraphicsContext& destinationContext) override;
void endClipAndDrawSourceImage(GraphicsContext& destinationContext, const DestinationColorSpace&) override;

void beginDrawSourceImage(GraphicsContext&) override { }
void endDrawSourceImage(GraphicsContext& destinationContext) override;
void endDrawSourceImage(GraphicsContext& destinationContext, const DestinationColorSpace&) override;

RefPtr<ImageBuffer> m_sourceImage;
FloatRect m_sourceImageRect;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ void FilterStyleTargetSwitcher::beginDrawSourceImage(GraphicsContext& destinatio
}
}

void FilterStyleTargetSwitcher::endDrawSourceImage(GraphicsContext& destinationContext)
void FilterStyleTargetSwitcher::endDrawSourceImage(GraphicsContext& destinationContext, const DestinationColorSpace&)
{
for ([[maybe_unused]] auto& filterStyle : makeReversedRange(m_filterStyles)) {
destinationContext.endTransparencyLayer();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,10 @@ class FilterStyleTargetSwitcher : public FilterTargetSwitcher {

private:
void beginClipAndDrawSourceImage(GraphicsContext& destinationContext, const FloatRect& repaintRect, const FloatRect& clipRect) override;
void endClipAndDrawSourceImage(GraphicsContext& destinationContext) override { endDrawSourceImage(destinationContext); }
void endClipAndDrawSourceImage(GraphicsContext& destinationContext, const DestinationColorSpace& colorSpace) override { endDrawSourceImage(destinationContext, colorSpace); }

void beginDrawSourceImage(GraphicsContext& destinationContext) override;
void endDrawSourceImage(GraphicsContext& destinationContext) override;
void endDrawSourceImage(GraphicsContext& destinationContext, const DestinationColorSpace&) override;

FilterStyleVector m_filterStyles;
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,10 @@ class FilterTargetSwitcher {
virtual bool hasSourceImage() const { return false; }

virtual void beginClipAndDrawSourceImage(GraphicsContext& destinationContext, const FloatRect& repaintRect, const FloatRect& clipRect) = 0;
virtual void endClipAndDrawSourceImage(GraphicsContext& destinationContext) = 0;
virtual void endClipAndDrawSourceImage(GraphicsContext& destinationContext, const DestinationColorSpace&) = 0;

virtual void beginDrawSourceImage(GraphicsContext& destinationContext) = 0;
virtual void endDrawSourceImage(GraphicsContext& destinationContext) = 0;
virtual void endDrawSourceImage(GraphicsContext& destinationContext, const DestinationColorSpace&) = 0;

protected:
FilterTargetSwitcher(Filter&);
Expand Down
2 changes: 1 addition & 1 deletion Source/WebCore/rendering/RenderLayerFilters.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ void RenderLayerFilters::applyFilterEffect(GraphicsContext& destinationContext)
LOG_WITH_STREAM(Filters, stream << "\nRenderLayerFilters " << this << " applyFilterEffect");

ASSERT(m_targetSwitcher);
m_targetSwitcher->endClipAndDrawSourceImage(destinationContext);
m_targetSwitcher->endClipAndDrawSourceImage(destinationContext, DestinationColorSpace::SRGB());

LOG_WITH_STREAM(Filters, stream << "RenderLayerFilters " << this << " applyFilterEffect done\n");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -139,10 +139,10 @@ bool LegacyRenderSVGResourceFilter::applyResource(RenderElement& renderer, const

filterData->filter->clampFilterRegionIfNeeded();

#if ENABLE(DESTINATION_COLOR_SPACE_LINEAR_SRGB)
auto colorSpace = DestinationColorSpace::LinearSRGB();
#else
#if USE(CAIRO)
auto colorSpace = DestinationColorSpace::SRGB();
#else
auto colorSpace = DestinationColorSpace::LinearSRGB();
#endif

auto& results = filterData->filter->ensureResults([&]() {
Expand Down Expand Up @@ -215,7 +215,7 @@ void LegacyRenderSVGResourceFilter::postApplyResource(RenderElement& renderer, G

if (filterData.targetSwitcher) {
filterData.state = FilterData::Built;
filterData.targetSwitcher->endDrawSourceImage(*context);
filterData.targetSwitcher->endDrawSourceImage(*context, DestinationColorSpace::LinearSRGB());
}

LOG_WITH_STREAM(Filters, stream << "LegacyRenderSVGResourceFilter " << this << " postApplyResource done\n");
Expand Down
3 changes: 2 additions & 1 deletion Source/WebCore/svg/graphics/filters/SVGFilter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,8 @@ static std::optional<std::tuple<SVGFilterEffectsGraph, FilterEffectGeometryMap>>
if (filterElement.countChildNodes() > maxCountChildNodes)
return std::nullopt;

SVGFilterEffectsGraph graph(SourceGraphic::create(), SourceAlpha::create());
const auto colorSpace = filterElement.colorInterpolation() == ColorInterpolation::LinearRGB ? DestinationColorSpace::LinearSRGB() : DestinationColorSpace::SRGB();
SVGFilterEffectsGraph graph(SourceGraphic::create(colorSpace), SourceAlpha::create(colorSpace));
FilterEffectGeometryMap effectGeometryMap;

for (auto& effectElement : childrenOfType<SVGFilterPrimitiveStandardAttributes>(filterElement)) {
Expand Down

0 comments on commit 5eb5ab6

Please sign in to comment.