[SVGCanvas] Clean up public factories
Remove the internal-only/XMLWriter-based factory.
Update SkSVGDevice to always own the xml writer.
The only internal client passing an interesting XMLWriter is
SVGDeviceTest - update to use the device factory directly.
While at it, update the SkSVGDevice factory to return smart pointers
(Create -> Make).
Change-Id: Ibda1ca86ef9fb81ab512822000835ace1af67978
Reviewed-on: https://skia-review.googlesource.com/c/192580
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Florin Malita <fmalita@chromium.org>
diff --git a/dm/DMSrcSink.cpp b/dm/DMSrcSink.cpp
index 9a10933..13b2c75 100644
--- a/dm/DMSrcSink.cpp
+++ b/dm/DMSrcSink.cpp
@@ -1728,11 +1728,10 @@
fPageIndex, pageCount));
}
}
- std::unique_ptr<SkXMLWriter> xmlWriter(new SkXMLStreamWriter(dst));
return src.draw(fPageIndex,
SkSVGCanvas::Make(SkRect::MakeWH(SkIntToScalar(src.size().width()),
SkIntToScalar(src.size().height())),
- xmlWriter.get())
+ dst)
.get());
#else
(void)fPageIndex;
diff --git a/include/svg/SkSVGCanvas.h b/include/svg/SkSVGCanvas.h
index bf1328e..1bf85a0 100644
--- a/include/svg/SkSVGCanvas.h
+++ b/include/svg/SkSVGCanvas.h
@@ -11,14 +11,13 @@
#include "SkCanvas.h"
class SkWStream;
-class SkXMLWriter;
class SK_API SkSVGCanvas {
public:
/**
* Returns a new canvas that will generate SVG commands from its draw calls, and send
- * them to the provided xmlwriter. Ownership of the xmlwriter is not transfered to the canvas,
- * but it must stay valid during the lifetime of the returned canvas.
+ * them to the provided stream. Ownership of the stream is not transfered, and it must
+ * remain valid for the lifetime of the returned canvas.
*
* The canvas may buffer some drawing calls, so the output is not guaranteed to be valid
* or complete until the canvas instance is deleted.
@@ -27,9 +26,6 @@
* SVG element).
*/
static std::unique_ptr<SkCanvas> Make(const SkRect& bounds, SkWStream*);
-
- // Internal only.
- static std::unique_ptr<SkCanvas> Make(const SkRect& bounds, SkXMLWriter*, bool ownsWriter=false);
};
#endif
diff --git a/src/svg/SkSVGCanvas.cpp b/src/svg/SkSVGCanvas.cpp
index c469551..55993af 100644
--- a/src/svg/SkSVGCanvas.cpp
+++ b/src/svg/SkSVGCanvas.cpp
@@ -10,15 +10,12 @@
#include "SkMakeUnique.h"
#include "SkXMLWriter.h"
-std::unique_ptr<SkCanvas> SkSVGCanvas::Make(const SkRect& bounds, SkXMLWriter* writer, bool ownsWriter) {
+std::unique_ptr<SkCanvas> SkSVGCanvas::Make(const SkRect& bounds, SkWStream* writer) {
// TODO: pass full bounds to the device
SkISize size = bounds.roundOut().size();
- sk_sp<SkBaseDevice> device(SkSVGDevice::Create(size, writer, ownsWriter));
- return skstd::make_unique<SkCanvas>(device);
-}
+ auto svgDevice = SkSVGDevice::Make(size, skstd::make_unique<SkXMLStreamWriter>(writer));
-std::unique_ptr<SkCanvas> SkSVGCanvas::Make(const SkRect& bounds, SkWStream* writer) {
- SkXMLStreamWriter *xmlWriter = new SkXMLStreamWriter(writer);
- return Make(bounds, xmlWriter, true);
+ return svgDevice ? skstd::make_unique<SkCanvas>(svgDevice)
+ : nullptr;
}
diff --git a/src/svg/SkSVGDevice.cpp b/src/svg/SkSVGDevice.cpp
index 0635d30..14c0451 100644
--- a/src/svg/SkSVGDevice.cpp
+++ b/src/svg/SkSVGDevice.cpp
@@ -190,9 +190,12 @@
fWriter->startElement(name);
}
- AutoElement(const char name[], SkXMLWriter* writer, ResourceBucket* bucket,
- const MxCp& mc, const SkPaint& paint)
- : fWriter(writer)
+ AutoElement(const char name[], const std::unique_ptr<SkXMLWriter>& writer)
+ : AutoElement(name, writer.get()) {}
+
+ AutoElement(const char name[], const std::unique_ptr<SkXMLWriter>& writer,
+ ResourceBucket* bucket, const MxCp& mc, const SkPaint& paint)
+ : fWriter(writer.get())
, fResourceBucket(bucket) {
Resources res = this->addResources(mc, paint);
@@ -634,22 +637,18 @@
}
}
-SkBaseDevice* SkSVGDevice::Create(const SkISize& size, SkXMLWriter* writer, bool ownsWriter) {
- if (!writer) {
- return nullptr;
- }
-
- return new SkSVGDevice(size, writer, ownsWriter);
+sk_sp<SkBaseDevice> SkSVGDevice::Make(const SkISize& size, std::unique_ptr<SkXMLWriter> writer) {
+ return writer ? sk_sp<SkBaseDevice>(new SkSVGDevice(size, std::move(writer)))
+ : nullptr;
}
-SkSVGDevice::SkSVGDevice(const SkISize& size, SkXMLWriter* writer, bool ownsWriter)
+SkSVGDevice::SkSVGDevice(const SkISize& size, std::unique_ptr<SkXMLWriter> writer)
: INHERITED(SkImageInfo::MakeUnknown(size.fWidth, size.fHeight),
SkSurfaceProps(0, kUnknown_SkPixelGeometry))
- , fWriter(writer)
- , fOwnsWriter(ownsWriter)
+ , fWriter(std::move(writer))
, fResourceBucket(new ResourceBucket)
{
- SkASSERT(writer);
+ SkASSERT(fWriter);
fWriter->writeHeader();
@@ -662,12 +661,7 @@
fRootElement->addAttribute("height", size.height());
}
-SkSVGDevice::~SkSVGDevice() {
- if (fOwnsWriter && fWriter) {
- fRootElement.reset(nullptr);
- delete fWriter;
- }
-}
+SkSVGDevice::~SkSVGDevice() = default;
void SkSVGDevice::drawPaint(const SkPaint& paint) {
AutoElement rect("rect", fWriter, fResourceBucket.get(), MxCp(this), paint);
diff --git a/src/svg/SkSVGDevice.h b/src/svg/SkSVGDevice.h
index 57d2b5c..2d7922a 100644
--- a/src/svg/SkSVGDevice.h
+++ b/src/svg/SkSVGDevice.h
@@ -15,7 +15,7 @@
class SkSVGDevice : public SkClipStackDevice {
public:
- static SkBaseDevice* Create(const SkISize& size, SkXMLWriter* writer, bool ownsWriter=false);
+ static sk_sp<SkBaseDevice> Make(const SkISize& size, std::unique_ptr<SkXMLWriter>);
protected:
void drawPaint(const SkPaint& paint) override;
@@ -42,7 +42,7 @@
const SkPaint&) override;
private:
- SkSVGDevice(const SkISize& size, SkXMLWriter* writer, bool ownsWriter=false);
+ SkSVGDevice(const SkISize& size, std::unique_ptr<SkXMLWriter>);
~SkSVGDevice() override;
struct MxCp;
@@ -51,8 +51,7 @@
class AutoElement;
class ResourceBucket;
- SkXMLWriter* fWriter;
- bool fOwnsWriter;
+ std::unique_ptr<SkXMLWriter> fWriter;
std::unique_ptr<AutoElement> fRootElement;
std::unique_ptr<ResourceBucket> fResourceBucket;
diff --git a/tests/AnnotationTest.cpp b/tests/AnnotationTest.cpp
index 70c9703..d0d273e 100644
--- a/tests/AnnotationTest.cpp
+++ b/tests/AnnotationTest.cpp
@@ -93,9 +93,8 @@
DEF_TEST(Annotation_SvgLink, reporter) {
SkDynamicMemoryWStream outStream;
- std::unique_ptr<SkXMLWriter> xmlWriter(new SkXMLStreamWriter(&outStream));
SkRect bounds = SkRect::MakeIWH(400, 400);
- std::unique_ptr<SkCanvas> canvas = SkSVGCanvas::Make(bounds, xmlWriter.get());
+ std::unique_ptr<SkCanvas> canvas = SkSVGCanvas::Make(bounds, &outStream);
SkRect r = SkRect::MakeXYWH(SkIntToScalar(72), SkIntToScalar(72), SkIntToScalar(288),
SkIntToScalar(72));
@@ -112,9 +111,8 @@
DEF_TEST(Annotation_SvgLinkNamedDestination, reporter) {
SkDynamicMemoryWStream outStream;
- std::unique_ptr<SkXMLWriter> xmlWriter(new SkXMLStreamWriter(&outStream));
SkRect bounds = SkRect::MakeIWH(400, 400);
- std::unique_ptr<SkCanvas> canvas = SkSVGCanvas::Make(bounds, xmlWriter.get());
+ std::unique_ptr<SkCanvas> canvas = SkSVGCanvas::Make(bounds, &outStream);
SkRect r = SkRect::MakeXYWH(SkIntToScalar(72), SkIntToScalar(72), SkIntToScalar(288),
SkIntToScalar(72));
diff --git a/tests/SVGDeviceTest.cpp b/tests/SVGDeviceTest.cpp
index 0a02f7c..9698108 100644
--- a/tests/SVGDeviceTest.cpp
+++ b/tests/SVGDeviceTest.cpp
@@ -19,6 +19,7 @@
#include "SkData.h"
#include "SkImage.h"
#include "SkImageShader.h"
+#include "SkMakeUnique.h"
#include "SkParse.h"
#include "SkShader.h"
#include "SkStream.h"
@@ -30,9 +31,16 @@
#ifdef SK_XML
#include "SkDOM.h"
-#include "SkSVGCanvas.h"
+#include "../src/svg/SkSVGDevice.h"
#include "SkXMLWriter.h"
+static std::unique_ptr<SkCanvas> MakeDOMCanvas(SkDOM* dom) {
+ auto svgDevice = SkSVGDevice::Make(SkISize::Make(100, 100),
+ skstd::make_unique<SkXMLParserWriter>(dom->beginParsing()));
+ return svgDevice ? skstd::make_unique<SkCanvas>(svgDevice)
+ : 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.
@@ -118,8 +126,7 @@
SkPoint offset = SkPoint::Make(10, 20);
{
- SkXMLParserWriter writer(dom.beginParsing());
- std::unique_ptr<SkCanvas> svgCanvas = SkSVGCanvas::Make(SkRect::MakeWH(100, 100), &writer);
+ auto svgCanvas = MakeDOMCanvas(&dom);
svgCanvas->drawSimpleText(txt, len, kUTF8_SkTextEncoding, offset.x(), offset.y(),
font, paint);
}
@@ -131,8 +138,7 @@
xpos[i] = SkIntToScalar(txt[i]);
}
- SkXMLParserWriter writer(dom.beginParsing());
- std::unique_ptr<SkCanvas> svgCanvas = SkSVGCanvas::Make(SkRect::MakeWH(100, 100), &writer);
+ auto svgCanvas = MakeDOMCanvas(&dom);
auto blob = SkTextBlob::MakeFromPosTextH(txt, len, &xpos[0], offset.y(), font);
svgCanvas->drawTextBlob(blob, 0, 0, paint);
}
@@ -231,8 +237,7 @@
int rectWidth, int rectHeight, SkShader::TileMode xTile,
SkShader::TileMode yTile) {
SetImageShader(paint, imageWidth, imageHeight, xTile, yTile);
- SkXMLParserWriter writer(dom->beginParsing());
- std::unique_ptr<SkCanvas> svgCanvas = SkSVGCanvas::Make(SkRect::MakeWH(100, 100), &writer);
+ auto svgCanvas = MakeDOMCanvas(dom);
SkRect bounds{0, 0, SkIntToScalar(rectWidth), SkIntToScalar(rectHeight)};
svgCanvas->drawRect(bounds, *paint);
@@ -357,8 +362,7 @@
SkPaint paint;
paint.setColorFilter(SkColorFilter::MakeModeFilter(SK_ColorRED, SkBlendMode::kSrcIn));
{
- SkXMLParserWriter writer(dom.beginParsing());
- std::unique_ptr<SkCanvas> svgCanvas = SkSVGCanvas::Make(SkRect::MakeWH(100, 100), &writer);
+ auto svgCanvas = MakeDOMCanvas(&dom);
SkRect bounds{0, 0, SkIntToScalar(100), SkIntToScalar(100)};
svgCanvas->drawRect(bounds, paint);
}