Merge branch 'tm_sla_png_minz'

src/libslic3r/PrintExport.hpp

@@ -80,9 +80,8 @@ public:
 // Provokes static_assert in the right way.
 template<class T = void> struct VeryFalse { static const bool value = false; };
 
-// This has to be explicitly implemented in the gui layer or a default zlib
+// This can be explicitly implemented in the gui layer or the default Zipper
-// based implementation is needed. I don't have time for that and I'm delegating
+// API in libslic3r with minz.
-// the implementation to the gui layer where the gui toolkit can cover this.
 template<class Fmt> class LayerWriter {
 public:
 
@@ -91,21 +90,28 @@ public:
                       "No layer writer implementation provided!");
     }
 
+    // Should create a new file within the zip with the given filename. It
+    // should also finish any previous entry.
     void next_entry(const std::string& /*fname*/) {}
 
-    // binary entry
+    // Should create a new file within the archive and write the provided data.
     void binary_entry(const std::string& /*fname*/,
                       const std::uint8_t* buf, size_t len);
 
+    // Get the name of the archive but only the name part without the path or
+    // the extension.
     std::string get_name() { return ""; }
 
+    // Test whether the object can still be used for writing.
     bool is_ok() { return false; }
 
+    // Write some data (text) into the current file (entry) within the archive.
     template<class T> LayerWriter& operator<<(T&& /*arg*/) {
         return *this;
     }
 
-    void close() {}
+    // Flush the current entry into the archive.
+    void finalize() {}
 };
 
 // Implementation for PNG raster output
@@ -267,7 +273,7 @@ public:
                 }
             }
 
-            writer.close();
+            writer.finalize();
         } catch(std::exception& e) {
             BOOST_LOG_TRIVIAL(error) << e.what();
             // Rethrow the exception

src/libslic3r/SLAPrint.hpp

@@ -322,18 +322,18 @@ template<> class LayerWriter<SLAminzZipper> {
     Zipper m_zip;
 public:
 
-    inline LayerWriter(const std::string& zipfile_path): m_zip(zipfile_path) {}
+    LayerWriter(const std::string& zipfile_path): m_zip(zipfile_path) {}
 
-    inline void next_entry(const std::string& fname) { m_zip.add_entry(fname); }
+    void next_entry(const std::string& fname) { m_zip.add_entry(fname); }
 
-    inline void binary_entry(const std::string& fname,
+    void binary_entry(const std::string& fname,
                              const std::uint8_t* buf,
                              size_t l)
     {
         m_zip.add_entry(fname, buf, l);
     }
 
-    inline std::string get_name() const {
+    std::string get_name() const {
         return m_zip.get_name();
     }
 
@@ -345,7 +345,11 @@ public:
         return true; // m_zip blows up if something goes wrong...
     }
 
-    inline void close() { m_zip.close(); }
+    // After finalize, no writing to the archive will have an effect. The only
+    // valid operation is to dispose the object calling the destructor which
+    // should close the file. This method can throw and signal potential errors
+    // when flushing the archive. This is why its present.
+    void finalize() { m_zip.finalize(); }
 };
 
 /**

src/libslic3r/Zipper.cpp

@@ -111,6 +111,11 @@ public:
     {
         throw std::runtime_error(formatted_errorstr());
     }
+
+    bool is_alive()
+    {
+        return arch.m_zip_mode != MZ_ZIP_MODE_WRITING_HAS_BEEN_FINALIZED;
+    }
 };
 
 Zipper::Zipper(const std::string &zipfname, e_compression compression)
@@ -129,11 +134,19 @@ Zipper::Zipper(const std::string &zipfname, e_compression compression)
 
 Zipper::~Zipper()
 {
-    try {
+    if(m_impl->is_alive()) {
-        close();
+        // Flush the current entry if not finished yet.
-    } catch(...) {
+        try { finish_entry(); } catch(...) {
             BOOST_LOG_TRIVIAL(error) << m_impl->formatted_errorstr();
         }
+
+        if(!mz_zip_writer_finalize_archive(&m_impl->arch))
+            BOOST_LOG_TRIVIAL(error) << m_impl->formatted_errorstr();
+    }
+
+    // The file should be closed no matter what...
+    if(!mz_zip_writer_end(&m_impl->arch))
+        BOOST_LOG_TRIVIAL(error) << m_impl->formatted_errorstr();
 }
 
 Zipper::Zipper(Zipper &&m):
@@ -152,12 +165,16 @@ Zipper &Zipper::operator=(Zipper &&m) {
 
 void Zipper::add_entry(const std::string &name)
 {
+    if(!m_impl->is_alive()) return;
+
     finish_entry(); // finish previous business
     m_entry = name;
 }
 
 void Zipper::add_entry(const std::string &name, const uint8_t *data, size_t l)
 {
+    if(!m_impl->is_alive()) return;
+
     finish_entry();
     mz_uint cmpr = MZ_NO_COMPRESSION;
     switch (m_compression) {
@@ -175,7 +192,9 @@ void Zipper::add_entry(const std::string &name, const uint8_t *data, size_t l)
 
 void Zipper::finish_entry()
 {
-    if(!m_data.empty() > 0 && !m_entry.empty()) {
+    if(!m_impl->is_alive()) return;
+
+    if(!m_data.empty() && !m_entry.empty()) {
         mz_uint compression = MZ_NO_COMPRESSION;
 
         switch (m_compression) {
@@ -198,12 +217,12 @@ std::string Zipper::get_name() const {
     return boost::filesystem::path(m_impl->m_zipname).stem().string();
 }
 
-void Zipper::close()
+void Zipper::finalize()
 {
     finish_entry();
 
-    if(!mz_zip_writer_finalize_archive(&m_impl->arch)) m_impl->blow_up();
+    if(m_impl->is_alive()) if(!mz_zip_writer_finalize_archive(&m_impl->arch))
-    if(!mz_zip_writer_end(&m_impl->arch)) m_impl->blow_up();
+        m_impl->blow_up();
 }
 
 }

src/libslic3r/Zipper.hpp

@@ -47,6 +47,7 @@ public:
     void add_entry(const std::string& name);
 
     /// Add a new binary file entry with an instantly given byte buffer.
+    /// This method throws exactly like finish_entry() does.
     void add_entry(const std::string& name, const std::uint8_t* data, size_t l);
 
     // Writing data to the archive works like with standard streams. The target
@@ -74,12 +75,16 @@ public:
     /// buffer and ones an entry is added, the buffer will bind to the new entry
     /// If the buffer was written, but no entry was added, the buffer will be
     /// cleared after this call.
+    ///
+    /// This method will throw a runtime exception if an error occures. The
+    /// entry will still be open (with the data intact) but the state of the
+    /// file is up to minz after the erroneous write.
     void finish_entry();
 
     /// Gets the name of the archive without the path or extension.
     std::string get_name() const;
 
-    void close();
+    void finalize();
 };