[svg] Fix text object bounding box resolution
Two issues:
1) we are currently computing object bounding boxes using the
RenderContext of the client resource, which can override presentation
attributes
-> introduce an OBBScope to capture not just the current OBB node but
also its original RenderContext
2) text fragments must use the root <text> bounding box for OBB-relative
units [1]
-> update SkSVGTextFragment::renderText to not override the current
OBBScope; this ensures we're using the root element scope for the
whole text subtree
[1] https://www.w3.org/TR/SVG11/text.html#ObjectBoundingBoxUnitsTextObjects
Bug: skia:11936
Change-Id: Ib48ccd3dc22639de42ea150b881a26dda72fa4bf
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/403037
Commit-Queue: Florin Malita <fmalita@chromium.org>
Commit-Queue: Florin Malita <fmalita@google.com>
Reviewed-by: Tyler Denniston <tdenniston@google.com>
diff --git a/modules/svg/include/SkSVGRenderContext.h b/modules/svg/include/SkSVGRenderContext.h
index 6e2421c..bc7a7e6 100644
--- a/modules/svg/include/SkSVGRenderContext.h
+++ b/modules/svg/include/SkSVGRenderContext.h
@@ -57,12 +57,19 @@
class SkSVGRenderContext {
public:
+ // Captures data required for object bounding box resolution.
+ struct OBBScope {
+ const SkSVGNode* fNode;
+ const SkSVGRenderContext* fCtx;
+ };
+
SkSVGRenderContext(SkCanvas*, const sk_sp<SkFontMgr>&,
const sk_sp<skresources::ResourceProvider>&, const SkSVGIDMapper&,
const SkSVGLengthContext&, const SkSVGPresentationContext&,
- const SkSVGNode*);
+ const OBBScope&);
SkSVGRenderContext(const SkSVGRenderContext&);
SkSVGRenderContext(const SkSVGRenderContext&, SkCanvas*);
+ // Establish a new OBB scope. Normally used when entering a node's render scope.
SkSVGRenderContext(const SkSVGRenderContext&, const SkSVGNode*);
~SkSVGRenderContext();
@@ -171,7 +178,8 @@
// Deferred opacity optimization for leaf nodes.
float fDeferredPaintOpacity = 1;
- const SkSVGNode* fNode;
+ // Current object bounding box scope.
+ const OBBScope fOBBScope;
};
#endif // SkSVGRenderContext_DEFINED
diff --git a/modules/svg/src/SkSVGDOM.cpp b/modules/svg/src/SkSVGDOM.cpp
index 7d0cb8f..dd4a734 100644
--- a/modules/svg/src/SkSVGDOM.cpp
+++ b/modules/svg/src/SkSVGDOM.cpp
@@ -437,7 +437,7 @@
SkSVGLengthContext lctx(fContainerSize);
SkSVGPresentationContext pctx;
fRoot->render(SkSVGRenderContext(canvas, fFontMgr, fResourceProvider, fIDMapper, lctx, pctx,
- nullptr));
+ {nullptr, nullptr}));
}
}
diff --git a/modules/svg/src/SkSVGRenderContext.cpp b/modules/svg/src/SkSVGRenderContext.cpp
index 2358f15..6a98855 100644
--- a/modules/svg/src/SkSVGRenderContext.cpp
+++ b/modules/svg/src/SkSVGRenderContext.cpp
@@ -149,7 +149,7 @@
const SkSVGIDMapper& mapper,
const SkSVGLengthContext& lctx,
const SkSVGPresentationContext& pctx,
- const SkSVGNode* node)
+ const OBBScope& obbs)
: fFontMgr(fmgr)
, fResourceProvider(rp)
, fIDMapper(mapper)
@@ -157,7 +157,7 @@
, fPresentationContext(pctx)
, fCanvas(canvas)
, fCanvasSaveCount(canvas->getSaveCount())
- , fNode(node) {}
+ , fOBBScope(obbs) {}
SkSVGRenderContext::SkSVGRenderContext(const SkSVGRenderContext& other)
: SkSVGRenderContext(other.fCanvas,
@@ -166,7 +166,7 @@
other.fIDMapper,
*other.fLengthContext,
*other.fPresentationContext,
- other.fNode) {}
+ other.fOBBScope) {}
SkSVGRenderContext::SkSVGRenderContext(const SkSVGRenderContext& other, SkCanvas* canvas)
: SkSVGRenderContext(canvas,
@@ -175,7 +175,7 @@
other.fIDMapper,
*other.fLengthContext,
*other.fPresentationContext,
- other.fNode) {}
+ other.fOBBScope) {}
SkSVGRenderContext::SkSVGRenderContext(const SkSVGRenderContext& other, const SkSVGNode* node)
: SkSVGRenderContext(other.fCanvas,
@@ -184,7 +184,7 @@
other.fIDMapper,
*other.fLengthContext,
*other.fPresentationContext,
- node) {}
+ OBBScope{node, this}) {}
SkSVGRenderContext::~SkSVGRenderContext() {
fCanvas->restoreToCount(fCanvasSaveCount);
@@ -396,6 +396,10 @@
// hierarchy. To avoid gross transgressions like leaf node presentation attributes
// leaking into the paint server context, use a pristine presentation context when
// following hrefs.
+ //
+ // Preserve the OBB scope because some paints use object bounding box coords
+ // (e.g. gradient control points), which requires access to the render context
+ // and node being rendered.
SkSVGPresentationContext pctx;
SkSVGRenderContext local_ctx(fCanvas,
fFontMgr,
@@ -403,7 +407,7 @@
fIDMapper,
*fLengthContext,
pctx,
- fNode);
+ fOBBScope);
const auto node = this->findNodeById(paint_selector.iri());
if (!node || !node->asPaint(local_ctx, p.get())) {
@@ -469,11 +473,12 @@
SkSVGRenderContext::OBBTransform
SkSVGRenderContext::transformForCurrentOBB(SkSVGObjectBoundingBoxUnits u) const {
- if (!fNode || u.type() == SkSVGObjectBoundingBoxUnits::Type::kUserSpaceOnUse) {
+ if (!fOBBScope.fNode || u.type() == SkSVGObjectBoundingBoxUnits::Type::kUserSpaceOnUse) {
return {{0,0},{1,1}};
}
+ SkASSERT(fOBBScope.fCtx);
- const auto obb = fNode->objectBoundingBox(*this);
+ const auto obb = fOBBScope.fNode->objectBoundingBox(*fOBBScope.fCtx);
return {{obb.x(), obb.y()}, {obb.width(), obb.height()}};
}
diff --git a/modules/svg/src/SkSVGText.cpp b/modules/svg/src/SkSVGText.cpp
index b405a72..e9f0a37 100644
--- a/modules/svg/src/SkSVGText.cpp
+++ b/modules/svg/src/SkSVGText.cpp
@@ -473,7 +473,9 @@
void SkSVGTextFragment::renderText(const SkSVGRenderContext& ctx, SkSVGTextContext* tctx,
SkSVGXmlSpace xs) const {
- SkSVGRenderContext localContext(ctx, this);
+ // N.B.: unlike regular elements, text fragments do not establish a new OBB scope -- they
+ // always defer to the root <text> element for OBB resolution.
+ SkSVGRenderContext localContext(ctx);
if (this->onPrepareToRender(&localContext)) {
this->onShapeText(localContext, tctx, xs);
diff --git a/modules/svg/tests/Text.cpp b/modules/svg/tests/Text.cpp
index 7308ac8..bcfd034 100644
--- a/modules/svg/tests/Text.cpp
+++ b/modules/svg/tests/Text.cpp
@@ -163,7 +163,7 @@
SkNoDrawCanvas canvas(0, 0);
sk_sp<SkFontMgr> fmgr;
sk_sp<skresources::ResourceProvider> rp;
- const SkSVGRenderContext ctx(&canvas, fmgr, rp, mapper, lctx, pctx, nullptr);
+ const SkSVGRenderContext ctx(&canvas, fmgr, rp, mapper, lctx, pctx, {nullptr, nullptr});
SkSVGTextContext tctx(ctx, mock_cb);
SkSVGTextContext::ScopedPosResolver pa(*a, lctx, &tctx, tst.offseta);