commit 86f31f287e09fffeb52e000d3bcf2e65c43d30cb
author Tyler Denniston <tdenniston@google.com> Tue May 04 14:50:49 2021 -0400
committer Skia Commit-Bot <skia-commit-bot@chromium.org> Tue May 04 19:45:55 2021 +0000
tree 8812dd4bb824795f28d3d67c335ae4e5a0d8e94f
parent c1a5a481596299ad287617d520c4855930ff6b94

[svg] Fix filter effect subregion calculation

Somewhat subtle, but we were improperly handling the override of the
default subregion via the 'width' and 'height' <fe*> attributes.

Simple example: when specifying 'x' but not 'width', previously we
would use the overridden x (subregion.fLeft = boundaries.fLeft;), but
were not updating subregion.fRight based on the new fLeft value.

Bug: skia:10841
Change-Id: I309de4f0cf50a413ea6a0e7605926a3eb6cd94e2
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/404205
Commit-Queue: Tyler Denniston <tdenniston@google.com>
Reviewed-by: Florin Malita <fmalita@chromium.org>

modules/svg/src/SkSVGFe.cpp

diff --git a/modules/svg/src/SkSVGFe.cpp b/modules/svg/src/SkSVGFe.cpp
index 37834bc..46a89be 100644
--- a/modules/svg/src/SkSVGFe.cpp
+++ b/modules/svg/src/SkSVGFe.cpp
@@ -59,13 +59,13 @@
     // default subregion is equal to the filter effects region
     // (https://www.w3.org/TR/SVG11/filters.html#FilterEffectsRegion).
     const std::vector<SkSVGFeInputType> inputs = this->getInputs();
-    SkRect subregion;
+    SkRect defaultSubregion;
     if (inputs.empty() || AnyIsStandardInput(fctx, inputs)) {
-        subregion = fctx.filterEffectsRegion();
+        defaultSubregion = fctx.filterEffectsRegion();
     } else {
-        subregion = fctx.filterPrimitiveSubregion(inputs[0]);
+        defaultSubregion = fctx.filterPrimitiveSubregion(inputs[0]);
         for (size_t i = 1; i < inputs.size(); i++) {
-            subregion.join(fctx.filterPrimitiveSubregion(inputs[i]));
+            defaultSubregion.join(fctx.filterPrimitiveSubregion(inputs[i]));
         }
     }
 
@@ -73,20 +73,12 @@
     // If those attributes were given, they override the corresponding attribute of the default
     // filter effect subregion calculated above.
     const SkRect boundaries = this->resolveBoundaries(ctx, fctx);
-    if (fX.isValid()) {
-        subregion.fLeft = boundaries.fLeft;
-    }
-    if (fY.isValid()) {
-        subregion.fTop = boundaries.fTop;
-    }
-    if (fWidth.isValid()) {
-        subregion.fRight = subregion.fLeft + boundaries.width();
-    }
-    if (fHeight.isValid()) {
-        subregion.fBottom = subregion.fTop + boundaries.height();
-    }
 
-    return subregion;
+    // Compute and return the fully resolved subregion.
+    return SkRect::MakeXYWH(fX.isValid() ? boundaries.fLeft : defaultSubregion.fLeft,
+                            fY.isValid() ? boundaries.fTop : defaultSubregion.fTop,
+                            fWidth.isValid() ? boundaries.width() : defaultSubregion.width(),
+                            fHeight.isValid() ? boundaries.height() : defaultSubregion.height());
 }
 
 SkSVGColorspace SkSVGFe::resolveColorspace(const SkSVGRenderContext& ctx,