Reland "[svg] Perform colorspace conversions for filter effects"
This reverts commit 36acb7b10c67af8c09eacccaa8ffc979efd6dcbc.
Reason for revert: artifacts in 1010102 are expected
Original change's description:
> Revert "[svg] Perform colorspace conversions for filter effects"
>
> This reverts commit a0880eda22cc9fb4db7ebc0b86b5b2170f2d93e5.
>
> Reason for revert: visual artifacts in 10-bit color depth (10-10-10-2)
>
> Original change's description:
> > [svg] Perform colorspace conversions for filter effects
> >
> > A filter effect can optionally be specified to operate in either sRGB
> > or linearRGB, according to the SVG spec:
> >
> > https://www.w3.org/TR/SVG11/painting.html#ColorInterpolationProperties
> >
> > This CL adds any necessary conversion steps (SkColorFilters) while
> > constructing the filter DAG. The default filter effect color space is
> > linearRGB. We should now be passing the filters-gauss-* W3C tests.
> >
> > Specific changes:
> > - Tag filter effect results with their colorspace when storing them in
> > the filter context map
> > - Add an SkColorFolor conversion step as necessary when resolving filter
> > effect inputs
> >
> > Bug: skia:10841
> > Change-Id: Ide12698ea64c4d40f09df93a60718788809086fa
> > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/353078
> > Commit-Queue: Tyler Denniston <tdenniston@google.com>
> > Reviewed-by: Florin Malita <fmalita@chromium.org>
>
> TBR=fmalita@chromium.org,tdenniston@google.com
>
> Change-Id: Id4a33c49643039cfb2d2867a1513e8ee1d7b181a
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug: skia:10841
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/353630
> Reviewed-by: John Stiles <johnstiles@google.com>
> Commit-Queue: John Stiles <johnstiles@google.com>
TBR=fmalita@chromium.org,tdenniston@google.com,johnstiles@google.com
# Not skipping CQ checks because this is a reland.
Bug: skia:10841
Change-Id: Id6d9e01d9b18ebfb6f9a6cb74518ad5cd73ea00a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/353777
Reviewed-by: Tyler Denniston <tdenniston@google.com>
Commit-Queue: Tyler Denniston <tdenniston@google.com>
diff --git a/modules/svg/src/SkSVGFe.cpp b/modules/svg/src/SkSVGFe.cpp
index 66a8592..aa2b66c 100644
--- a/modules/svg/src/SkSVGFe.cpp
+++ b/modules/svg/src/SkSVGFe.cpp
@@ -84,7 +84,9 @@
}
SkSVGColorspace SkSVGFe::resolveColorspace(const SkSVGRenderContext& ctx) const {
- return *ctx.presentationContext().fInherited.fColorInterpolationFilters;
+ constexpr SkSVGColorspace kDefaultCS = SkSVGColorspace::kSRGB;
+ const SkSVGColorspace cs = *ctx.presentationContext().fInherited.fColorInterpolationFilters;
+ return cs == SkSVGColorspace::kAuto ? kDefaultCS : cs;
}
void SkSVGFe::applyProperties(SkSVGRenderContext* ctx) const { this->onPrepareToRender(ctx); }
diff --git a/modules/svg/src/SkSVGFeColorMatrix.cpp b/modules/svg/src/SkSVGFeColorMatrix.cpp
index be07e65..9f45bc8 100644
--- a/modules/svg/src/SkSVGFeColorMatrix.cpp
+++ b/modules/svg/src/SkSVGFeColorMatrix.cpp
@@ -91,9 +91,10 @@
sk_sp<SkImageFilter> SkSVGFeColorMatrix::onMakeImageFilter(const SkSVGRenderContext& ctx,
const SkSVGFilterContext& fctx) const {
- return SkImageFilters::ColorFilter(SkColorFilters::Matrix(makeMatrixForType()),
- fctx.resolveInput(ctx, this->getIn()),
- this->resolveFilterSubregion(ctx, fctx));
+ return SkImageFilters::ColorFilter(
+ SkColorFilters::Matrix(makeMatrixForType()),
+ fctx.resolveInput(ctx, this->getIn(), this->resolveColorspace(ctx)),
+ this->resolveFilterSubregion(ctx, fctx));
}
template <> bool SkSVGAttributeParser::parse(SkSVGFeColorMatrixValues* values) {
diff --git a/modules/svg/src/SkSVGFeComposite.cpp b/modules/svg/src/SkSVGFeComposite.cpp
index 5c88492..d21b456 100644
--- a/modules/svg/src/SkSVGFeComposite.cpp
+++ b/modules/svg/src/SkSVGFeComposite.cpp
@@ -47,8 +47,9 @@
sk_sp<SkImageFilter> SkSVGFeComposite::onMakeImageFilter(const SkSVGRenderContext& ctx,
const SkSVGFilterContext& fctx) const {
const SkRect cropRect = this->resolveFilterSubregion(ctx, fctx);
- const sk_sp<SkImageFilter> background = fctx.resolveInput(ctx, fIn2);
- const sk_sp<SkImageFilter> foreground = fctx.resolveInput(ctx, this->getIn());
+ const SkSVGColorspace colorspace = this->resolveColorspace(ctx);
+ const sk_sp<SkImageFilter> background = fctx.resolveInput(ctx, fIn2, colorspace);
+ const sk_sp<SkImageFilter> foreground = fctx.resolveInput(ctx, this->getIn(), colorspace);
if (fOperator == SkSVGFeCompositeOperator::kArithmetic) {
constexpr bool enforcePMColor = true;
return SkImageFilters::Arithmetic(
diff --git a/modules/svg/src/SkSVGFeGaussianBlur.cpp b/modules/svg/src/SkSVGFeGaussianBlur.cpp
index 9e4407b..6ee1399 100644
--- a/modules/svg/src/SkSVGFeGaussianBlur.cpp
+++ b/modules/svg/src/SkSVGFeGaussianBlur.cpp
@@ -30,7 +30,7 @@
}
return SkImageFilters::Blur(sigmaX, sigmaY,
- fctx.resolveInput(ctx, this->getIn()),
+ fctx.resolveInput(ctx, this->getIn(), this->resolveColorspace(ctx)),
this->resolveFilterSubregion(ctx, fctx));
}
diff --git a/modules/svg/src/SkSVGFilter.cpp b/modules/svg/src/SkSVGFilter.cpp
index 917c628..13de852 100644
--- a/modules/svg/src/SkSVGFilter.cpp
+++ b/modules/svg/src/SkSVGFilter.cpp
@@ -5,6 +5,7 @@
* found in the LICENSE file.
*/
+#include "include/core/SkColorFilter.h"
#include "include/effects/SkImageFilters.h"
#include "modules/svg/include/SkSVGFe.h"
#include "modules/svg/include/SkSVGFilter.h"
@@ -46,6 +47,7 @@
sk_sp<SkImageFilter> SkSVGFilter::buildFilterDAG(const SkSVGRenderContext& ctx) const {
sk_sp<SkImageFilter> filter;
SkSVGFilterContext fctx(resolveFilterRegion(ctx), fPrimitiveUnits);
+ SkSVGColorspace cs = SkSVGColorspace::kSRGB;
for (const auto& child : fChildren) {
if (!SkSVGFe::IsFilterEffect(child)) {
continue;
@@ -62,14 +64,19 @@
feNode.applyProperties(&localCtx);
// TODO: there are specific composition rules that need to be followed
- // TODO: perform colorspace conversions depending on 'color-interpolation-filters' setting
- // of the current node and its inputs.
+ cs = feNode.resolveColorspace(ctx);
filter = feNode.makeImageFilter(localCtx, fctx);
if (!feResultType.isEmpty()) {
- fctx.registerResult(feResultType, filter, feNode.resolveFilterSubregion(localCtx, fctx));
+ fctx.registerResult(
+ feResultType, filter, feNode.resolveFilterSubregion(localCtx, fctx), cs);
}
}
+ // Convert to final destination colorspace
+ if (cs != SkSVGColorspace::kSRGB) {
+ filter = SkImageFilters::ColorFilter(SkColorFilters::LinearToSRGBGamma(), filter);
+ }
+
return filter;
}
diff --git a/modules/svg/src/SkSVGFilterContext.cpp b/modules/svg/src/SkSVGFilterContext.cpp
index a7d1cbb..aa15341 100644
--- a/modules/svg/src/SkSVGFilterContext.cpp
+++ b/modules/svg/src/SkSVGFilterContext.cpp
@@ -5,15 +5,33 @@
* found in the LICENSE file.
*/
+#include "include/core/SkColorFilter.h"
#include "include/effects/SkImageFilters.h"
#include "modules/svg/include/SkSVGFilterContext.h"
#include "modules/svg/include/SkSVGNode.h"
#include "modules/svg/include/SkSVGRenderContext.h"
#include "modules/svg/include/SkSVGTypes.h"
-sk_sp<SkImageFilter> SkSVGFilterContext::findResultById(const SkSVGStringType& id) const {
- const Result* res = fResults.find(id);
- return res ? res->fImageFilter : nullptr;
+namespace {
+
+sk_sp<SkImageFilter> ConvertFilterColorspace(sk_sp<SkImageFilter>&& input,
+ SkSVGColorspace src,
+ SkSVGColorspace dst) {
+ if (src == dst) {
+ return std::move(input);
+ } else if (src == SkSVGColorspace::kSRGB && dst == SkSVGColorspace::kLinearRGB) {
+ return SkImageFilters::ColorFilter(SkColorFilters::SRGBToLinearGamma(), input);
+ } else {
+ SkASSERT(src == SkSVGColorspace::kLinearRGB && dst == SkSVGColorspace::kSRGB);
+ return SkImageFilters::ColorFilter(SkColorFilters::LinearToSRGBGamma(), input);
+ }
+}
+
+} // namespace
+
+const SkSVGFilterContext::Result* SkSVGFilterContext::findResultById(
+ const SkSVGStringType& id) const {
+ return fResults.find(id);
}
const SkRect& SkSVGFilterContext::filterPrimitiveSubregion(const SkSVGFeInputType& input) const {
@@ -25,18 +43,24 @@
void SkSVGFilterContext::registerResult(const SkSVGStringType& id,
const sk_sp<SkImageFilter>& result,
- const SkRect& subregion) {
+ const SkRect& subregion,
+ SkSVGColorspace resultColorspace) {
SkASSERT(!id.isEmpty());
- fResults[id] = {result, subregion};
+ fResults[id] = {result, subregion, resultColorspace};
}
sk_sp<SkImageFilter> SkSVGFilterContext::resolveInput(const SkSVGRenderContext& ctx,
- const SkSVGFeInputType& inputType) const {
+ const SkSVGFeInputType& inputType,
+ SkSVGColorspace colorspace) const {
+ SkSVGColorspace inputCS = SkSVGColorspace::kSRGB;
+ sk_sp<SkImageFilter> result;
switch (inputType.type()) {
case SkSVGFeInputType::Type::kSourceGraphic:
- return nullptr;
+ // Do nothing.
+ break;
case SkSVGFeInputType::Type::kFillPaint:
- return SkImageFilters::Paint(*ctx.fillPaint());
+ result = SkImageFilters::Paint(*ctx.fillPaint());
+ break;
case SkSVGFeInputType::Type::kStrokePaint: {
// The paint filter doesn't handle stroke paints properly, so convert to fill for
// simplicity.
@@ -44,12 +68,21 @@
// requires some extra work to handle all paint features (gradients, etc).
SkPaint p = *ctx.strokePaint();
p.setStyle(SkPaint::kFill_Style);
- return SkImageFilters::Paint(p);
+ result = SkImageFilters::Paint(p);
+ break;
}
- case SkSVGFeInputType::Type::kFilterPrimitiveReference:
- return findResultById(inputType.id());
+ case SkSVGFeInputType::Type::kFilterPrimitiveReference: {
+ const Result* res = findResultById(inputType.id());
+ if (res) {
+ result = res->fImageFilter;
+ inputCS = res->fColorspace;
+ }
+ break;
+ }
default:
SkDebugf("unhandled filter input type %d\n", inputType.type());
return nullptr;
}
+
+ return ConvertFilterColorspace(std::move(result), inputCS, colorspace);
}