Merge topic 'rendering_cpp_cleanup'

62eb0e3f3 copying Actor into Scene
04b6d96aa enable copying for Actor and ConnectivityProxy
09c78eec4 use unique_ptr for PIMPL ConnectivityProxy
b2880566d move Actor
6b0d2e369 use unique_ptr for PIMPL Actor
fe211adfd Merge branch 'master' into rendering_cpp_cleanup
b4f17b851 use unique_ptr for PIMPL for View

Acked-by: Kitware Robot <kwrobot@kitware.com>
Acked-by: Kenneth Moreland <morelandkd@ornl.gov>
Merge-request: !3074

examples/demo/Demo.cxx

@@ -78,7 +78,7 @@ int main(int argc, char* argv[])
   // from, so we want the scalar range to match that of the previous image.
   isoActor.SetScalarRange(actor.GetScalarRange());
   vtkm::rendering::Scene isoScene;
-  isoScene.AddActor(isoActor);
+  isoScene.AddActor(std::move(isoActor));
 
   // Wireframe surface:
   vtkm::rendering::View3D isoView(isoScene, MapperWireframer(), canvas, camera, bg);

tutorial/rendering.cxx

@@ -35,7 +35,7 @@ int main(int argc, char** argv)
 
   //Creating Scene and adding Actor
   vtkm::rendering::Scene scene;
-  scene.AddActor(actor);
+  scene.AddActor(std::move(actor));
 
   //Creating and initializing the View using the Canvas, Ray Tracer Mappers, and Scene
   vtkm::rendering::MapperRayTracer mapper;

vtkm/rendering/Actor.cxx

@@ -10,10 +10,11 @@
 
 #include <vtkm/rendering/Actor.h>
 
-#include <vtkm/Assert.h>
 #include <vtkm/cont/TryExecute.h>
 #include <vtkm/cont/UnknownCellSet.h>
 
+#include <utility>
+
 namespace vtkm
 {
 namespace rendering
@@ -30,25 +31,25 @@ struct Actor::InternalsType
   vtkm::Bounds SpatialBounds;
 
   VTKM_CONT
-  InternalsType(const vtkm::cont::UnknownCellSet& cells,
-                const vtkm::cont::CoordinateSystem& coordinates,
-                const vtkm::cont::Field& scalarField,
+  InternalsType(vtkm::cont::UnknownCellSet cells,
+                vtkm::cont::CoordinateSystem coordinates,
+                vtkm::cont::Field scalarField,
                 const vtkm::rendering::Color& color)
-    : Cells(cells)
-    , Coordinates(coordinates)
-    , ScalarField(scalarField)
+    : Cells(std::move(cells))
+    , Coordinates(std::move(coordinates))
+    , ScalarField(std::move(scalarField))
     , ColorTable(vtkm::Range{ 0, 1 }, color.Components, color.Components)
   {
   }
 
   VTKM_CONT
-  InternalsType(const vtkm::cont::UnknownCellSet& cells,
-                const vtkm::cont::CoordinateSystem& coordinates,
-                const vtkm::cont::Field& scalarField,
+  InternalsType(vtkm::cont::UnknownCellSet cells,
+                vtkm::cont::CoordinateSystem coordinates,
+                vtkm::cont::Field scalarField,
                 const vtkm::cont::ColorTable& colorTable = vtkm::cont::ColorTable::Preset::Default)
-    : Cells(cells)
-    , Coordinates(coordinates)
-    , ScalarField(scalarField)
+    : Cells(std::move(cells))
+    , Coordinates(std::move(coordinates))
+    , ScalarField(std::move(scalarField))
     , ColorTable(colorTable)
   {
   }
@@ -57,7 +58,7 @@ struct Actor::InternalsType
 Actor::Actor(const vtkm::cont::UnknownCellSet& cells,
              const vtkm::cont::CoordinateSystem& coordinates,
              const vtkm::cont::Field& scalarField)
-  : Internals(new InternalsType(cells, coordinates, scalarField))
+  : Internals(std::make_unique<InternalsType>(cells, coordinates, scalarField))
 {
   this->Init(coordinates, scalarField);
 }
@@ -66,7 +67,7 @@ Actor::Actor(const vtkm::cont::UnknownCellSet& cells,
              const vtkm::cont::CoordinateSystem& coordinates,
              const vtkm::cont::Field& scalarField,
              const vtkm::rendering::Color& color)
-  : Internals(new InternalsType(cells, coordinates, scalarField, color))
+  : Internals(std::make_unique<InternalsType>(cells, coordinates, scalarField, color))
 {
   this->Init(coordinates, scalarField);
 }
@@ -75,11 +76,42 @@ Actor::Actor(const vtkm::cont::UnknownCellSet& cells,
              const vtkm::cont::CoordinateSystem& coordinates,
              const vtkm::cont::Field& scalarField,
              const vtkm::cont::ColorTable& colorTable)
-  : Internals(new InternalsType(cells, coordinates, scalarField, colorTable))
+  : Internals(std::make_unique<InternalsType>(cells, coordinates, scalarField, colorTable))
 {
   this->Init(coordinates, scalarField);
 }
 
+Actor::Actor(const Actor& rhs)
+  : Internals(nullptr)
+{
+  // rhs might have been moved, its Internal would be nullptr
+  if (rhs.Internals)
+    Internals = std::make_unique<InternalsType>(*rhs.Internals);
+}
+
+Actor& Actor::operator=(const Actor& rhs)
+{
+  // both *this and rhs might have been moved.
+  if (!rhs.Internals)
+  {
+    Internals.reset();
+  }
+  else if (!Internals)
+  {
+    Internals = std::make_unique<InternalsType>(*rhs.Internals);
+  }
+  else
+  {
+    *Internals = *rhs.Internals;
+  }
+
+  return *this;
+}
+
+Actor::Actor(vtkm::rendering::Actor&&) noexcept = default;
+Actor& Actor::operator=(Actor&&) noexcept = default;
+Actor::~Actor() = default;
+
 void Actor::Init(const vtkm::cont::CoordinateSystem& coordinates,
                  const vtkm::cont::Field& scalarField)
 {

vtkm/rendering/Actor.h

@@ -40,6 +40,13 @@ public:
         const vtkm::cont::Field& scalarField,
         const vtkm::rendering::Color& color);
 
+  Actor(const Actor&);
+  Actor& operator=(const Actor&);
+
+  Actor(Actor&&) noexcept;
+  Actor& operator=(Actor&&) noexcept;
+  ~Actor();
+
   void Render(vtkm::rendering::Mapper& mapper,
               vtkm::rendering::Canvas& canvas,
               const vtkm::rendering::Camera& camera) const;
@@ -60,9 +67,7 @@ public:
 
 private:
   struct InternalsType;
-  std::shared_ptr<InternalsType> Internals;
-
-  struct RangeFunctor;
+  std::unique_ptr<InternalsType> Internals;
 
   void Init(const vtkm::cont::CoordinateSystem& coordinates, const vtkm::cont::Field& scalarField);
 };

vtkm/rendering/ConnectivityProxy.cxx

@@ -15,6 +15,8 @@
 #include <vtkm/rendering/raytracing/Logger.h>
 #include <vtkm/rendering/raytracing/RayOperations.h>
 
+#include <memory>
+
 
 namespace vtkm
 {
@@ -52,8 +54,6 @@ public:
     }
   }
 
-  ~InternalsType() {}
-
   VTKM_CONT
   void SetUnitScalar(vtkm::Float32 unitScalar) { Tracer.SetUnitScalar(unitScalar); }
 
@@ -241,7 +241,7 @@ public:
 VTKM_CONT
 ConnectivityProxy::ConnectivityProxy(const vtkm::cont::DataSet& dataSet,
                                      const std::string& fieldName)
-  : Internals(new InternalsType(dataSet, fieldName))
+  : Internals(std::make_unique<InternalsType>(dataSet, fieldName))
 {
 }
 
@@ -256,11 +256,45 @@ ConnectivityProxy::ConnectivityProxy(const vtkm::cont::UnknownCellSet& cellset,
   dataset.AddCoordinateSystem(coords);
   dataset.AddField(scalarField);
 
-  Internals = std::shared_ptr<InternalsType>(new InternalsType(dataset, scalarField.GetName()));
+  Internals = std::make_unique<InternalsType>(dataset, scalarField.GetName());
+}
+
+ConnectivityProxy::ConnectivityProxy(const ConnectivityProxy& rhs)
+  : Internals(nullptr)
+{
+  // rhs might have been moved, its Internal would be nullptr
+  if (rhs.Internals)
+  {
+    Internals = std::make_unique<InternalsType>(*rhs.Internals);
+  }
+}
+
+ConnectivityProxy& ConnectivityProxy::operator=(const ConnectivityProxy& rhs)
+{
+  // both *this and rhs might have been moved.
+  if (!rhs.Internals)
+  {
+    Internals.reset();
+  }
+  else if (!Internals)
+  {
+    Internals = std::make_unique<InternalsType>(*rhs.Internals);
+  }
+  else
+  {
+    *Internals = *rhs.Internals;
+  }
+
+  return *this;
 }
 
 VTKM_CONT
-ConnectivityProxy::~ConnectivityProxy() {}
+ConnectivityProxy::ConnectivityProxy(ConnectivityProxy&&) noexcept = default;
+VTKM_CONT
+ConnectivityProxy& ConnectivityProxy::operator=(vtkm::rendering::ConnectivityProxy&&) noexcept =
+  default;
+VTKM_CONT
+ConnectivityProxy::~ConnectivityProxy() = default;
 
 VTKM_CONT
 void ConnectivityProxy::SetSampleDistance(const vtkm::Float32& distance)

vtkm/rendering/ConnectivityProxy.h

@@ -34,9 +34,15 @@ public:
   ConnectivityProxy(const vtkm::cont::UnknownCellSet& cellset,
                     const vtkm::cont::CoordinateSystem& coords,
                     const vtkm::cont::Field& scalarField);
-  // Do not allow the default constructor
-  ConnectivityProxy() = delete;
+
+  ConnectivityProxy(const ConnectivityProxy&);
+  ConnectivityProxy& operator=(const ConnectivityProxy&);
+
+  ConnectivityProxy(ConnectivityProxy&&) noexcept;
+  ConnectivityProxy& operator=(ConnectivityProxy&&) noexcept;
+
   ~ConnectivityProxy();
+
   enum struct RenderMode
   {
     Volume,
@@ -45,10 +51,8 @@ public:
 
   void SetRenderMode(RenderMode mode);
   void SetSampleDistance(const vtkm::Float32&);
-  void SetCanvas(vtkm::rendering::Canvas* canvas);
   void SetScalarField(const std::string& fieldName);
   void SetEmissionField(const std::string& fieldName);
-  void SetCamera(const vtkm::rendering::Camera& camera);
   void SetScalarRange(const vtkm::Range& range);
   void SetColorMap(vtkm::cont::ArrayHandle<vtkm::Vec4f_32>& colormap);
   void SetCompositeBackground(bool on);
@@ -69,9 +73,8 @@ public:
 
 protected:
   struct InternalsType;
-  struct BoundsFunctor;
-  std::shared_ptr<InternalsType> Internals;
+  std::unique_ptr<InternalsType> Internals;
 };
 }
 } //namespace vtkm::rendering
-#endif //vtk_m_rendering_SceneRendererVolume_h
+#endif //vtk_m_rendering_ConnectivityProxy_h

vtkm/rendering/Scene.cxx

@@ -27,9 +27,9 @@ Scene::Scene()
 {
 }
 
-void Scene::AddActor(const vtkm::rendering::Actor& actor)
+void Scene::AddActor(vtkm::rendering::Actor actor)
 {
-  this->Internals->Actors.push_back(actor);
+  this->Internals->Actors.push_back(std::move(actor));
 }
 
 const vtkm::rendering::Actor& Scene::GetActor(vtkm::IdComponent index) const
@@ -46,9 +46,8 @@ void Scene::Render(vtkm::rendering::Mapper& mapper,
                    vtkm::rendering::Canvas& canvas,
                    const vtkm::rendering::Camera& camera) const
 {
-  for (vtkm::IdComponent actorIndex = 0; actorIndex < this->GetNumberOfActors(); actorIndex++)
+  for (const auto& actor : this->Internals->Actors)
   {
-    const vtkm::rendering::Actor& actor = this->GetActor(actorIndex);
     actor.Render(mapper, canvas, camera);
   }
 }
@@ -56,10 +55,9 @@ void Scene::Render(vtkm::rendering::Mapper& mapper,
 vtkm::Bounds Scene::GetSpatialBounds() const
 {
   vtkm::Bounds bounds;
-  for (vtkm::IdComponent actorIndex = 0; actorIndex < this->GetNumberOfActors(); actorIndex++)
+  for (const auto& actor : this->Internals->Actors)
   {
-    // accumulate all Actors' spatial bounds into the scene spatial bounds
-    bounds.Include(this->GetActor(actorIndex).GetSpatialBounds());
+    bounds.Include(actor.GetSpatialBounds());
   }
 
   return bounds;

vtkm/rendering/Scene.h

@@ -29,7 +29,7 @@ class VTKM_RENDERING_EXPORT Scene
 public:
   Scene();
 
-  void AddActor(const vtkm::rendering::Actor& actor);
+  void AddActor(vtkm::rendering::Actor actor);
 
   const vtkm::rendering::Actor& GetActor(vtkm::IdComponent index) const;
 

vtkm/rendering/View.cxx

@@ -37,7 +37,7 @@ View::View(const vtkm::rendering::Scene& scene,
            const vtkm::rendering::Canvas& canvas,
            const vtkm::rendering::Color& backgroundColor,
            const vtkm::rendering::Color& foregroundColor)
-  : Internal(std::make_shared<InternalData>())
+  : Internal(std::make_unique<InternalData>())
 {
   this->Internal->Scene = scene;
   this->Internal->MapperPointer = mapper.NewCopy();
@@ -65,7 +65,7 @@ View::View(const vtkm::rendering::Scene& scene,
            const vtkm::rendering::Camera& camera,
            const vtkm::rendering::Color& backgroundColor,
            const vtkm::rendering::Color& foregroundColor)
-  : Internal(std::make_shared<InternalData>())
+  : Internal(std::make_unique<InternalData>())
 {
   this->Internal->Scene = scene;
   this->Internal->MapperPointer = mapper.NewCopy();
@@ -77,7 +77,7 @@ View::View(const vtkm::rendering::Scene& scene,
   this->AxisColor = foregroundColor;
 }
 
-View::~View() {}
+View::~View() = default;
 
 const vtkm::rendering::Scene& View::GetScene() const
 {

vtkm/rendering/View.h

@@ -90,7 +90,7 @@ public:
   void SetWorldAnnotationsEnabled(bool val) { this->WorldAnnotationsEnabled = val; }
 
   VTKM_CONT void SetRenderAnnotationsEnabled(bool val) { this->RenderAnnotationsEnabled = val; }
-  VTKM_CONT bool GetRenderAnnotationsEnabled() { return this->RenderAnnotationsEnabled; }
+  VTKM_CONT bool GetRenderAnnotationsEnabled() const { return this->RenderAnnotationsEnabled; }
 
   virtual void Paint() = 0;
   virtual void RenderScreenAnnotations() = 0;
@@ -126,7 +126,7 @@ protected:
   bool RenderAnnotationsEnabled = true;
 
 private:
-  std::shared_ptr<InternalData> Internal;
+  std::unique_ptr<InternalData> Internal;
 };
 
 } // namespace vtkm::rendering

vtkm/rendering/View1D.cxx

@@ -36,8 +36,6 @@ View1D::View1D(const vtkm::rendering::Scene& scene,
 {
 }
 
-View1D::~View1D() {}
-
 void View1D::Paint()
 {
   this->GetCanvas().Clear();
@@ -104,7 +102,7 @@ void View1D::RenderColorLegendAnnotations()
     this->GetCanvas().BeginTextRenderingBatch();
     for (int i = 0; i < this->GetScene().GetNumberOfActors(); ++i)
     {
-      vtkm::rendering::Actor act = this->GetScene().GetActor(i);
+      const auto& act = this->GetScene().GetActor(i);
 
       vtkm::Vec<double, 4> colorData;
       act.GetColorTable().GetPoint(0, colorData);

vtkm/rendering/View1D.h

@@ -35,8 +35,6 @@ public:
          const vtkm::rendering::Color& backgroundColor = vtkm::rendering::Color(0, 0, 0, 1),
          const vtkm::rendering::Color& foregroundColor = vtkm::rendering::Color(1, 1, 1, 1));
 
-  ~View1D();
-
   void Paint() override;
   void RenderScreenAnnotations() override;
   void RenderWorldAnnotations() override;

vtkm/rendering/View2D.cxx

@@ -34,8 +34,6 @@ View2D::View2D(const vtkm::rendering::Scene& scene,
 {
 }
 
-View2D::~View2D() {}
-
 void View2D::Paint()
 {
   this->GetCanvas().Clear();

vtkm/rendering/View2D.h

@@ -36,8 +36,6 @@ public:
          const vtkm::rendering::Color& backgroundColor = vtkm::rendering::Color(0, 0, 0, 1),
          const vtkm::rendering::Color& foregroundColor = vtkm::rendering::Color(1, 1, 1, 1));
 
-  ~View2D();
-
   void Paint() override;
 
   void RenderScreenAnnotations() override;

vtkm/rendering/View3D.cxx

@@ -34,8 +34,6 @@ View3D::View3D(const vtkm::rendering::Scene& scene,
 {
 }
 
-View3D::~View3D() {}
-
 void View3D::Paint()
 {
   this->GetCanvas().Clear();

vtkm/rendering/View3D.h

@@ -37,8 +37,6 @@ public:
          const vtkm::rendering::Color& backgroundColor = vtkm::rendering::Color(0, 0, 0, 1),
          const vtkm::rendering::Color& foregroundColor = vtkm::rendering::Color(1, 1, 1, 1));
 
-  ~View3D();
-
   void Paint() override;
 
   void RenderScreenAnnotations() override;