changeset 31762:d94ceed56929

GLCanvas: avoid multiple inheritance from both QWidget and QObject To avoid having GLCanvas inherit from both QWidget and QObject, create a separate class, GLWidget, derived from QOpenGLWidget and use that as a member of GLCanvas instead of a base class. * Canvas.h (class Canvas): Derive from QWidget instead of QObject. (Canvas::Canvas): New argument for parent widget. Pass parent to QWidget base class constructor. * GLCanvas.h, GLCanvas.cc (class GLCanvas): Split QOpenGLWidget parts into separate GLWidget class. GLCanvas now processes things related to the interpreter and forwards to GLWidget for rendering. Use unwind_action objects to ensure OpenGL context is restored. (GLCanvas::m_glwidget): New data member for GLWidget. * libbgui/graphics/module.mk (OCTAVE_GUI_GRAPHICS_MOC): Add moc-GLCanvas.cc to the list.
author John W. Eaton <jwe@octave.org>
date Fri, 20 Jan 2023 08:49:19 -0500
parents 3105755fa5f1
children 8c57434587c9
files libgui/graphics/Canvas.h libgui/graphics/GLCanvas.cc libgui/graphics/GLCanvas.h libgui/graphics/module.mk
diffstat 4 files changed, 228 insertions(+), 119 deletions(-) [+]
line wrap: on
line diff
--- a/libgui/graphics/Canvas.h	Fri Jan 20 15:33:05 2023 +0100
+++ b/libgui/graphics/Canvas.h	Fri Jan 20 08:49:19 2023 -0500
@@ -26,7 +26,7 @@
 #if ! defined (octave_Canvas_h)
 #define octave_Canvas_h 1
 
-#include <QObject>
+#include <QWidget>
 #include <QPoint>
 
 #include "event-manager.h"
@@ -45,7 +45,7 @@
 
   class interpreter;
 
-  class Canvas : public QObject
+  class Canvas : public QWidget
   {
     Q_OBJECT
 
@@ -112,16 +112,15 @@
                            const graphics_handle& handle) = 0;
 
   protected:
-    Canvas (octave::interpreter& interp, const graphics_handle& handle)
-      : m_interpreter (interp),
-        m_handle (handle),
-        m_redrawBlocked (false),
-        m_mouseMode (NoMode),
-        m_clickMode (false),
-        m_eventMask (0),
-        m_rectMode (false)
+
+    Canvas (octave::interpreter& interp, const graphics_handle& handle,
+            QWidget *parent)
+      : QWidget (parent), m_interpreter (interp), m_handle (handle),
+        m_redrawBlocked (false), m_mouseMode (NoMode), m_clickMode (false),
+        m_eventMask (0), m_rectMode (false)
     { }
 
+  public:
     void canvasToggleAxes (const graphics_handle& handle);
     void canvasToggleGrid (const graphics_handle& handle);
     void canvasAutoAxes (const graphics_handle& handle);
--- a/libgui/graphics/GLCanvas.cc	Fri Jan 20 15:33:05 2023 +0100
+++ b/libgui/graphics/GLCanvas.cc	Fri Jan 20 08:49:19 2023 -0500
@@ -37,52 +37,54 @@
 
 OCTAVE_BEGIN_NAMESPACE(octave)
 
-  GLCanvas::GLCanvas (octave::interpreter& interp,
-                      const graphics_handle& gh, QWidget *xparent)
-    : QOpenGLWidget (xparent), Canvas (interp, gh), m_glfcns (),
-      m_renderer (m_glfcns)
+  GLWidget::GLWidget (Canvas &parent_canvas)
+    : QOpenGLWidget (&parent_canvas), m_parent_canvas (parent_canvas),
+      m_glfcns (), m_renderer (m_glfcns)
   {
     setFocusPolicy (Qt::ClickFocus);
     setFocus ();
   }
 
-  GLCanvas::~GLCanvas (void)
-  { }
+  GLWidget::~GLWidget (void) { }
 
   void
-  GLCanvas::initializeGL (void)
+  GLWidget::initializeGL (void)
   {
+    // The qopengl_functions object (part of Octave, not Qt) is just
+    // wrapper around QOpenGLFunctions_1_1.  Does initialization really
+    // need to be deferred until initializeGL is called?
+
     m_glfcns.init ();
+
+    // All other resources we need are currently (supposed to be)
+    // managed by the QOpenGLWidget object so there is else nothing to
+    // do here.  If we used custom shader programs, then we would need
+    // to initialize them here.
   }
 
   void
-  GLCanvas::draw (const graphics_handle& gh)
+  GLWidget::draw (graphics_object go)
   {
-    gh_manager& gh_mgr = m_interpreter.get_gh_manager ();
-
-    octave::autolock guard  (gh_mgr.graphics_lock ());
-
-    graphics_object go = gh_mgr.get_object (gh);
-
     if (go)
       {
+        begin_rendering ();
+
+        unwind_action reset_current ([=] () { end_rendering (); });
+
         graphics_object fig = go.get_ancestor ("figure");
         double dpr = fig.get ("__device_pixel_ratio__").double_value ();
         m_renderer.set_viewport (dpr * width (), dpr * height ());
         m_renderer.set_device_pixel_ratio (dpr);
+
         m_renderer.draw (go);
       }
   }
 
   uint8NDArray
-  GLCanvas::do_getPixels (const graphics_handle& gh)
+  GLWidget::do_getPixels (graphics_object go)
   {
     uint8NDArray retval;
 
-    gh_manager& gh_mgr = m_interpreter.get_gh_manager ();
-
-    graphics_object go = gh_mgr.get_object (gh);
-
     if (go && go.isa ("figure"))
       {
         Matrix pos = go.get ("position").matrix_value ();
@@ -90,9 +92,9 @@
         pos(2) *= dpr;
         pos(3) *= dpr;
 
-        // Make sure we have a valid current context
-        if (! begin_rendering ())
-          return retval;
+        begin_rendering ();
+
+        unwind_action reset_current ([=] () { end_rendering (); });
 
         // When the figure is not visible or its size is frozen for printing,
         // we use a framebuffer object to make sure we are rendering on a
@@ -106,104 +108,89 @@
 
             fbo.bind ();
 
+            unwind_action release_fbo ([&] () { fbo.release (); });
+
             m_renderer.set_viewport (pos(2), pos(3));
             m_renderer.set_device_pixel_ratio (dpr);
             m_renderer.draw (go);
+
             retval = m_renderer.get_pixels (pos(2), pos(3));
-
-            fbo.release ();
           }
         else
           {
             m_renderer.set_viewport (pos(2), pos(3));
             m_renderer.set_device_pixel_ratio (dpr);
             m_renderer.draw (go);
+
             retval = m_renderer.get_pixels (pos(2), pos(3));
           }
-
-        end_rendering ();
       }
 
     return retval;
   }
 
   void
-  GLCanvas::do_print (const QString& file_cmd, const QString& term,
-                      const graphics_handle& handle)
+  GLWidget::do_print (const QString& file_cmd, const QString& term,
+                      graphics_object go)
   {
-    gh_manager& gh_mgr = m_interpreter.get_gh_manager ();
-
-    octave::autolock guard  (gh_mgr.graphics_lock ());
-
-    graphics_object go = gh_mgr.get_object (handle);
-
     if (go.valid_object ())
       {
+        begin_rendering ();
+
+        unwind_action reset_current ([=] () { end_rendering (); });
+
         graphics_object fig (go.get_ancestor ("figure"));
 
-        // Make sure we have a valid current context
-        if (! begin_rendering ())
-          error ("print: no valid OpenGL offscreen context");
-
-        try
+        if (fig.get ("visible").string_value () == "on")
+          octave::gl2ps_print (m_glfcns, fig, file_cmd.toStdString (),
+                               term.toStdString ());
+        else
           {
-            if (fig.get ("visible").string_value () == "on")
-              octave::gl2ps_print (m_glfcns, fig, file_cmd.toStdString (),
-                                   term.toStdString ());
-            else
-              {
-                // When the figure is not visible, we use a framebuffer object
-                // to make sure we are rendering on a suitably large frame.
-                Matrix pos = fig.get ("position").matrix_value ();
-                double dpr = fig.get ("__device_pixel_ratio__").double_value ();
-                pos(2) *= dpr;
-                pos(3) *= dpr;
+            // When the figure is not visible, we use a framebuffer object
+            // to make sure we are rendering on a suitably large frame.
+            Matrix pos = fig.get ("position").matrix_value ();
+            double dpr = fig.get ("__device_pixel_ratio__").double_value ();
+            pos(2) *= dpr;
+            pos(3) *= dpr;
 
-                QOpenGLFramebufferObject
-                  fbo (pos(2), pos(3),
-                       QOpenGLFramebufferObject::Attachment::Depth);
-
-                fbo.bind ();
+            QOpenGLFramebufferObject
+              fbo (pos(2), pos(3),
+                   QOpenGLFramebufferObject::Attachment::Depth);
 
-                octave::gl2ps_print (m_glfcns, fig, file_cmd.toStdString (),
-                                     term.toStdString ());
+            fbo.bind ();
 
-                fbo.release ();
-              }
+            unwind_action release_fbo ([&] () { fbo.release (); });
+
+            octave::gl2ps_print (m_glfcns, fig, file_cmd.toStdString (),
+                                 term.toStdString ());
           }
-        catch (octave::execution_exception& ee)
-          {
-            emit interpreter_event
-              ([=] (void)
-               {
-                 // INTERPRETER THREAD
-                 throw ee;
-               });
-          }
-
-        end_rendering ();
       }
   }
 
   graphics_object
-  GLCanvas::selectFromAxes (const graphics_object& ax, const QPoint& pt)
+  GLWidget::selectFromAxes (const graphics_object& ax, const QPoint& pt)
   {
-    makeCurrent ();
-
     if (ax)
       {
+        begin_rendering ();
+
+        unwind_action reset_current ([=] () { end_rendering (); });
+
         octave::opengl_selector s (m_glfcns);
 
         s.set_viewport (width (), height ());
-        return s.select (ax, pt.x (), height () - pt.y (),
-                         octave::select_ignore_hittest);
+
+        graphics_object go = s.select (ax, pt.x (), height () - pt.y (),
+                                       octave::select_ignore_hittest);
+
+        doneCurrent ();
       }
 
     return graphics_object ();
   }
 
   void
-  GLCanvas::drawZoomBox (const QPoint& p1, const QPoint& p2)
+  GLWidget::drawZoomBox (const QPoint& p1, const QPoint& p2)
   {
     Matrix overlaycolor (3, 1);
     overlaycolor(0) = 0.45;
@@ -221,67 +208,72 @@
   }
 
   void
-  GLCanvas::paintGL (void)
+  GLWidget::paintGL (void)
   {
-    canvasPaintEvent ();
+    m_parent_canvas.canvasPaintEvent ();
   }
 
   void
-  GLCanvas::mouseDoubleClickEvent (QMouseEvent *xevent)
+  GLWidget::mouseDoubleClickEvent (QMouseEvent *xevent)
   {
-    canvasMouseDoubleClickEvent (xevent);
+    m_parent_canvas.canvasMouseDoubleClickEvent (xevent);
   }
 
   void
-  GLCanvas::mouseMoveEvent (QMouseEvent *xevent)
+  GLWidget::mouseMoveEvent (QMouseEvent *xevent)
   {
-    canvasMouseMoveEvent (xevent);
+    m_parent_canvas.canvasMouseMoveEvent (xevent);
   }
 
   void
-  GLCanvas::mousePressEvent (QMouseEvent *xevent)
+  GLWidget::mousePressEvent (QMouseEvent *xevent)
   {
-    canvasMousePressEvent (xevent);
+    m_parent_canvas.canvasMousePressEvent (xevent);
   }
 
   void
-  GLCanvas::mouseReleaseEvent (QMouseEvent *xevent)
+  GLWidget::mouseReleaseEvent (QMouseEvent *xevent)
   {
-    canvasMouseReleaseEvent (xevent);
+    m_parent_canvas.canvasMouseReleaseEvent (xevent);
   }
 
   void
-  GLCanvas::wheelEvent (QWheelEvent *xevent)
+  GLWidget::wheelEvent (QWheelEvent *xevent)
   {
-    canvasWheelEvent (xevent);
+    m_parent_canvas.canvasWheelEvent (xevent);
   }
 
   void
-  GLCanvas::keyPressEvent (QKeyEvent *xevent)
+  GLWidget::keyPressEvent (QKeyEvent *xevent)
   {
-    if (! canvasKeyPressEvent (xevent))
+    if (! m_parent_canvas.canvasKeyPressEvent (xevent))
       QOpenGLWidget::keyPressEvent (xevent);
   }
 
   void
-  GLCanvas::keyReleaseEvent (QKeyEvent *xevent)
+  GLWidget::keyReleaseEvent (QKeyEvent *xevent)
   {
-    if (! canvasKeyReleaseEvent (xevent))
+    if (! m_parent_canvas.canvasKeyReleaseEvent (xevent))
       QOpenGLWidget::keyReleaseEvent (xevent);
   }
 
   bool
-  GLCanvas::begin_rendering (void)
+  GLWidget::begin_rendering (void)
   {
     bool retval = true;
 
     if (! isValid ())
       {
+        // FIXME: Is this really the right way to manager offscreen
+        // rendering for printing?
+
         static bool os_ctx_ok = true;
+
         if (os_ctx_ok && ! m_os_context.isValid ())
           {
             // Try to initialize offscreen context
             m_os_surface.create ();
+
             if (! m_os_context.create ())
               {
                 os_ctx_ok = false;
@@ -298,9 +290,92 @@
   }
 
   void
-  GLCanvas::end_rendering (void)
+  GLWidget::end_rendering (void)
   {
     doneCurrent ();
   }
 
+  GLCanvas::GLCanvas (octave::interpreter& interp,
+                      const graphics_handle& gh, QWidget *parent)
+    : Canvas (interp, gh, parent), m_glwidget (new GLWidget (*this))
+  { }
+
+  GLCanvas::~GLCanvas (void)
+  {
+    delete m_glwidget;
+  }
+
+  void
+  GLCanvas::draw (const graphics_handle& gh)
+  {
+    gh_manager& gh_mgr = m_interpreter.get_gh_manager ();
+
+    octave::autolock guard  (gh_mgr.graphics_lock ());
+
+    graphics_object go = gh_mgr.get_object (gh);
+
+    m_glwidget->draw (go);
+  }
+
+  uint8NDArray
+  GLCanvas::do_getPixels (const graphics_handle& gh)
+  {
+    uint8NDArray retval;
+
+    gh_manager& gh_mgr = m_interpreter.get_gh_manager ();
+
+    graphics_object go = gh_mgr.get_object (gh);
+
+    return m_glwidget->do_getPixels (go);
+  }
+
+  void
+  GLCanvas::do_print (const QString& file_cmd, const QString& term,
+                      const graphics_handle& handle)
+  {
+    gh_manager& gh_mgr = m_interpreter.get_gh_manager ();
+
+    octave::autolock guard  (gh_mgr.graphics_lock ());
+
+    graphics_object go = gh_mgr.get_object (handle);
+
+    try
+      {
+        m_glwidget->do_print (file_cmd, term, go);
+      }
+    catch (octave::execution_exception& ee)
+      {
+        emit interpreter_event
+          ([=] (void)
+          {
+            // INTERPRETER THREAD
+            throw ee;
+          });
+      }
+  }
+
+  graphics_object
+  GLCanvas::selectFromAxes (const graphics_object& ax, const QPoint& pt)
+  {
+    return m_glwidget->selectFromAxes (ax, pt);
+  }
+
+  void
+  GLCanvas::drawZoomBox (const QPoint& p1, const QPoint& p2)
+  {
+    m_glwidget->drawZoomBox (p1, p2);
+  }
+
+  bool
+  GLCanvas::begin_rendering (void)
+  {
+    return m_glwidget->begin_rendering ();
+  }
+
+  void
+  GLCanvas::end_rendering (void)
+  {
+    m_glwidget->end_rendering ();
+  }
+
 OCTAVE_END_NAMESPACE(octave)
--- a/libgui/graphics/GLCanvas.h	Fri Jan 20 15:33:05 2023 +0100
+++ b/libgui/graphics/GLCanvas.h	Fri Jan 20 08:49:19 2023 -0500
@@ -26,10 +26,10 @@
 #if ! defined (octave_GLCanvas_h)
 #define octave_GLCanvas_h 1
 
-#include <QOpenGLWidget>
+#include <QOffscreenSurface>
+#include <QOpenGLContext>
 #include <QOpenGLFramebufferObject>
-#include <QOpenGLContext>
-#include <QOffscreenSurface>
+#include <QOpenGLWidget>
 
 #include "Canvas.h"
 
@@ -38,27 +38,33 @@
 
 OCTAVE_BEGIN_NAMESPACE(octave)
 
-  class GLCanvas : public QOpenGLWidget, public Canvas
+  class GLWidget : public QOpenGLWidget
   {
+    Q_OBJECT
+
   public:
-    GLCanvas (octave::interpreter& interp,
-              const graphics_handle& handle, QWidget *parent);
-    ~GLCanvas (void);
+
+    GLWidget (Canvas& parent_canvas);
+
+    ~GLWidget (void);
 
     void initializeGL (void);
 
-    void draw (const graphics_handle& handle);
-    uint8NDArray  do_getPixels (const graphics_handle& handle);
+    void draw (graphics_object go);
+    uint8NDArray  do_getPixels (graphics_object go);
     void do_print (const QString& file_cmd, const QString& term,
-                   const graphics_handle& handle);
+                   graphics_object go);
     void drawZoomBox (const QPoint& p1, const QPoint& p2);
     void resize (int /* x */, int /* y */,
                  int /* width */, int /* height */) { }
     graphics_object selectFromAxes (const graphics_object& ax,
                                     const QPoint& pt);
-    QWidget * qWidget (void) { return this; }
+
+    bool begin_rendering (void);
+    void end_rendering (void);
 
   protected:
+
     void paintGL (void);
     void mouseDoubleClickEvent (QMouseEvent *event);
     void mouseMoveEvent (QMouseEvent *event);
@@ -70,16 +76,44 @@
 
   private:
 
-    bool begin_rendering (void);
-    void end_rendering (void);
+    Canvas& m_parent_canvas;
 
-    octave::qopengl_functions m_glfcns;
-    octave::opengl_renderer m_renderer;
+    qopengl_functions m_glfcns;
+    opengl_renderer m_renderer;
 
     QOpenGLContext m_os_context;
     QOffscreenSurface m_os_surface;
   };
 
+  class GLCanvas : public Canvas
+  {
+  public:
+
+    GLCanvas (octave::interpreter& interp, const graphics_handle& handle,
+              QWidget *parent);
+
+    ~GLCanvas (void);
+
+    void draw (const graphics_handle& handle);
+    uint8NDArray  do_getPixels (const graphics_handle& handle);
+    void do_print (const QString& file_cmd, const QString& term,
+                   const graphics_handle& handle);
+    void drawZoomBox (const QPoint& p1, const QPoint& p2);
+    void resize (int /* x */, int /* y */,
+                 int /* width */, int /* height */) { }
+    graphics_object selectFromAxes (const graphics_object& ax,
+                                    const QPoint& pt);
+
+    QWidget * qWidget (void) { return m_glwidget; }
+
+  private:
+
+    GLWidget *m_glwidget;
+
+    bool begin_rendering (void);
+    void end_rendering (void);
+  };
+
 OCTAVE_END_NAMESPACE(octave)
 
 #endif
--- a/libgui/graphics/module.mk	Fri Jan 20 15:33:05 2023 +0100
+++ b/libgui/graphics/module.mk	Fri Jan 20 08:49:19 2023 -0500
@@ -7,6 +7,7 @@
   %reldir%/moc-EditControl.cc \
   %reldir%/moc-Figure.cc \
   %reldir%/moc-FigureWindow.cc \
+  %reldir%/moc-GLCanvas.cc \
   %reldir%/moc-ListBoxControl.cc \
   %reldir%/moc-Menu.cc \
   %reldir%/moc-Object.cc \