Change C++ canvas import/export semantics.

6 files changed, 126 insertions, 168 deletions

diff --git a/libdimensionxx/canvas.cpp b/libdimensionxx/canvas.cpp
index c8b3389..3ff9d32 100644
--- a/libdimensionxx/canvas.cpp
+++ b/libdimensionxx/canvas.cpp
@@ -24,61 +24,59 @@ namespace Dimension
 {
   // Allocate the canvas with dmnsn_new_canvas()
   Canvas::Canvas(unsigned int width, unsigned int height)
-    : m_canvas(dmnsn_new_canvas(width, height)) { }
+    : m_canvas(new dmnsn_canvas*(dmnsn_new_canvas(width, height))) { }
 
   // Wrap an existing dmnsn_canvas*
   Canvas::Canvas(dmnsn_canvas* canvas)
-    : m_canvas(canvas) { }
+    : m_canvas(new dmnsn_canvas*(canvas)) { }
 
   // Virtual destructor: delete the canvas with dmnsn_delete_canvas().
   Canvas::~Canvas()
   {
-    dmnsn_delete_canvas(m_canvas);
+    if (m_canvas && m_canvas.unique()) {
+      dmnsn_delete_canvas(dmnsn());
+    }
   }
 
   // Get the width
   unsigned int
   Canvas::width() const
   {
-    return m_canvas->x;
+    return dmnsn()->x;
   }
 
   // Get the height
   unsigned int
   Canvas::height() const
   {
-    return m_canvas->y;
+    return dmnsn()->y;
   }
 
   // Get a particular pixel
   Color
   Canvas::pixel(unsigned int x, unsigned int y) const
   {
-    return Color(dmnsn_get_pixel(m_canvas, x, y));
+    return Color(dmnsn_get_pixel(dmnsn(), x, y));
   }
 
   // Set a particular pixel
   void
   Canvas::pixel(unsigned int x, unsigned int y, const Color& c)
   {
-    dmnsn_set_pixel(m_canvas, x, y, c.dmnsn());
+    dmnsn_set_pixel(dmnsn(), x, y, c.dmnsn());
   }
 
   // Return the wrapped canvas
   dmnsn_canvas* 
   Canvas::dmnsn()
   {
-    return m_canvas;
+    return *m_canvas;
   }
 
   // Return a const version of the wrapped canvas
   const dmnsn_canvas*
   Canvas::dmnsn() const
   {
-    return m_canvas;
+    return *m_canvas;
   }
-
-  // Protected default constructor: set m_canvas to NULL.
-  Canvas::Canvas()
-    : m_canvas(0) { }
 }
diff --git a/libdimensionxx/dimensionxx/canvas.hpp b/libdimensionxx/dimensionxx/canvas.hpp
index f519fbd..b8f2887 100644
--- a/libdimensionxx/dimensionxx/canvas.hpp
+++ b/libdimensionxx/dimensionxx/canvas.hpp
@@ -23,6 +23,8 @@
 #ifndef DIMENSIONXX_CANVAS_HPP
 #define DIMENSIONXX_CANVAS_HPP
 
+#include <tr1/memory>
+
 namespace Dimension
 {
   // Base canvas class.  Wraps a dmnsn_canvas*.
@@ -31,10 +33,14 @@ namespace Dimension
   public:
     // Allocate a dmnsn_canvas of specified width and height
     Canvas(unsigned int width, unsigned int height);
+
     // Wrap an existing canvas
     explicit Canvas(dmnsn_canvas* canvas);
+
+    // Canvas(const Canvas& canvas);
+
     // Delete the canvas
-    virtual ~Canvas();
+    ~Canvas();
 
     // Get the width and height
     unsigned int width() const;
@@ -48,17 +54,11 @@ namespace Dimension
     dmnsn_canvas*       dmnsn();
     const dmnsn_canvas* dmnsn() const;
 
-  protected:
-    // Derived classes may want to set m_canvas later.  Set it to NULL now, so
-    // that the destructor can still dmnsn_delete_canvas it.
-    Canvas();
-
-    dmnsn_canvas* m_canvas;
-
   private:
-    // Copying prohibited
-    Canvas(const Canvas&);
+    // Copy-assignment prohibited
     Canvas& operator=(const Canvas&);
+
+    std::tr1::shared_ptr<dmnsn_canvas*> m_canvas;
   };
 }
 
diff --git a/libdimensionxx/dimensionxx/png.hpp b/libdimensionxx/dimensionxx/png.hpp
index 980840c..17cb883 100644
--- a/libdimensionxx/dimensionxx/png.hpp
+++ b/libdimensionxx/dimensionxx/png.hpp
@@ -28,55 +28,43 @@
 
 namespace Dimension
 {
-  // PNG_Canvas handles reading a Canvas from a PNG file, writing one to a PNG
-  // file, or both, depending on what type of stream(s) are given to the
-  // constructor.
-  class PNG_Canvas : public Canvas
+  class PNG_Writer
   {
   public:
-    // Input PNG_Canvas; read the Canvas from istr now
-    explicit PNG_Canvas(std::istream& istr);
+    PNG_Writer(Canvas& canvas, std::ostream& ostr);
+    ~PNG_Writer();
 
-    // Output PNG_Canvas; write the Canvas to ostr at destruction, or when
-    // write() is called.
-    PNG_Canvas(unsigned int x, unsigned int y, std::ostream& ostr);
-
-    // I/O PNG_Canvas; read the Canvas from istr now, and write to ostr at
-    // destruction or then write() is called.
-    PNG_Canvas(std::istream& istr, std::ostream& ostr);
+    void write();
+    Progress write_async();
 
-    // Call write() if we're an output PNG_Canvas, but trap any exceptions and
-    // report a dmnsn_error() instead.
-    virtual ~PNG_Canvas();
+  private:
+    // Copying prohibited
+    PNG_Writer(const PNG_Writer&);
+    PNG_Writer& operator=(const PNG_Writer&);
 
-    // Write the Canvas to the output stream, throwing a Dimension_Error on
-    // error.
-    void write();
+    Canvas* m_canvas;
+    std::ostream* m_ostr;
+    bool m_written;
+  };
 
-    // Write the Canvas to the output stream in the background
-    Progress write_async();
+  class PNG_Reader
+  {
+  public:
+    PNG_Reader(std::istream& istr);
+    // ~PNG_Reader();
 
-    // Construct an input or I/O PNG_Canvas in the background
-    static Progress read_async(std::istream& istr);
-    explicit PNG_Canvas(Progress& progress);
-    explicit PNG_Canvas(Progress& progress, std::ostream& ostr);
+    Canvas read();
 
-  protected:
-    // In case a derived class needs to set m_canvas after we're constructed
-    explicit PNG_Canvas(std::ostream& ostr);
+    Progress read_async();
+    static Canvas finish(Progress& progress);
 
   private:
     // Copying prohibited
-    PNG_Canvas(const PNG_Canvas&);
-    PNG_Canvas& operator=(const PNG_Canvas&);
+    PNG_Reader(const PNG_Reader&);
+    PNG_Reader& operator=(const PNG_Reader&);
 
     std::istream* m_istr;
-    std::ostream* m_ostr;
-    bool m_written;
-
-    // Read the Canvas from a PNG file, and throw a Dimension_Error upon
-    // failure.
-    void read();
+    bool m_read;
   };
 }
 
diff --git a/libdimensionxx/png.cpp b/libdimensionxx/png.cpp
index b626269..1f61e72 100644
--- a/libdimensionxx/png.cpp
+++ b/libdimensionxx/png.cpp
@@ -23,57 +23,42 @@
 
 namespace Dimension
 {
-  PNG_Canvas::PNG_Canvas(std::istream& istr)
-    : Canvas(), m_istr(&istr), m_ostr(0), m_written(false)
+  PNG_Writer::PNG_Writer(Canvas& canvas, std::ostream& ostr)
+    : m_canvas(&canvas), m_ostr(&ostr), m_written(false)
   {
-    read();
+    // Optimize the canvas for PNG export
+    dmnsn_png_optimize_canvas(m_canvas->dmnsn());
   }
 
-  PNG_Canvas::PNG_Canvas(unsigned int x, unsigned int y, std::ostream& ostr)
-    : Canvas(x, y), m_istr(0), m_ostr(&ostr), m_written(false)
-  { }
-
-  PNG_Canvas::PNG_Canvas(std::istream& istr, std::ostream& ostr)
-    : Canvas(), m_istr(&istr), m_ostr(&ostr), m_written(false)
-  {
-    read();
-  }
-
-  // PNG_Canvas destructor.  Call write() to write the PNG file if not already
+  // PNG_Writer destructor.  Call write() to write the PNG file if not already
   // written, but catch any exceptions and instead report the error with
   // dmnsn_error() to avoid throwing from a destructor.
-  PNG_Canvas::~PNG_Canvas()
+  PNG_Writer::~PNG_Writer()
   {
-    if (m_ostr && !m_written) {
+    if (!m_written) {
       try {
         write();
       } catch (...) {
         dmnsn_error(SEVERITY_MEDIUM,
-                    "Writing canvas to PNG failed in PNG_Canvas destructor.");
+                    "Writing canvas to PNG failed in PNG_Writer destructor.");
       }
     }
   }
 
   // Write the PNG file.  Uses the FILE_Cookie() interface to make a FILE*
   // corresponding to an std::ostream (including std::ostringstream, etc).
-  void PNG_Canvas::write()
+  void PNG_Writer::write()
   {
     if (m_written) {
       // Does writing a PNG file twice make sense?
       throw Dimension_Error("Attempt to write canvas to PNG twice.");
     }
 
-    if (!m_ostr) {
-      // Don't call write() if we're not an output PNG_Canvas...
-      throw Dimension_Error("Attempt to write canvas to PNG without an output"
-                            " stream.");
-    }
-
     // Make the C++/C I/O interface
     FILE_Cookie cookie(*m_ostr);
 
     // Write the PNG file
-    if (dmnsn_png_write_canvas(m_canvas, cookie.file())) {
+    if (dmnsn_png_write_canvas(m_canvas->dmnsn(), cookie.file())) {
       // The actual write operation failed, for some reason
       throw Dimension_Error("Writing canvas to PNG failed.");
     }
@@ -83,19 +68,13 @@ namespace Dimension
 
   // Write a PNG file in the background
   Progress
-  PNG_Canvas::write_async()
+  PNG_Writer::write_async()
   {
     if (m_written) {
       // Does writing a PNG file twice make sense?
       throw Dimension_Error("Attempt to write canvas to PNG twice.");
     }
 
-    if (!m_ostr) {
-      // Don't call write_async() if we're not an output PNG_Canvas...
-      throw Dimension_Error("Attempt to write canvas to PNG without an output"
-                            " stream.");
-    }
-
     m_written = true; // We've written the file now, don't do it again
 
     // Object to persist local variables past function return
@@ -107,7 +86,7 @@ namespace Dimension
 
     // Start the asynchronous task
     dmnsn_progress *progress
-      = dmnsn_png_write_canvas_async(m_canvas, cookie->file());
+      = dmnsn_png_write_canvas_async(m_canvas->dmnsn(), cookie->file());
     if (!progress) {
       throw Dimension_Error("Starting background PNG write failed.");
     }
@@ -116,20 +95,58 @@ namespace Dimension
     return Progress(progress, persister);
   }
 
+  // Construct a PNG reader
+  PNG_Reader::PNG_Reader(std::istream& istr)
+    : m_istr(&istr), m_read(false) { }
+
+  // Read a canvas from a PNG file.  Uses the FILE_Cookie() interface to make a
+  // FILE* corresponding to an std::istream
+  Canvas
+  PNG_Reader::read()
+  {
+    if (m_read) {
+      // Does reading a PNG file twice make sense?
+      throw Dimension_Error("Attempt to read canvas from PNG twice.");
+    }
+
+    // Make the C++/C I/O interface
+    FILE_Cookie cookie(*m_istr);
+
+    // Read the canvas from a PNG file
+    dmnsn_canvas* canvas = dmnsn_png_read_canvas(cookie.file());
+    if (!canvas) {
+      // The read operation failed
+      throw Dimension_Error("Reading canvas from PNG failed.");
+    }
+
+    // Only set m_read if nothing threw an exception
+    Canvas ret(canvas);
+    m_read = true;
+    return ret;
+  }
+
   // Read a PNG file in the background
   Progress
-  PNG_Canvas::read_async(std::istream& istr)
+  PNG_Reader::read_async()
   {
+    if (m_read) {
+      // Does reading a PNG file twice make sense?
+      throw Dimension_Error("Attempt to read canvas from PNG twice.");
+    }
+
+    // Don't read again
+    m_read = true;
+
     // Object to persist local variables past function return
     Persister persister;
 
     // Store a pointer to a dmnsn_canvas* in the persister to later construct
-    // the PNG_Canvas
+    // the PNG_Writer
     dmnsn_canvas** canvas = new dmnsn_canvas*;
     persister.persist(canvas);
 
     // Make the C++/C I/O interface
-    FILE_Cookie* cookie = new FILE_Cookie(istr);
+    FILE_Cookie* cookie = new FILE_Cookie(*m_istr);
     persister.persist(cookie);
 
     // Start the asynchronous task
@@ -143,11 +160,11 @@ namespace Dimension
     return Progress(progress, persister);
   }
 
-  // Construct an input PNG_Canvas from a background task
-  PNG_Canvas::PNG_Canvas(Progress& progress)
-    : Canvas(), m_istr(0), m_ostr(0), m_written(false)
+  // Construct an input PNG_Writer from a background task
+  Canvas
+  PNG_Reader::finish(Progress& progress)
   {
-    // Will throw if progress is not from a PNG_Canvas::read_async call
+    // Will throw if progress is not from a PNG_Writer::read_async call
     dmnsn_canvas** canvas
       = progress.persister().first<dmnsn_canvas*>().persisted();
 
@@ -158,50 +175,6 @@ namespace Dimension
       throw;
     }
 
-    m_canvas = *canvas;
-  }
-
-  // Construct an I/O PNG_Canvas from a background task
-  PNG_Canvas::PNG_Canvas(Progress& progress, std::ostream& ostr)
-    : Canvas(), m_istr(0), m_ostr(&ostr), m_written(false)
-  {
-    // Will throw if progress is not from a PNG_Canvas::read_async call
-    dmnsn_canvas** canvas
-      = progress.persister().first<dmnsn_canvas*>().persisted();
-
-    try {
-      progress.finish();
-    } catch (...) {
-      dmnsn_delete_canvas(*canvas);
-      throw;
-    }
-
-    m_canvas = *canvas;
-  }
-
-  // Protected constructor which sets the canvas to NULL for now
-  PNG_Canvas::PNG_Canvas(std::ostream& ostr)
-    : Canvas(), m_istr(0), m_ostr(&ostr), m_written(false)
-  { }
-
-  // Read a canvas from a PNG file.  Uses the FILE_Cookie() interface to make a
-  // FILE* corresponding to an std::istream (including std::istringstream, etc).
-  void PNG_Canvas::read()
-  {
-    if (!m_istr) {
-      // read() is private, and only called from the appropriate constructors,
-      // so this REALLY shouldn't happen.
-      throw Dimension_Error("Attempt to read canvas from PNG without an input"
-                            " stream.");
-    }
-
-    // Make the C++/C I/O interface
-    FILE_Cookie cookie(*m_istr);
-
-    // Read the canvas from a PNG file
-    if (!(m_canvas = dmnsn_png_read_canvas(cookie.file()))) {
-      // The read operation failed
-      throw Dimension_Error("Reading canvas from PNG failed.");
-    }
+    return Canvas(*canvas);
   }
 }
diff --git a/tests/pngxx.cpp b/tests/pngxx.cpp
index 43c4f5d..27fe89c 100644
--- a/tests/pngxx.cpp
+++ b/tests/pngxx.cpp
@@ -34,7 +34,8 @@ main()
 
   {
     std::ofstream ofstr("dimensionxx1.png", std::ios::binary);
-    PNG_Canvas ocanvas(3*width, height, ofstr);
+    Canvas canvas(3*width, height);
+    PNG_Writer writer(canvas, ofstr);
 
     CIE_xyY xyY;
     CIE_Lab Lab;
@@ -46,8 +47,8 @@ main()
       for (unsigned int y = 0; y < height; ++y) {
         /* CIE xyY colorspace */
         xyY = CIE_xyY(static_cast<double>(x)/(width - 1),
-                                 static_cast<double>(y)/(height - 1),
-                                 0.5);
+                      static_cast<double>(y)/(height - 1),
+                      0.5);
         color = xyY;
         RGB = color;
 
@@ -57,15 +58,13 @@ main()
           color.trans(0.5);
         }
 
-        ocanvas.pixel(x, y, color);
+        canvas.pixel(x, y, color);
 
         /* CIE Lab colorspace */
 
         Lab = CIE_Lab(75.0,
-                                 200.0*(static_cast<double>(x)/
-                                        (width - 1) - 0.5),
-                                 200.0*(static_cast<double>(y)/
-                                        (height - 1) - 0.5));
+                      200.0*(static_cast<double>(x)/(width - 1) - 0.5),
+                      200.0*(static_cast<double>(y)/(height - 1) - 0.5));
         color = Lab;
         RGB = color;
 
@@ -75,15 +74,13 @@ main()
           color.trans(0.5);
         }
 
-        ocanvas.pixel(x + width, y, color);
+        canvas.pixel(x + width, y, color);
 
         /* CIE Luv colorspace */
 
         Luv = CIE_Luv(75.0,
-                                 200.0*(static_cast<double>(x)/
-                                        (width - 1) - 0.5),
-                                 200.0*(static_cast<double>(y)/
-                                        (height - 1) - 0.5));
+                      200.0*(static_cast<double>(x)/(width - 1) - 0.5),
+                      200.0*(static_cast<double>(y)/(height - 1) - 0.5));
         color = Luv;
         RGB = color;
 
@@ -93,24 +90,25 @@ main()
           color.trans(0.5);
         }
 
-        ocanvas.pixel(x + 2*width, y, color);
+        canvas.pixel(x + 2*width, y, color);
       }
     }
 
-    Progress progress = ocanvas.write_async();
+    Progress progress = writer.write_async();
     std::cout << "Writing PNG file: " << progress << std::endl;
   }
 
   std::ifstream ifstr("dimensionxx1.png", std::ios::binary);
-  std::ofstream ofstr("dimensionxx2.png", std::ios::binary);
+  PNG_Reader reader(ifstr);
 
-  Progress iprogress
-    = PNG_Canvas::read_async(ifstr);
+  Progress iprogress = reader.read_async();
   std::cout << "Reading PNG file: " << iprogress << std::endl;
+  Canvas canvas = PNG_Reader::finish(iprogress);
 
-  PNG_Canvas iocanvas(iprogress, ofstr);
+  std::ofstream ofstr("dimensionxx2.png", std::ios::binary);
+  PNG_Writer writer(canvas, ofstr);
 
-  Progress oprogress = iocanvas.write_async();
+  Progress oprogress = writer.write_async();
   std::cout << "Writing PNG file: " << oprogress << std::endl;
 
   return 0;
diff --git a/tests/raytracexx.cpp b/tests/raytracexx.cpp
index 2c1d3c9..e3d55cf 100644
--- a/tests/raytracexx.cpp
+++ b/tests/raytracexx.cpp
@@ -33,8 +33,7 @@ main() {
   background.filter(0.1);
 
   // Canvas
-  std::ofstream file("raytracexx.png");
-  PNG_Canvas canvas(768, 480, file);
+  Canvas canvas(768, 480);
 
   // Camera
   Perspective_Camera camera(
@@ -65,8 +64,10 @@ main() {
   }
 
   // Write the canvas
-  Progress wprogress = canvas.write_async();
-  std::cout << "Writing PNG file: " << wprogress << std::endl;
+  std::ofstream file("raytracexx.png");
+  PNG_Writer writer(canvas, file);
+  Progress progress = writer.write_async();
+  std::cout << "Writing PNG file: " << progress << std::endl;
 
   return EXIT_SUCCESS;
 }