image: Modernize API for C++17

library/public/image.h

@@ -4,6 +4,7 @@
 #include "exception.h"
 #include "export.h"
 
+#include <filesystem>
 #include <string>
 #include <vector>
 
@@ -45,13 +46,12 @@ class F3D_EXPORT image
   };
 
   /**
-   * Create an image from file, the following formats are supported:
-   * PNG, PNM, TIFF, BMP, HDR, JPEG, GESigna, MetaImage, TGA.
-   * EXR files are also supported if the associated module is built.
-   * The complete list can be retrieve at runtime by calling `getSupportedFormats()`.
-   * Throw an `image::read_exception` in case of failure.
+   * Read provided file path (used as is) into a new image instance, the following formats are
+   * supported: PNG, PNM, TIFF, BMP, HDR, JPEG, GESigna, MetaImage, TGA. EXR files are also
+   * supported if the associated module is built. The complete list can be retrieve at runtime by
+   * calling `getSupportedFormats()`. Throw an `image::read_exception` in case of failure.
    */
-  explicit image(const std::string& path);
+  explicit image(const std::filesystem::path& filePath);
 
   /**
    * Create an image from a given width, height, and channel count.
@@ -154,7 +154,7 @@ class F3D_EXPORT image
   bool compare(const image& reference, double threshold, double& error) const;
 
   /**
-   * Save an image to a file in the specified format.
+   * Save an image to the provided file path, used as is, in the specified format.
    * Default format is PNG if not specified.
    * PNG: Supports channel type BYTE and SHORT with channel count of 1 to 4
    * JPG: Supports channel type BYTE with channel count of 1 or 3
@@ -163,7 +163,8 @@ class F3D_EXPORT image
    * Throw an `image::write_exception` if the format is incompatible with with image channel type or
    * channel count
    */
-  const image& save(const std::string& path, SaveFormat format = SaveFormat::PNG) const;
+  const image& save(
+    const std::filesystem::path& filePath, SaveFormat format = SaveFormat::PNG) const;
 
   /**
    * Save an image to a memory buffer in the specified format and returns it.
@@ -200,7 +201,7 @@ class F3D_EXPORT image
   /**
    * Set the value for a metadata key. Setting an empty value (`""`) removes the key.
    */
-  f3d::image& setMetadata(const std::string& key, const std::string& value);
+  f3d::image& setMetadata(std::string key, std::string value);
 
   /**
    * Get the value for a metadata key.

library/src/image.cxx

@@ -34,6 +34,8 @@
 #include <string>
 #include <unordered_map>
 
+namespace fs = std::filesystem;
+
 namespace f3d
 {
 class image::internals
@@ -175,36 +177,42 @@ image::image(unsigned int width, unsigned int height, unsigned int channelCount,
 }
 
 //----------------------------------------------------------------------------
-image::image(const std::string& path)
+image::image(const fs::path& filePath)
   : Internals(new image::internals())
 {
   detail::init::initialize();
 
-  std::string fullPath = vtksys::SystemTools::CollapseFullPath(path);
-  if (!vtksys::SystemTools::FileExists(fullPath))
+  try
   {
-    throw read_exception("Cannot open file " + path);
-  }
+    if (!fs::exists(filePath))
+    {
+      throw read_exception("Cannot open file " + filePath.string());
+    }
 
-  auto reader = vtkSmartPointer<vtkImageReader2>::Take(
-    vtkImageReader2Factory::CreateImageReader2(fullPath.c_str()));
+    auto reader = vtkSmartPointer<vtkImageReader2>::Take(
+      vtkImageReader2Factory::CreateImageReader2(filePath.string().c_str()));
 
-  if (reader)
-  {
-    reader->SetFileName(fullPath.c_str());
-    reader->Update();
-    this->Internals->Image = reader->GetOutput();
+    if (reader)
+    {
+      reader->SetFileName(filePath.string().c_str());
+      reader->Update();
+      this->Internals->Image = reader->GetOutput();
+
+      vtkPNGReader* pngReader = vtkPNGReader::SafeDownCast(reader);
+      if (pngReader != nullptr)
+      {
+        this->Internals->ReadPngMetadata(pngReader);
+      }
+    }
 
-    vtkPNGReader* pngReader = vtkPNGReader::SafeDownCast(reader);
-    if (pngReader != nullptr)
+    if (!this->Internals->Image)
     {
-      this->Internals->ReadPngMetadata(pngReader);
+      throw read_exception("Cannot read image " + filePath.string());
     }
   }
-
-  if (!this->Internals->Image)
+  catch (const fs::filesystem_error& ex)
   {
-    throw read_exception("Cannot read image " + path);
+    throw read_exception(std::string("Cannot read image: ") + ex.what());
   }
 }
 
@@ -467,7 +475,7 @@ std::vector<double> image::getNormalizedPixel(const std::pair<int, int>& xy) con
 }
 
 //----------------------------------------------------------------------------
-const image& image::save(const std::string& path, SaveFormat format) const
+const image& image::save(const fs::path& filePath, SaveFormat format) const
 {
   internals::checkSaveFormatCompatibility(*this, format);
 
@@ -492,14 +500,25 @@ const image& image::save(const std::string& path, SaveFormat format) const
       break;
   }
 
-  writer->SetFileName(path.c_str());
-  writer->SetInputData(this->Internals->Image);
-  writer->Write();
+  try
+  {
+    // Ensure the directories exists
+    fs::create_directories(filePath.parent_path());
+
+    writer->SetFileName(filePath.string().c_str());
+    writer->SetInputData(this->Internals->Image);
+    writer->Write();
 
-  if (writer->GetErrorCode() != 0)
+    if (writer->GetErrorCode() != 0)
+    {
+      throw write_exception("Cannot write " + filePath.string());
+    }
+  }
+  catch (const fs::filesystem_error& ex)
   {
-    throw write_exception("Cannot write " + path);
+    throw write_exception(std::string("Cannot write image: ") + ex.what());
   }
+
   return *this;
 }
 
@@ -668,15 +687,15 @@ std::string image::toTerminalText() const
 }
 
 //----------------------------------------------------------------------------
-image& image::setMetadata(const std::string& key, const std::string& value)
+image& image::setMetadata(std::string key, std::string value)
 {
   if (value.empty())
   {
     this->Internals->Metadata.erase(key);
   }
   else
   {
-    this->Internals->Metadata[key] = value;
+    this->Internals->Metadata[std::move(key)] = std::move(value);
   }
   return *this;
 }

library/testing/TestSDKImage.cxx

@@ -79,8 +79,6 @@ int TestSDKImage(int argc, char* argv[])
   f3d::image img32(width, height, channels, f3d::image::ChannelType::FLOAT);
 
   // test exceptions
-  test.expect<f3d::image::write_exception>(
-    "save buffer to incorrect path", [&]() { generated.save("/dummy/folder/img.png"); });
   test.expect<f3d::image::write_exception>("save incompatible buffer to BMP format",
     [&]() { std::ignore = img16.saveBuffer(f3d::image::SaveFormat::BMP); });
   test.expect<f3d::image::write_exception>("save incompatible buffer to PNG format",
@@ -92,9 +90,10 @@ int TestSDKImage(int argc, char* argv[])
     [&]() { std::ignore = img5Ch.saveBuffer(f3d::image::SaveFormat::BMP); });
   test.expect<f3d::image::write_exception>("save incompatible channel count to JPG format",
     [&]() { std::ignore = img2Ch.saveBuffer(f3d::image::SaveFormat::JPG); });
-
-  test.expect<f3d::image::read_exception>(
-    "read image from incorrect path", [&]() { f3d::image img("/dummy/folder/img.png"); });
+  test.expect<f3d::image::write_exception>("save image to invalid path",
+    [&]() { img2Ch.save("/" + std::string(257, 'x') + "/file.ext"); });
+  test.expect<f3d::image::write_exception>("save image to invalid filename",
+    [&]() { img2Ch.save(testingDir + std::string(257, 'x') + ".ext"); });
 
   // check 16-bits image code paths
   f3d::image shortImg(testingDir + "/data/16bit.png");
@@ -120,6 +119,14 @@ int TestSDKImage(int argc, char* argv[])
   test.expect<f3d::image::read_exception>(
     "read invalid image", [&]() { f3d::image invalidImg(testingDir + "/data/invalid.png"); });
 
+  // check reading inexistent image, do not create a "/dummy/folder/img.png"
+  test.expect<f3d::image::read_exception>(
+    "read image from incorrect path", [&]() { f3d::image img("/dummy/folder/img.png"); });
+
+  // check reading image with invalid path
+  test.expect<f3d::image::read_exception>("read image from invalid path",
+    [&]() { f3d::image img("/" + std::string(257, 'x') + "/file.ext"); });
+
   // check generated image with baseline
   test(
     "check generated image size", generated.getWidth() == width && generated.getHeight() == height);
@@ -310,5 +317,5 @@ int TestSDKImage(int argc, char* argv[])
   test("compare with negative threshold", !empty.compare(empty, -1, error) && error == 1.);
   test("compare with threshold == 1", !empty.compare(empty, 1, error) && error == 1.);
 
-  return EXIT_SUCCESS;
+  return test.result();
 }

python/F3DPythonBindings.cxx

@@ -119,7 +119,7 @@ PYBIND11_MODULE(pyf3d, module)
 
   image //
     .def(py::init<>())
-    .def(py::init<const std::string&>())
+    .def(py::init<const std::filesystem::path&>())
     .def(py::init<unsigned int, unsigned int, unsigned int, f3d::image::ChannelType>())
     .def(py::self == py::self)
     .def(py::self != py::self)
@@ -139,11 +139,11 @@ PYBIND11_MODULE(pyf3d, module)
     .def("to_terminal_text", [](const f3d::image& img) { return img.toTerminalText(); })
     .def("set_metadata", &f3d::image::setMetadata)
     .def("get_metadata",
-      [](const f3d::image& img, const std::string& key)
+      [](const f3d::image& img, std::string key)
       {
         try
         {
-          return img.getMetadata(key);
+          return img.getMetadata(std::move(key));
         }
         catch (const f3d::image::metadata_exception&)
         {