review

f3d-app · Meakk · Jul 10, 2023 · Jun 16, 2023 · Jun 19, 2023 · Jun 19, 2023
commit 2c0928a09d3480e3c81cab34d1d4bc12a016aee2
diff --git a/library/public/image.h b/library/public/image.h
@@ -119,8 +119,8 @@ class F3D_EXPORT image
    * Set/Get image buffer data.
    * Its size is expected to be `width * height * channelCount * typeSize`.
    */
-  image& setData(unsigned char* buffer);
-  unsigned char* getData() const;
+  image& setData(void* buffer);
+  void* getData() const;
   ///@}
 
   /**

diff --git a/library/src/image.cxx b/library/src/image.cxx
@@ -37,18 +37,18 @@ image::image(unsigned int width, unsigned int height, unsigned int channelCount,
   : Internals(new image::internals())
 {
   this->Internals->Image = vtkSmartPointer<vtkImageData>::New();
-  this->Internals->Image->SetDimensions(width, height, 1);
+  this->Internals->Image->SetDimensions(static_cast<int>(width), static_cast<int>(height), 1);
 
   switch (type)
   {
     case ChannelType::BYTE:
-      this->Internals->Image->AllocateScalars(VTK_UNSIGNED_CHAR, channelCount);
+      this->Internals->Image->AllocateScalars(VTK_UNSIGNED_CHAR, static_cast<int>(channelCount));
       break;
     case ChannelType::SHORT:
-      this->Internals->Image->AllocateScalars(VTK_UNSIGNED_SHORT, channelCount);
+      this->Internals->Image->AllocateScalars(VTK_UNSIGNED_SHORT, static_cast<int>(channelCount));
       break;
     case ChannelType::FLOAT:
-      this->Internals->Image->AllocateScalars(VTK_FLOAT, channelCount);
+      this->Internals->Image->AllocateScalars(VTK_FLOAT, static_cast<int>(channelCount));
       break;
   }
 }
@@ -98,8 +98,11 @@ image::image(const image& img)
 //----------------------------------------------------------------------------
 image& image::operator=(const image& img) noexcept
 {
-  this->Internals->Image = vtkSmartPointer<vtkImageData>::New();
-  this->Internals->Image->DeepCopy(img.Internals->Image);
+  if (this != &img)
+  {
+    this->Internals->Image = vtkSmartPointer<vtkImageData>::New();
+    this->Internals->Image->DeepCopy(img.Internals->Image);
+  }
   return *this;
 }
 
@@ -136,8 +139,9 @@ unsigned int image::getHeight() const
 //----------------------------------------------------------------------------
 image& image::setResolution(unsigned int width, unsigned int height)
 {
-  this->Internals->Image->SetDimensions(width, height, 1);
-  this->Internals->Image->AllocateScalars(VTK_UNSIGNED_CHAR, this->getChannelCount());
+  this->Internals->Image->SetDimensions(static_cast<int>(width), static_cast<int>(height), 1);
+  this->Internals->Image->AllocateScalars(
+    VTK_UNSIGNED_CHAR, static_cast<int>(this->getChannelCount()));
   return *this;
 }
 #endif
@@ -152,7 +156,7 @@ unsigned int image::getChannelCount() const
 //----------------------------------------------------------------------------
 image& image::setChannelCount(unsigned int dim)
 {
-  this->Internals->Image->AllocateScalars(VTK_UNSIGNED_CHAR, dim);
+  this->Internals->Image->AllocateScalars(VTK_UNSIGNED_CHAR, static_cast<int>(dim));
   return *this;
 }
 #endif
@@ -168,25 +172,28 @@ image::ChannelType image::getChannelType() const
       return ChannelType::SHORT;
     case VTK_FLOAT:
       return ChannelType::FLOAT;
+    default:
+      break;
   }
   throw read_exception("Unknown channel type");
 }
 
 //----------------------------------------------------------------------------
-image& image::setData(unsigned char* buffer)
+image& image::setData(void* buffer)
 {
-  int scalarSize = this->Internals->Image->GetScalarSize();
-  int totalSize = this->getWidth() * this->getHeight() * this->getChannelCount() * scalarSize;
+  unsigned int scalarSize = this->Internals->Image->GetScalarSize();
+  unsigned int totalSize =
+    this->getWidth() * this->getHeight() * this->getChannelCount() * scalarSize;
   unsigned char* internalBuffer =
-    reinterpret_cast<unsigned char*>(this->Internals->Image->GetScalarPointer());
-  std::copy(buffer, buffer + totalSize, internalBuffer);
+    static_cast<unsigned char*>(this->Internals->Image->GetScalarPointer());
+  std::copy_n(static_cast<unsigned char*>(buffer), totalSize, internalBuffer);
   return *this;
 }
 
 //----------------------------------------------------------------------------
-unsigned char* image::getData() const
+void* image::getData() const
 {
-  return reinterpret_cast<unsigned char*>(this->Internals->Image->GetScalarPointer());
+  return this->Internals->Image->GetScalarPointer();
 }
 
 //----------------------------------------------------------------------------

diff --git a/library/testing/TestSDKImage.cxx b/library/testing/TestSDKImage.cxx
@@ -84,6 +84,24 @@ int TestSDKImage(int argc, char* argv[])
   }
 #endif
 
+  // check reading invalid image
+  try
+  {
+    f3d::image invalidImg(std::string(argv[1]) + "/data/invalid.png");
+
+    std::cerr << "An exception has not been thrown when reading an invalid file" << std::endl;
+    return EXIT_FAILURE;
+  }
+  catch (const f3d::image::read_exception&)
+  {
+  }
+
+  if (hdrImg.getChannelType() != f3d::image::ChannelType::FLOAT)
+  {
+    std::cerr << "Cannot read a HDR 32-bits image" << std::endl;
+    return EXIT_FAILURE;
+  }
+
   // check generated image with baseline
   f3d::image baseline(std::string(argv[1]) + "/baselines/TestSDKImage.png");
 

diff --git a/python/F3DPythonBindings.cxx b/python/F3DPythonBindings.cxx
@@ -4,7 +4,6 @@
 
 #include "camera.h"
 #include "engine.h"
-#include "export.h"
 #include "image.h"
 #include "interactor.h"
 #include "loader.h"
@@ -121,14 +120,14 @@ PYBIND11_MODULE(f3d, module)
         {
           throw py::value_error();
         }
-        img.setData((unsigned char*)info.ptr);
+        img.setData(info.ptr);
       })
     .def("getData",
       [](const f3d::image& img)
       {
         size_t expectedSize = img.getChannelCount() * img.getWidth() * img.getHeight() *
           ::get_channel_size(img.getChannelType());
-        return py::bytes((char*)img.getData(), expectedSize);
+        return py::bytes(static_cast<char*>(img.getData()), expectedSize);
       })
     .def("compare", &f3d::image::compare)
     .def(

diff --git a/python/testing/TestPythonImageData.py b/python/testing/TestPythonImageData.py
@@ -45,6 +45,13 @@
 img.setData(data)
 assert img.getData() == data
 
+'''check channel type and save image'''
+
+assert img.getChannelType() == f3d.image.ChannelType.BYTE
+
+img.save(sys.argv[3] + "/Testing/Temporary/TestPythonSaveFile.bmp", f3d.image.SaveFormat.BMP)
+assert os.path.isfile(sys.argv[3] + "/Testing/Temporary/TestPythonSaveFile.bmp")
+
 
 '''attempt to set partial data back'''