[svg] Optional ellipse radii
SVG 2 introduced optional rx/ry semantics for <ellipse> elements [1],
similar to existing <rect> radii bahavior.
In addition to enabling the new ellipse semantics, this change also
tweaks the current rect radii logic to correctly resolve percentages
against the specified dimension (instead of the dimension they are being
used for).
[1] https://www.w3.org/TR/SVG2/shapes.html#EllipseElement
Bug: b/336711946
Change-Id: I85e98e23dd93fd70be23fc6b9b5e9bbc663e328c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/845358
Reviewed-by: Ben Wagner <bungeman@google.com>
Commit-Queue: Florin Malita <fmalita@google.com>
diff --git a/modules/svg/include/SkSVGEllipse.h b/modules/svg/include/SkSVGEllipse.h
index 7ec3596..eb2eb51 100644
--- a/modules/svg/include/SkSVGEllipse.h
+++ b/modules/svg/include/SkSVGEllipse.h
@@ -19,8 +19,9 @@
SVG_ATTR(Cx, SkSVGLength, SkSVGLength(0))
SVG_ATTR(Cy, SkSVGLength, SkSVGLength(0))
- SVG_ATTR(Rx, SkSVGLength, SkSVGLength(0))
- SVG_ATTR(Ry, SkSVGLength, SkSVGLength(0))
+
+ SVG_OPTIONAL_ATTR(Rx, SkSVGLength)
+ SVG_OPTIONAL_ATTR(Ry, SkSVGLength)
protected:
bool parseAndSetAttribute(const char*, const char*) override;
diff --git a/modules/svg/src/BUILD.bazel b/modules/svg/src/BUILD.bazel
index d4c4570..7531381 100644
--- a/modules/svg/src/BUILD.bazel
+++ b/modules/svg/src/BUILD.bazel
@@ -11,7 +11,10 @@
# This group is exported as //modules/svg/svg.gni:skia_svg_sources.
skia_filegroup(
name = "private_hdrs",
- srcs = ["SkSVGTextPriv.h"],
+ srcs = [
+ "SkSVGRectPriv.h",
+ "SkSVGTextPriv.h",
+ ],
visibility = ["//modules/svg:__pkg__"],
)
diff --git a/modules/svg/src/SkSVGEllipse.cpp b/modules/svg/src/SkSVGEllipse.cpp
index d62d7e9..33a8f69 100644
--- a/modules/svg/src/SkSVGEllipse.cpp
+++ b/modules/svg/src/SkSVGEllipse.cpp
@@ -8,7 +8,8 @@
#include "include/core/SkCanvas.h"
#include "modules/svg/include/SkSVGEllipse.h"
#include "modules/svg/include/SkSVGRenderContext.h"
-#include "modules/svg/include/SkSVGValue.h"
+#include "modules/svg/include/SkSVGTypes.h"
+#include "modules/svg/src/SkSVGRectPriv.h"
SkSVGEllipse::SkSVGEllipse() : INHERITED(SkSVGTag::kEllipse) {}
@@ -23,9 +24,15 @@
SkRect SkSVGEllipse::resolve(const SkSVGLengthContext& lctx) const {
const auto cx = lctx.resolve(fCx, SkSVGLengthContext::LengthType::kHorizontal);
const auto cy = lctx.resolve(fCy, SkSVGLengthContext::LengthType::kVertical);
- const auto rx = lctx.resolve(fRx, SkSVGLengthContext::LengthType::kHorizontal);
- const auto ry = lctx.resolve(fRy, SkSVGLengthContext::LengthType::kVertical);
+ // https://www.w3.org/TR/SVG2/shapes.html#EllipseElement
+ //
+ // An auto value for either rx or ry is converted to a used value, following the rules given
+ // above for rectangles (but without any clamping based on width or height).
+ const auto [ rx, ry ] = ResolveOptionalRadii(fRx, fRy, lctx);
+
+ // A computed value of zero for either dimension, or a computed value of auto for both
+ // dimensions, disables rendering of the element.
return (rx > 0 && ry > 0)
? SkRect::MakeXYWH(cx - rx, cy - ry, rx * 2, ry * 2)
: SkRect::MakeEmpty();
diff --git a/modules/svg/src/SkSVGRect.cpp b/modules/svg/src/SkSVGRect.cpp
index 15790a9..46f3d9c 100644
--- a/modules/svg/src/SkSVGRect.cpp
+++ b/modules/svg/src/SkSVGRect.cpp
@@ -12,7 +12,40 @@
#include "include/core/SkRect.h"
#include "modules/svg/include/SkSVGRect.h"
#include "modules/svg/include/SkSVGRenderContext.h"
-#include "modules/svg/include/SkSVGValue.h"
+#include "modules/svg/src/SkSVGRectPriv.h"
+
+std::tuple<float, float> ResolveOptionalRadii(const SkTLazy<SkSVGLength>& opt_rx,
+ const SkTLazy<SkSVGLength>& opt_ry,
+ const SkSVGLengthContext& lctx) {
+ // https://www.w3.org/TR/SVG2/shapes.html#RectElement
+ //
+ // The used values for rx and ry are determined from the computed values by following these
+ // steps in order:
+ //
+ // 1. If both rx and ry have a computed value of auto (since auto is the initial value for both
+ // properties, this will also occur if neither are specified by the author or if all
+ // author-supplied values are invalid), then the used value of both rx and ry is 0.
+ // (This will result in square corners.)
+ // 2. Otherwise, convert specified values to absolute values as follows:
+ // 1. If rx is set to a length value or a percentage, but ry is auto, calculate an absolute
+ // length equivalent for rx, resolving percentages against the used width of the
+ // rectangle; the absolute value for ry is the same.
+ // 2. If ry is set to a length value or a percentage, but rx is auto, calculate the absolute
+ // length equivalent for ry, resolving percentages against the used height of the
+ // rectangle; the absolute value for rx is the same.
+ // 3. If both rx and ry were set to lengths or percentages, absolute values are generated
+ // individually, resolving rx percentages against the used width, and resolving ry
+ // percentages against the used height.
+ const float rx = opt_rx.isValid()
+ ? lctx.resolve(*opt_rx, SkSVGLengthContext::LengthType::kHorizontal)
+ : 0;
+ const float ry = opt_ry.isValid()
+ ? lctx.resolve(*opt_ry, SkSVGLengthContext::LengthType::kVertical)
+ : 0;
+
+ return std::make_tuple(opt_rx.isValid() ? rx : ry,
+ opt_ry.isValid() ? ry : rx);
+}
SkSVGRect::SkSVGRect() : INHERITED(SkSVGTag::kRect) {}
@@ -28,38 +61,20 @@
SkRRect SkSVGRect::resolve(const SkSVGLengthContext& lctx) const {
const auto rect = lctx.resolveRect(fX, fY, fWidth, fHeight);
+ const auto [ rx, ry ] = ResolveOptionalRadii(fRx, fRy, lctx);
- // https://www.w3.org/TR/SVG11/shapes.html#RectElementRXAttribute:
- //
- // - Let rx and ry be length values.
- // - If neither ‘rx’ nor ‘ry’ are properly specified, then set both rx and ry to 0.
- // - Otherwise, if a properly specified value is provided for ‘rx’, but not for ‘ry’,
- // then set both rx and ry to the value of ‘rx’.
- // - Otherwise, if a properly specified value is provided for ‘ry’, but not for ‘rx’,
- // then set both rx and ry to the value of ‘ry’.
- // - Otherwise, both ‘rx’ and ‘ry’ were specified properly. Set rx to the value of ‘rx’
- // and ry to the value of ‘ry’.
- // - If rx is greater than half of ‘width’, then set rx to half of ‘width’.
- // - If ry is greater than half of ‘height’, then set ry to half of ‘height’.
- // - The effective values of ‘rx’ and ‘ry’ are rx and ry, respectively.
- //
- auto radii = [this]() {
- return fRx.isValid()
- ? fRy.isValid()
- ? std::make_tuple(*fRx, *fRy)
- : std::make_tuple(*fRx, *fRx)
- : fRy.isValid()
- ? std::make_tuple(*fRy, *fRy)
- : std::make_tuple(SkSVGLength(0), SkSVGLength(0));
- };
+ // https://www.w3.org/TR/SVG2/shapes.html#RectElement
+ // ...
+ // 3. Finally, apply clamping to generate the used values:
+ // 1. If the absolute rx (after the above steps) is greater than half of the used width,
+ // then the used value of rx is half of the used width.
+ // 2. If the absolute ry (after the above steps) is greater than half of the used height,
+ // then the used value of ry is half of the used height.
+ // 3. Otherwise, the used values of rx and ry are the absolute values computed previously.
- const auto [ rxlen, rylen ] = radii();
- const auto rx = std::min(lctx.resolve(rxlen, SkSVGLengthContext::LengthType::kHorizontal),
- rect.width() / 2),
- ry = std::min(lctx.resolve(rylen, SkSVGLengthContext::LengthType::kVertical),
- rect.height() / 2);
-
- return SkRRect::MakeRectXY(rect, rx, ry);
+ return SkRRect::MakeRectXY(rect,
+ std::min(rx, rect.width() / 2),
+ std::min(ry, rect.height() / 2));
}
void SkSVGRect::onDraw(SkCanvas* canvas, const SkSVGLengthContext& lctx,
diff --git a/modules/svg/src/SkSVGRectPriv.h b/modules/svg/src/SkSVGRectPriv.h
new file mode 100644
index 0000000..9326311
--- /dev/null
+++ b/modules/svg/src/SkSVGRectPriv.h
@@ -0,0 +1,22 @@
+/*
+ * Copyright 2024 Google Inc.
+ *
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+#ifndef SkSVGRectPriv_DEFINED
+#define SkSVGRectPriv_DEFINED
+
+#include <tuple>
+
+#include "src/base/SkTLazy.h"
+
+class SkSVGLength;
+class SkSVGLengthContext;
+
+std::tuple<float, float> ResolveOptionalRadii(const SkTLazy<SkSVGLength>& rx,
+ const SkTLazy<SkSVGLength>& ry,
+ const SkSVGLengthContext&);
+
+#endif // SkSVGRectPriv_DEFINED
diff --git a/modules/svg/svg.gni b/modules/svg/svg.gni
index 123cd28..98556c6 100644
--- a/modules/svg/svg.gni
+++ b/modules/svg/svg.gni
@@ -98,6 +98,7 @@
"$_modules/svg/src/SkSVGPoly.cpp",
"$_modules/svg/src/SkSVGRadialGradient.cpp",
"$_modules/svg/src/SkSVGRect.cpp",
+ "$_modules/svg/src/SkSVGRectPriv.h",
"$_modules/svg/src/SkSVGRenderContext.cpp",
"$_modules/svg/src/SkSVGSVG.cpp",
"$_modules/svg/src/SkSVGShape.cpp",
