fix build / refactor backends

0. Most importantly this fixes the dumb mistake I made in the
   previous commit: not all backends implemented destroySurface.
1. Both Backend and Pdf/SvgBackend were cleaning up memory on exit.
   This responsibility should not be unclear; let's just do it all
   in the child class, since PdfBackend and SvgBackend have other
   stuff to cleanup. This is all still less code.
2. The only reason to destroy the surface is if it's dirty from
   e.g. setWidth (this is referring to isSurfaceValid)
3. Make createSurface idempotent. This allows us to merge it with
   getSurface() and makes it safer. Call it ensureSurface.

Author: chearon

src/Canvas.cc

@@ -131,13 +131,13 @@ Canvas::Canvas(const Napi::CallbackInfo& info) : Napi::ObjectWrap<Canvas>(info),
     if (instance.IsJust()) backend = ImageBackend::Unwrap(jsBackend = instance.Unwrap());
   }
 
+  backend->setCanvas(this);
+
   if (!backend->isSurfaceValid()) {
     Napi::Error::New(env, backend->getError()).ThrowAsJavaScriptException();
     return;
   }
 
-  backend->setCanvas(this);
-
   // Note: the backend gets destroyed when the jsBackend is GC'd. The cleaner
   // way would be to only store the jsBackend and unwrap it when the c++ ref is
   // needed, but that's slower and a burden. The _backend might be null if we
@@ -908,7 +908,7 @@ Canvas::resurface(Napi::Object This) {
 
   if (This.Get("context").UnwrapTo(&context) && context.IsObject()) {
     backend()->destroySurface();
-    backend()->createSurface();
+    backend()->ensureSurface();
     // Reset context
     Context2d *context2d = Context2d::Unwrap(context.As<Napi::Object>());
     cairo_t *prev = context2d->context();

src/Canvas.h

@@ -76,7 +76,7 @@ class Canvas : public Napi::ObjectWrap<Canvas> {
     static PangoFontDescription *ResolveFontDescription(const PangoFontDescription *desc);
 
     DLL_PUBLIC inline Backend* backend() { return _backend; }
-    DLL_PUBLIC inline cairo_surface_t* surface(){ return backend()->getSurface(); }
+    DLL_PUBLIC inline cairo_surface_t* surface(){ return backend()->ensureSurface(); }
     cairo_t* createCairoContext();
 
     DLL_PUBLIC inline uint8_t *data(){ return cairo_image_surface_get_data(surface()); }

src/backend/Backend.cc

@@ -11,31 +11,12 @@ Backend::Backend(std::string name, Napi::CallbackInfo& info) : name(name), env(i
   this->height = height;
 }
 
-Backend::~Backend()
-{
-  Backend::destroySurface();
-}
-
 void Backend::setCanvas(Canvas* _canvas)
 {
   this->canvas = _canvas;
 }
 
 
-DLL_PUBLIC cairo_surface_t* Backend::getSurface() {
-  if (!surface) createSurface();
-  return surface;
-}
-
-void Backend::destroySurface()
-{
-  if(this->surface)
-  {
-    cairo_surface_destroy(this->surface);
-    this->surface = NULL;
-  }
-}
-
 
 std::string Backend::getName()
 {
@@ -50,7 +31,6 @@ void Backend::setWidth(int width_)
 {
   this->destroySurface();
   this->width = width_;
-  this->createSurface();
 }
 
 int Backend::getHeight()
@@ -61,23 +41,18 @@ void Backend::setHeight(int height_)
 {
   this->destroySurface();
   this->height = height_;
-  this->createSurface();
 }
 
-bool Backend::isSurfaceValid(){
-  bool hadSurface = surface != NULL;
+bool Backend::isSurfaceValid() {
   bool isValid = true;
 
-  cairo_status_t status = cairo_surface_status(getSurface());
+  cairo_status_t status = cairo_surface_status(ensureSurface());
 
   if (status != CAIRO_STATUS_SUCCESS) {
     error = cairo_status_to_string(status);
     isValid = false;
   }
 
-  if (!hadSurface)
-    destroySurface();
-
   return isValid;
 }
 

src/backend/Backend.h

@@ -17,22 +17,17 @@ class Backend
   protected:
     int width;
     int height;
-    cairo_surface_t* surface = nullptr;
     Canvas* canvas = nullptr;
 
     Backend(std::string name, Napi::CallbackInfo& info);
 
   public:
     Napi::Env env;
 
-    virtual ~Backend();
-
     void setCanvas(Canvas* canvas);
 
-    virtual cairo_surface_t* createSurface() = 0;
-
-    DLL_PUBLIC cairo_surface_t* getSurface();
-    virtual void destroySurface();
+    virtual cairo_surface_t* ensureSurface() = 0;
+    virtual void destroySurface() = 0;
 
     DLL_PUBLIC std::string getName();
 

src/backend/ImageBackend.cc

@@ -3,8 +3,10 @@
 #include <napi.h>
 #include <cassert>
 
-ImageBackend::ImageBackend(Napi::CallbackInfo& info) : Napi::ObjectWrap<ImageBackend>(info), Backend("image", info)
-{
+ImageBackend::ImageBackend(Napi::CallbackInfo& info) : Napi::ObjectWrap<ImageBackend>(info), Backend("image", info) {}
+
+ImageBackend::~ImageBackend() {
+    destroySurface();
 }
 
 // This returns an approximate value only, suitable for
@@ -29,11 +31,12 @@ int32_t ImageBackend::approxBytesPerPixel() {
   }
 }
 
-cairo_surface_t* ImageBackend::createSurface() {
-  assert(!surface);
-  surface = cairo_image_surface_create(format, width, height);
-  assert(surface);
-  Napi::MemoryManagement::AdjustExternalMemory(env, approxBytesPerPixel() * width * height);
+cairo_surface_t* ImageBackend::ensureSurface() {
+  if (!surface) {
+    surface = cairo_image_surface_create(format, width, height);
+    assert(surface);
+    Napi::MemoryManagement::AdjustExternalMemory(env, approxBytesPerPixel() * width * height);
+  }
   return surface;
 }
 
@@ -50,7 +53,8 @@ cairo_format_t ImageBackend::getFormat() {
 }
 
 void ImageBackend::setFormat(cairo_format_t _format) {
-	this->format = _format;
+    this->destroySurface();
+    this->format = _format;
 }
 
 Napi::FunctionReference ImageBackend::constructor;

src/backend/ImageBackend.h

@@ -6,12 +6,14 @@
 class ImageBackend : public Napi::ObjectWrap<ImageBackend>, public Backend
 {
   private:
-    cairo_surface_t* createSurface();
+    cairo_surface_t* ensureSurface();
     void destroySurface();
     cairo_format_t format = DEFAULT_FORMAT;
+    cairo_surface_t* surface = nullptr;
 
   public:
     ImageBackend(Napi::CallbackInfo& info);
+    ~ImageBackend();
 
     cairo_format_t getFormat();
     void setFormat(cairo_format_t format);

src/backend/PdfBackend.cc

@@ -5,26 +5,29 @@
 #include "../Canvas.h"
 #include "../closure.h"
 
-PdfBackend::PdfBackend(Napi::CallbackInfo& info) : Napi::ObjectWrap<PdfBackend>(info), Backend("pdf", info) {
-  PdfBackend::createSurface();
-}
+PdfBackend::PdfBackend(Napi::CallbackInfo& info) : Napi::ObjectWrap<PdfBackend>(info), Backend("pdf", info) {}
 
 PdfBackend::~PdfBackend() {
-  cairo_surface_finish(surface);
-  if (_closure) delete _closure;
   destroySurface();
 }
 
-cairo_surface_t* PdfBackend::createSurface() {
-  if (!_closure) _closure = new PdfSvgClosure(canvas);
-  surface = cairo_pdf_surface_create_for_stream(PdfSvgClosure::writeVec, _closure, width, height);
+cairo_surface_t* PdfBackend::ensureSurface() {
+  if (!surface) {
+    _closure = new PdfSvgClosure(canvas);
+    surface = cairo_pdf_surface_create_for_stream(PdfSvgClosure::writeVec, _closure, width, height);
+  }
   return surface;
 }
 
-cairo_surface_t* PdfBackend::recreateSurface() {
-  cairo_pdf_surface_set_size(surface, width, height);
-
-  return surface;
+void PdfBackend::destroySurface() {
+  if (surface) {
+    cairo_surface_destroy(surface);
+    surface = nullptr;
+    if (_closure) {
+      delete _closure;
+      _closure = nullptr;
+    }
+  }
 }
 
 void

src/backend/PdfBackend.h

@@ -7,8 +7,9 @@
 class PdfBackend : public Napi::ObjectWrap<PdfBackend>, public Backend
 {
   private:
-    cairo_surface_t* createSurface();
-    cairo_surface_t* recreateSurface();
+    cairo_surface_t* ensureSurface();
+    void destroySurface();
+    cairo_surface_t* surface = nullptr;
 
   public:
     PdfSvgClosure* _closure = NULL;

src/backend/SvgBackend.cc

@@ -9,36 +9,32 @@
 
 using namespace Napi;
 
-SvgBackend::SvgBackend(Napi::CallbackInfo& info) : Napi::ObjectWrap<SvgBackend>(info), Backend("svg", info) {
-  SvgBackend::createSurface();
-}
+SvgBackend::SvgBackend(Napi::CallbackInfo& info) : Napi::ObjectWrap<SvgBackend>(info), Backend("svg", info) {}
 
 SvgBackend::~SvgBackend() {
-  cairo_surface_finish(surface);
-  if (_closure) {
-    delete _closure;
-    _closure = nullptr;
-  }
   destroySurface();
 }
 
-cairo_surface_t* SvgBackend::createSurface() {
-  assert(!_closure);
-  _closure = new PdfSvgClosure(canvas);
-  surface = cairo_svg_surface_create_for_stream(PdfSvgClosure::writeVec, _closure, width, height);
+cairo_surface_t* SvgBackend::ensureSurface() {
+  if (!surface) {
+    assert(!_closure);
+     _closure = new PdfSvgClosure(canvas);
+    surface = cairo_svg_surface_create_for_stream(PdfSvgClosure::writeVec, _closure, width, height);
+  }
   return surface;
 }
 
-cairo_surface_t* SvgBackend::recreateSurface() {
-  cairo_surface_finish(surface);
-  delete _closure;
-  _closure = nullptr;
-  cairo_surface_destroy(surface);
-
-  return createSurface();
+void SvgBackend::destroySurface() {
+  if (surface) {
+    cairo_surface_destroy(surface);
+    surface = nullptr;
+    if (_closure) {
+      delete _closure;
+      _closure = nullptr;
+    }
+  }
  }
 
-
 void
 SvgBackend::Initialize(Napi::Object target) {
   Napi::Env env = target.Env();

src/backend/SvgBackend.h

@@ -7,8 +7,9 @@
 class SvgBackend : public Napi::ObjectWrap<SvgBackend>, public Backend
 {
   private:
-    cairo_surface_t* createSurface();
-    cairo_surface_t* recreateSurface();
+    cairo_surface_t* ensureSurface();
+    void destroySurface();
+    cairo_surface_t* surface = nullptr;
 
   public:
     PdfSvgClosure* _closure = NULL;