[svgcanvas] Consolidate constant-Y text positions
When SkBaseDevice switched to drawGlyphRunList(), we lost the ability to
detect a) constant-Y text and b) default-positioned text.
As a result, the emitted SVG contains lots of redundant/repeating glyph
positions.
This CL enhances SVGTextBuilder to detect and consolidate constant-Y
glyph positions.
Also restore a useful whitespace unit test.
Change-Id: I50568aef1955f75898ebab41441ad5fe418dac43
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/228563
Commit-Queue: Florin Malita <fmalita@chromium.org>
Reviewed-by: Mike Reed <reed@google.com>
diff --git a/src/svg/SkSVGDevice.cpp b/src/svg/SkSVGDevice.cpp
index 23006e9..b26b001 100644
--- a/src/svg/SkSVGDevice.cpp
+++ b/src/svg/SkSVGDevice.cpp
@@ -869,8 +869,7 @@
class SVGTextBuilder : SkNoncopyable {
public:
SVGTextBuilder(SkPoint origin, const SkGlyphRun& glyphRun)
- : fOrigin(origin)
- , fLastCharWasWhitespace(true) { // start off in whitespace mode to strip all leadingspace
+ : fOrigin(origin) {
auto runSize = glyphRun.runSize();
SkAutoSTArray<64, SkUnichar> unichars(runSize);
SkFontPriv::GlyphsToUnichars(glyphRun.font(), glyphRun.glyphsIDs().data(),
@@ -882,8 +881,8 @@
}
const SkString& text() const { return fText; }
- const SkString& posX() const { return fPosX; }
- const SkString& posY() const { return fPosY; }
+ const SkString& posX() const { return fPosXStr; }
+ const SkString& posY() const { return fHasConstY ? fConstYStr : fPosYStr; }
private:
void appendUnichar(SkUnichar c, SkPoint position) {
@@ -928,22 +927,32 @@
break;
}
- this->advancePos(discardPos, position);
fLastCharWasWhitespace = isWhitespace;
- }
- void advancePos(bool discard, SkPoint position) {
- if (!discard) {
- SkPoint finalPosition = fOrigin + position;
- fPosX.appendf("%.8g, ", finalPosition.x());
- fPosY.appendf("%.8g, ", finalPosition.y());
+ if (discardPos) {
+ return;
+ }
+
+ position += fOrigin;
+ fPosXStr.appendf("%.8g, ", position.fX);
+ fPosYStr.appendf("%.8g, ", position.fY);
+
+ if (fConstYStr.isEmpty()) {
+ fConstYStr = fPosYStr;
+ fConstY = position.fY;
+ } else {
+ fHasConstY &= SkScalarNearlyEqual(fConstY, position.fY);
}
}
const SkPoint fOrigin;
- SkString fText, fPosX, fPosY;
- bool fLastCharWasWhitespace;
+ SkString fText,
+ fPosXStr, fPosYStr,
+ fConstYStr;
+ SkScalar fConstY;
+ bool fLastCharWasWhitespace = true, // start off in whitespace mode to strip leading space
+ fHasConstY = true;
};
void SkSVGDevice::drawGlyphRunAsPath(const SkGlyphRun& glyphRun, const SkPoint& origin,
diff --git a/tests/SVGDeviceTest.cpp b/tests/SVGDeviceTest.cpp
index 0cf993f..e45ba52 100644
--- a/tests/SVGDeviceTest.cpp
+++ b/tests/SVGDeviceTest.cpp
@@ -20,6 +20,7 @@
#include "include/core/SkImage.h"
#include "include/core/SkShader.h"
#include "include/core/SkStream.h"
+#include "include/core/SkTextBlob.h"
#include "include/private/SkTo.h"
#include "include/svg/SkSVGCanvas.h"
#include "include/utils/SkParse.h"
@@ -44,9 +45,6 @@
: nullptr;
}
-#if 0
-Using the new system where devices only gets glyphs causes this to fail because the font has no
-glyph to unichar data.
namespace {
@@ -55,6 +53,7 @@
const SkDOM::Node* root,
const SkPoint& offset,
unsigned scalarsPerPos,
+ const char* txt,
const char* expected) {
if (root == nullptr) {
ERRORF(reporter, "root element not found.");
@@ -72,9 +71,6 @@
REPORTER_ASSERT(reporter, textNode != nullptr);
if (textNode != nullptr) {
REPORTER_ASSERT(reporter, dom.getType(textNode) == SkDOM::kText_Type);
- if (strcmp(expected, dom.getName(textNode)) != 0) {
- SkDebugf("string fail %s == %s\n", expected, dom.getName(textNode));
- }
REPORTER_ASSERT(reporter, strcmp(expected, dom.getName(textNode)) == 0);
}
@@ -83,18 +79,19 @@
const char* x = dom.findAttr(textElem, "x");
REPORTER_ASSERT(reporter, x != nullptr);
if (x != nullptr) {
- int xposCount = (scalarsPerPos < 1) ? 1 : textLen;
+ int xposCount = textLen;
REPORTER_ASSERT(reporter, SkParse::Count(x) == xposCount);
SkAutoTMalloc<SkScalar> xpos(xposCount);
SkParse::FindScalars(x, xpos.get(), xposCount);
if (scalarsPerPos < 1) {
- REPORTER_ASSERT(reporter, xpos[0] == offset.x());
+ // For default-positioned text, we cannot make any assumptions regarding
+ // the first glyph position when the string has leading whitespace (to be stripped).
+ if (txt[0] != ' ' && txt[0] != '\t') {
+ REPORTER_ASSERT(reporter, xpos[0] == offset.x());
+ }
} else {
for (int i = 0; i < xposCount; ++i) {
- if (xpos[i] != SkIntToScalar(expected[i])) {
- SkDebugf("Bad xs %g == %g\n", xpos[i], SkIntToScalar(expected[i]));
- }
REPORTER_ASSERT(reporter, xpos[i] == SkIntToScalar(expected[i]));
}
}
@@ -112,7 +109,7 @@
REPORTER_ASSERT(reporter, ypos[0] == offset.y());
} else {
for (int i = 0; i < yposCount; ++i) {
- REPORTER_ASSERT(reporter, ypos[i] == -SkIntToScalar(expected[i]));
+ REPORTER_ASSERT(reporter, ypos[i] == 150 - SkIntToScalar(expected[i]));
}
}
}
@@ -125,15 +122,14 @@
SkDOM dom;
SkPaint paint;
- SkFont font;
+ SkFont font(ToolUtils::create_portable_typeface());
SkPoint offset = SkPoint::Make(10, 20);
{
- auto svgCanvas = MakeDOMCanvas(&dom);
- svgCanvas->drawSimpleText(txt, len, SkTextEncoding::kUTF8, offset.x(), offset.y(),
- font, paint);
+ MakeDOMCanvas(&dom)->drawSimpleText(txt, len, SkTextEncoding::kUTF8,
+ offset.x(), offset.y(), font, paint);
}
- check_text_node(reporter, dom, dom.finishParsing(), offset, 2, expected);
+ check_text_node(reporter, dom, dom.finishParsing(), offset, 0, txt, expected);
{
SkAutoTMalloc<SkScalar> xpos(len);
@@ -141,27 +137,24 @@
xpos[i] = SkIntToScalar(txt[i]);
}
- auto svgCanvas = MakeDOMCanvas(&dom);
auto blob = SkTextBlob::MakeFromPosTextH(txt, len, &xpos[0], offset.y(), font);
- svgCanvas->drawTextBlob(blob, 0, 0, paint);
+ MakeDOMCanvas(&dom)->drawTextBlob(blob, 0, 0, paint);
}
- check_text_node(reporter, dom, dom.finishParsing(), offset, 2, expected);
+ check_text_node(reporter, dom, dom.finishParsing(), offset, 1, txt, expected);
{
SkAutoTMalloc<SkPoint> pos(len);
for (int i = 0; i < SkToInt(len); ++i) {
- pos[i] = SkPoint::Make(SkIntToScalar(txt[i]), -SkIntToScalar(txt[i]));
+ pos[i] = SkPoint::Make(SkIntToScalar(txt[i]), 150 - SkIntToScalar(txt[i]));
}
- SkXMLParserWriter writer(dom.beginParsing());
- auto blob = SkTextBlob::MakeFromPosTextH(txt, len, &pos[0], font);
- svgCanvas->drawTextBlob(blob, 0, 0, paint);
+ auto blob = SkTextBlob::MakeFromPosText(txt, len, &pos[0], font);
+ MakeDOMCanvas(&dom)->drawTextBlob(blob, 0, 0, paint);
}
- check_text_node(reporter, dom, dom.finishParsing(), offset, 2, expected);
+ check_text_node(reporter, dom, dom.finishParsing(), offset, 2, txt, expected);
}
-}
-
+} // namespace
DEF_TEST(SVGDevice_whitespace_pos, reporter) {
static const struct {
@@ -176,7 +169,7 @@
{ " \t\t abcd", "abcd" },
{ "abcd " , "abcd " }, // we allow one trailing whitespace char
{ "abcd " , "abcd " }, // because it makes no difference and
- { "abcd\t " , "abcd\t" }, // simplifies the implementation
+ { "abcd\t " , "abcd " }, // simplifies the implementation
{ "\t\t \t ab \t\t \t cd \t\t \t ", "ab cd " },
};
@@ -184,8 +177,6 @@
test_whitespace_pos(reporter, tests[i].tst_in, tests[i].tst_out);
}
}
-#endif
-
void SetImageShader(SkPaint* paint, int imageWidth, int imageHeight, SkTileMode xTile,
SkTileMode yTile) {