changeset 1307:8a28a9bf8876 broker

ViewportController now gets a ref to its parent viewport for proper lock usage
author Benjamin Golinvaux <bgo@osimis.io>
date Wed, 04 Mar 2020 10:07:37 +0100
parents fef1ec42a7db
children adf234ecaa00
files Framework/Scene2DViewport/ViewportController.cpp Framework/Scene2DViewport/ViewportController.h Framework/Viewport/SdlViewport.cpp Framework/Viewport/SdlViewport.h Framework/Viewport/WebAssemblyViewport.cpp
diffstat 5 files changed, 72 insertions(+), 32 deletions(-) [+]
line wrap: on
line diff
--- a/Framework/Scene2DViewport/ViewportController.cpp	Wed Mar 04 10:07:14 2020 +0100
+++ b/Framework/Scene2DViewport/ViewportController.cpp	Wed Mar 04 10:07:37 2020 +0100
@@ -33,24 +33,24 @@
 namespace OrthancStone
 {
   IFlexiblePointerTracker* DefaultViewportInteractor::CreateTracker(
-    boost::shared_ptr<ViewportController> controller,
+    IViewport&          viewport,
     const PointerEvent& event,
-    unsigned int viewportWidth,
-    unsigned int viewportHeight)
+    unsigned int        viewportWidth,
+    unsigned int        viewportHeight)
   {
     switch (event.GetMouseButton())
     {
       case MouseButton_Left:
-        return new RotateSceneTracker(controller, event);
+        return new RotateSceneTracker(viewport, event);
 
       case MouseButton_Middle:
-        return new PanSceneTracker(controller, event);
+        return new PanSceneTracker(viewport, event);
       
       case MouseButton_Right:
       {
         if (viewportWidth != 0)
         {
-          return new ZoomSceneTracker(controller, event, viewportWidth);
+          return new ZoomSceneTracker(viewport, event, viewportWidth);
         }
         else
         {
@@ -64,24 +64,29 @@
   }
 
 
-  ViewportController::ViewportController() :
-    undoStackW_(boost::make_shared<OrthancStone::UndoStack>()),
-    scene_(new Scene2D),
-    canvasToSceneFactor_(1)
+  ViewportController::ViewportController(IViewport& viewport) 
+    : viewport_(viewport)
+    , undoStackW_(boost::make_shared<OrthancStone::UndoStack>())
+    , scene_(new Scene2D)
+    , canvasToSceneFactor_(1)
   {
   }
 
-  ViewportController::ViewportController(const Scene2D& scene) : 
-    undoStackW_(boost::make_shared<OrthancStone::UndoStack>()),
-    scene_(scene.Clone()),
-    canvasToSceneFactor_(1)
+  ViewportController::ViewportController(IViewport& viewport,
+                                         const Scene2D& scene)
+    : viewport_(viewport)
+    , undoStackW_(boost::make_shared<OrthancStone::UndoStack>())
+    , scene_(scene.Clone())
+    , canvasToSceneFactor_(1)
   {
   }
 
-  ViewportController::ViewportController(boost::weak_ptr<UndoStack> undoStackW) :
-    undoStackW_(undoStackW),
-    scene_(new Scene2D),
-    canvasToSceneFactor_(1)
+  ViewportController::ViewportController(IViewport& viewport, 
+                                         boost::weak_ptr<UndoStack> undoStackW)
+    : viewport_(viewport)
+    , undoStackW_(undoStackW)
+    , scene_(new Scene2D)
+    , canvasToSceneFactor_(1)
   {
   }
  
@@ -274,7 +279,7 @@
       }
 
       // No measure tool, create new tracker from the interactor
-      activeTracker_.reset(interactor.CreateTracker(shared_from_this(), event, viewportWidth, viewportHeight));
+      activeTracker_.reset(interactor.CreateTracker(viewport_, event, viewportWidth, viewportHeight));
     }
   }
 
--- a/Framework/Scene2DViewport/ViewportController.h	Wed Mar 04 10:07:14 2020 +0100
+++ b/Framework/Scene2DViewport/ViewportController.h	Wed Mar 04 10:07:37 2020 +0100
@@ -39,7 +39,7 @@
     {
     }
 
-    virtual IFlexiblePointerTracker* CreateTracker(boost::shared_ptr<ViewportController> controller,
+    virtual IFlexiblePointerTracker* CreateTracker(IViewport& viewport,
                                                    const PointerEvent& event,
                                                    unsigned int viewportWidth,
                                                    unsigned int viewportHeight) = 0;
@@ -50,7 +50,7 @@
   class DefaultViewportInteractor : public IViewportInteractor
   {
   public:
-    virtual IFlexiblePointerTracker* CreateTracker(boost::shared_ptr<ViewportController> controller,
+    virtual IFlexiblePointerTracker* CreateTracker(IViewport& viewport,
                                                    const PointerEvent& event,
                                                    unsigned int viewportWidth,
                                                    unsigned int viewportHeight) ORTHANC_OVERRIDE;
@@ -109,11 +109,11 @@
     ORTHANC_STONE_DEFINE_ORIGIN_MESSAGE(__FILE__, __LINE__, \
                                         SceneTransformChanged, ViewportController);
 
-    ViewportController();
+    ViewportController(IViewport& viewport);
 
-    ViewportController(const Scene2D& scene /* will be cloned */);
+    ViewportController(IViewport& viewport, const Scene2D& scene /* will be cloned */);
 
-    ViewportController(boost::weak_ptr<UndoStack> undoStackW);
+    ViewportController(IViewport& viewport, boost::weak_ptr<UndoStack> undoStackW);
 
     ~ViewportController();
 
@@ -229,6 +229,7 @@
   private:
     double GetCanvasToSceneFactor() const;
 
+    IViewport&                                    viewport_;
     boost::weak_ptr<UndoStack>                    undoStackW_;  // Global stack, possibly shared by all viewports
     std::vector<boost::shared_ptr<MeasureTool> >  measureTools_;
     boost::shared_ptr<IFlexiblePointerTracker>    activeTracker_;  // TODO - Couldn't this be a "std::auto_ptr"?
--- a/Framework/Viewport/SdlViewport.cpp	Wed Mar 04 10:07:14 2020 +0100
+++ b/Framework/Viewport/SdlViewport.cpp	Wed Mar 04 10:07:37 2020 +0100
@@ -48,18 +48,27 @@
     compositor_.reset(compositor);
   }
 
-
   SdlViewport::SdlViewport() :
-    controller_(new ViewportController)
+    controller_(new ViewportController(*this))
   {
     refreshEvent_ = SDL_RegisterEvents(1);
-    
+
     if (refreshEvent_ == static_cast<uint32_t>(-1))
     {
       throw Orthanc::OrthancException(Orthanc::ErrorCode_InternalError);
     }
   }
 
+  SdlViewport::SdlViewport(boost::weak_ptr<UndoStack> undoStackW) :
+    controller_(new ViewportController(*this,undoStackW))
+  {
+    refreshEvent_ = SDL_RegisterEvents(1);
+
+    if (refreshEvent_ == static_cast<uint32_t>(-1))
+    {
+      throw Orthanc::OrthancException(Orthanc::ErrorCode_InternalError);
+    }
+  }
 
   void SdlViewport::SendRefreshEvent()
   {
@@ -79,6 +88,16 @@
     AcquireCompositor(new OpenGLCompositor(context_));  // (*)
   }
 
+  SdlOpenGLViewport::SdlOpenGLViewport(const char* title,
+                                       boost::weak_ptr<UndoStack> undoStackW,
+                                       unsigned int width,
+                                       unsigned int height,
+                                       bool allowDpiScaling) :
+    SdlViewport(undoStackW),
+    context_(title, width, height, allowDpiScaling)
+  {
+    AcquireCompositor(new OpenGLCompositor(context_));  // (*)
+  }
 
   SdlOpenGLViewport::~SdlOpenGLViewport()
   {
--- a/Framework/Viewport/SdlViewport.h	Wed Mar 04 10:07:14 2020 +0100
+++ b/Framework/Viewport/SdlViewport.h	Wed Mar 04 10:07:37 2020 +0100
@@ -43,12 +43,20 @@
 
 #include <SDL_events.h>
 
+// TODO: required for UndoStack injection
+// I don't like it either :)
+#include <boost/weak_ptr.hpp>
+
+#include <boost/thread/recursive_mutex.hpp>
+
 namespace OrthancStone
 {
+  class UndoStack;
+
   class SdlViewport : public IViewport
   {
   private:
-    boost::mutex                           mutex_;
+    boost::recursive_mutex                 mutex_;
     uint32_t                               refreshEvent_;
     boost::shared_ptr<ViewportController>  controller_;
     std::auto_ptr<ICompositor>             compositor_;
@@ -59,8 +67,8 @@
     class SdlLock : public ILock
     {
     private:
-      SdlViewport&                that_;
-      boost::mutex::scoped_lock   lock_;
+      SdlViewport&                        that_;
+      boost::recursive_mutex::scoped_lock lock_;
 
     public:
       SdlLock(SdlViewport& that) :
@@ -96,6 +104,7 @@
 
   public:
     SdlViewport();
+    SdlViewport(boost::weak_ptr<UndoStack> undoStackW);
 
     bool IsRefreshEvent(const SDL_Event& event) const
     {
@@ -128,6 +137,12 @@
                       unsigned int height,
                       bool allowDpiScaling = true);
 
+    SdlOpenGLViewport(const char* title,
+                      boost::weak_ptr<UndoStack> undoStackW,
+                      unsigned int width,
+                      unsigned int height,
+                      bool allowDpiScaling = true);
+
     virtual ~SdlOpenGLViewport();
 
     virtual void Paint() ORTHANC_OVERRIDE;
--- a/Framework/Viewport/WebAssemblyViewport.cpp	Wed Mar 04 10:07:14 2020 +0100
+++ b/Framework/Viewport/WebAssemblyViewport.cpp	Wed Mar 04 10:07:37 2020 +0100
@@ -213,11 +213,11 @@
   {
     if (scene == NULL)
     {
-      controller_ = boost::make_shared<ViewportController>();
+      controller_ = boost::make_shared<ViewportController>(*this);
     }
     else
     {
-      controller_ = boost::make_shared<ViewportController>(*scene);
+      controller_ = boost::make_shared<ViewportController>(*this,*scene);
     }
 
     LOG(INFO) << "Initializing Stone viewport on HTML canvas: " << canvasId;