Use new FBO wrapper in MouseSelector (openscad#5646)

* Use our own FBO wrapper in MouseSelector, to avoid mix QObjects with non-QObjects
* Move MouseSelector from MainWindow to QGLView to better manage OpenGL resource lifecycle
* Allocate modal dialogs on the stack to simplify memory management

src/glview/Camera.cc

@@ -8,8 +8,14 @@
 #include <memory>
 #include <vector>
 
-static const double DEFAULT_DISTANCE = 140.0;
-static const double DEFAULT_FOV = 22.5;
+namespace {
+
+constexpr double DEFAULT_DISTANCE = 140.0;
+constexpr double DEFAULT_FOV = 22.5;
+constexpr int DEFAULT_WIDTH = 512;
+constexpr int DEFAULT_HEIGHT = 512;
+
+}  // namespace
 
 Camera::Camera() : fov(DEFAULT_FOV)
 {
@@ -18,8 +24,8 @@ Camera::Camera() : fov(DEFAULT_FOV)
   // gimbal cam values
   resetView();
 
-  pixel_width = RenderSettings::inst()->img_width;
-  pixel_height = RenderSettings::inst()->img_height;
+  pixel_width = DEFAULT_WIDTH;
+  pixel_height = DEFAULT_HEIGHT;
   locked = false;
 }
 

src/glview/RenderSettings.cc

@@ -44,7 +44,5 @@ RenderSettings::RenderSettings() {
   backend3D = DEFAULT_RENDERING_BACKEND_3D;
   openCSGTermLimit = 100000;
   far_gl_clip_limit = 100000.0;
-  img_width = 512;
-  img_height = 512;
   colorscheme = "Cornfield";
 }

src/glview/RenderSettings.h

@@ -20,8 +20,6 @@ class RenderSettings
 
   RenderBackend3D backend3D;
   unsigned int openCSGTermLimit;
-  unsigned int img_width;
-  unsigned int img_height;
   double far_gl_clip_limit;
   std::string colorscheme;
 private:

src/glview/fbo.cc

@@ -61,7 +61,7 @@ std::unique_ptr<FBO> createFBO(int width, int height) {
   }
 }
 
-FBO::FBO(int width, int height, bool useEXT) : use_ext_(useEXT) {
+FBO::FBO(int width, int height, bool useEXT) : width_(width), height_(height), use_ext_(useEXT) {
   // Generate and bind FBO
   GL_CHECKD(glGenFramebuffers(1, &this->fbo_id_));
   this->bind();
@@ -112,6 +112,9 @@ bool FBO::resize(size_t width, size_t height)
   }
   GL_CHECKD(glRenderbufferStorage(GL_RENDERBUFFER, GL_DEPTH24_STENCIL8, width, height));
 
+  width_ = width;
+  height_ = height;
+
   return true;
 }
 

src/glview/fbo.h

@@ -10,7 +10,11 @@ class FBO
 public:
   FBO(int width, int height, bool useEXT);
   ~FBO() { destroy(); };
+
+  int width() const { return this->width_; }  
+  int height() const { return this->height_; }
   bool isComplete() const { return this->complete_; }
+
   bool resize(size_t width, size_t height);
   GLuint bind();
   void unbind();
@@ -19,6 +23,8 @@ class FBO
   void destroy();
 
   bool use_ext_;
+  int width_ = 0;
+  int height_ = 0;
   GLuint fbo_id_ = 0;
   GLuint old_fbo_id_ = 0;
   GLuint renderbuf_id_ = 0;

src/gui/MainWindow.cc

@@ -100,7 +100,6 @@
 #include "glview/preview/ThrownTogetherRenderer.h"
 #include "glview/preview/CSGTreeNormalizer.h"
 #include "gui/QGLView.h"
-#include "gui/MouseSelector.h"
 #ifdef Q_OS_MACOS
 #include "platform/CocoaUtils.h"
 #endif
@@ -723,7 +722,6 @@ MainWindow::MainWindow(const QStringList& filenames)
 
   updateExportActions();
 
-  this->selector = std::make_unique<MouseSelector>(this->qglview);
   activeEditor->setFocus();
 }
 
@@ -2221,30 +2219,19 @@ void MainWindow::leftClick(QPoint mouse)
  * Use the generated ID and try to find it within the list of products
  * And finally move the cursor to the beginning of the selected object in the editor
  */
-void MainWindow::rightClick(QPoint mouse)
+void MainWindow::rightClick(QPoint position)
 {
   // selecting without a renderer?!
   if (!this->qglview->renderer) {
     return;
   }
-
-  // selecting without select object?!
-  if (!this->selector) {
-    return;
-  }
-
   // Nothing to select
   if (!this->rootProduct) {
     return;
   }
 
-  this->qglview->renderer->prepare(&this->selector->shaderinfo);
-
-  // Update the selector with the right image size
-  this->selector->reset(this->qglview);
-
   // Select the object at mouse coordinates
-  int index = this->selector->select(this->qglview->getRenderer(), mouse.x(), mouse.y());
+  int index = this->qglview->pickObject(position);
   std::deque<std::shared_ptr<const AbstractNode>> path;
   std::shared_ptr<const AbstractNode> result = this->rootNode->getNodeByID(index, path);
 
@@ -2295,7 +2282,7 @@ void MainWindow::rightClick(QPoint mouse)
       }
     }
 
-    tracemenu.exec(this->qglview->mapToGlobal(mouse));
+    tracemenu.exec(this->qglview->mapToGlobal(position));
   } else {
     clearAllSelectionIndicators();
   }
@@ -2717,25 +2704,23 @@ void MainWindow::actionExportFileFormat(int fmt)
   switch (format) {
   case FileFormat::PDF:
   {
-    auto exportPdfDialog = new ExportPdfDialog();
-    exportPdfDialog->deleteLater();
-    if (exportPdfDialog->exec() == QDialog::Rejected) {
+    ExportPdfDialog exportPdfDialog;
+    if (exportPdfDialog.exec() == QDialog::Rejected) {
       return;
     }
 
-    exportInfo.optionsPdf = exportPdfDialog->getOptions();
+    exportInfo.optionsPdf = exportPdfDialog.getOptions();
     actionExport(2, exportInfo);
   }
   break;
   case FileFormat::_3MF:
   {
-    auto export3mfDialog = new Export3mfDialog();
-    export3mfDialog->deleteLater();
-    if (export3mfDialog->exec() == QDialog::Rejected) {
+    Export3mfDialog export3mfDialog;
+    if (export3mfDialog.exec() == QDialog::Rejected) {
       return;
     }
 
-    exportInfo.options3mf = export3mfDialog->getOptions();
+    exportInfo.options3mf = export3mfDialog.getOptions();
     actionExport(3, exportInfo);
   }
   break;

src/gui/MainWindow.h

@@ -92,7 +92,6 @@ class MainWindow : public QMainWindow, public Ui::MainWindow, public InputEventH
   std::shared_ptr<Renderer> cgalRenderer;
 #ifdef ENABLE_OPENCSG
   std::shared_ptr<Renderer> opencsgRenderer;
-  std::unique_ptr<class MouseSelector> selector;
 #endif
   std::shared_ptr<Renderer> thrownTogetherRenderer;
 

src/gui/MouseSelector.cc

@@ -1,9 +1,9 @@
 #include "gui/MouseSelector.h"
 
 #include "glview/system-gl.h"
+#include "glview/fbo.h"
 
 #include <cstdint>
-#include <QOpenGLFramebufferObject>
 #include <string>
 #include <memory>
 /**
@@ -21,11 +21,6 @@
 
 MouseSelector::MouseSelector(GLView *view) {
   this->view = view;
-  if (view && !view->has_shaders) {
-    return;
-  }
-  this->initShader();
-
   if (view) this->reset(view);
 }
 
@@ -34,49 +29,42 @@ MouseSelector::MouseSelector(GLView *view) {
  */
 void MouseSelector::reset(GLView *view) {
   this->view = view;
-  this->setupFramebuffer(view);
+  this->setupFramebuffer(view->cam.pixel_width, view->cam.pixel_height);
 }
 
 /**
  * Initialize the used shaders and setup the ShaderInfo struct
  */
 void MouseSelector::initShader() {
-  /*
-     Attributes:
-   * frag_idcolor - (uniform) 24 bit of the selected object's id encoded into R/G/B components as float values
-   */
-
-  const std::string vs_str = ShaderUtils::loadShaderSource("MouseSelector.vert");
-  const std::string fs_str = ShaderUtils::loadShaderSource("MouseSelector.frag");
-  const auto selectshader = ShaderUtils::compileShaderProgram(vs_str, fs_str);
-
-  this->shaderinfo.resource = selectshader;
-  this->shaderinfo.type = ShaderUtils::ShaderType::SELECT_RENDERING;
-  this->shaderinfo.uniforms = {
-    {"frag_idcolor", glGetUniformLocation(selectshader.shader_program, "frag_idcolor")},
-  };
+  // Attributes:
+  // frag_idcolor - (uniform) 24 bit of the selected object's id encoded into R/G/B components as float values 
+  const auto selectshader = ShaderUtils::compileShaderProgram(
+    ShaderUtils::loadShaderSource("MouseSelector.vert"),
+    ShaderUtils::loadShaderSource("MouseSelector.frag"));
 
   const GLint frag_idcolor = glGetUniformLocation(selectshader.shader_program, "frag_idcolor");
   if (frag_idcolor < 0) {
+    // TODO: Surface error better
     fprintf(stderr, __FILE__ ": OpenGL symbol retrieval went wrong, id is %i\n\n", frag_idcolor);
-    this->shaderinfo.uniforms["frag_idcolor"] = 0;
-  } else {
-    this->shaderinfo.uniforms["frag_idcolor"] = frag_idcolor;
   }
+  this->shaderinfo = {
+    .resource = selectshader,
+    .type = ShaderUtils::ShaderType::SELECT_RENDERING,
+    .uniforms = {
+      {"frag_idcolor", glGetUniformLocation(selectshader.shader_program, "frag_idcolor")},
+    },
+  };
 }
 
 /**
  * Resize or create the framebuffer
  */
-void MouseSelector::setupFramebuffer(const GLView *view) {
+void MouseSelector::setupFramebuffer(int width, int height) {
   if (!this->framebuffer ||
-      static_cast<unsigned int>(this->framebuffer->width()) != view->cam.pixel_width ||
-      static_cast<unsigned int>(this->framebuffer->height()) != view->cam.pixel_height) {
-    this->framebuffer = std::make_unique<QOpenGLFramebufferObject>(
-      view->cam.pixel_width,
-      view->cam.pixel_height,
-      QOpenGLFramebufferObject::Depth);
-    this->framebuffer->release();
+      this->framebuffer->width() != width ||
+      this->framebuffer->height() != height) {
+    this->framebuffer = createFBO(width, height);
+    this->initShader();
   }
 }
 
@@ -85,11 +73,17 @@ void MouseSelector::setupFramebuffer(const GLView *view) {
  * The renderer has to support rendering with ID colors (using the shader we provide),
  * otherwise the selection won't work.
  *
- * returns index of picked node (AbstractNode::idx) or 0 if no object was found.
+ * returns index of picked node (AbstractNode::idx) or -1 if no object was found.
  */
 int MouseSelector::select(const Renderer *renderer, int x, int y) {
-  // x/y is originated topleft, so turn y around
-  y = this->view->cam.pixel_height - y;
+  // This function should render a frame, as usual, with the following changes:
+  // * Render to as custom framebuffer
+  // * The shader should be the selector shader
+  // * Since we use ID color, no color setup is needed
+  // * No lighting
+  // * No decorations, like axes
+
+  // TODO: Ideally, we should make the above configurable and reduce duplicate render code in this function.
 
   if (x > static_cast<int>(this->view->cam.pixel_width) || x < 0 ||
       y > static_cast<int>(this->view->cam.pixel_height) || y < 0) {
@@ -127,13 +121,14 @@ int MouseSelector::select(const Renderer *renderer, int x, int y) {
 
   // Grab the color from the framebuffer and convert it back to an identifier
   GLubyte color[3] = { 0 };
-  GL_CHECKD(glReadPixels(x, y, 1, 1, GL_RGB, GL_UNSIGNED_BYTE, color));
+  // Qt position is originated top-left, so flip y to get GL coordinates.
+  GL_CHECKD(glReadPixels(x, this->view->cam.pixel_height - y, 1, 1, GL_RGB, GL_UNSIGNED_BYTE, color));
   glDisable(GL_DEPTH_TEST);
 
   const int index = (uint32_t)color[0] | ((uint32_t)color[1] << 8) | ((uint32_t)color[2] << 16);
 
   // Switch the active framebuffer back to the default
-  this->framebuffer->release();
+  this->framebuffer->unbind();
 
   return index;
 }

src/gui/MouseSelector.h

@@ -2,7 +2,7 @@
 
 #include "glview/GLView.h"
 #include "glview/Renderer.h"
-#include <QOpenGLFramebufferObject>
+#include "glview/fbo.h"
 
 #include <memory>
 
@@ -23,9 +23,9 @@ class MouseSelector
 
 private:
   void initShader();
-  void setupFramebuffer(const GLView *view);
+  void setupFramebuffer(int width, int height);
 
-  std::unique_ptr<QOpenGLFramebufferObject> framebuffer;
+  std::unique_ptr<FBO> framebuffer;
 
   GLView *view;
 };

src/gui/QGLView.cc

@@ -74,6 +74,7 @@ QGLView::QGLView(QWidget *parent) : QOpenGLWidget(parent)
 
 QGLView::~QGLView()
 {
+  std::cout << "QGLView::~QGLView()" << std::endl;
   // Just to make sure we can call GL functions in the supertype destructor
   makeCurrent();
 }
@@ -106,7 +107,7 @@ void QGLView::initializeGL()
 {
 #if defined(USE_GLEW) || defined(OPENCSG_GLEW)
   // Since OpenCSG requires glew, we need to initialize it.
-  // ..in a separate compilation unit to avoid duplicate symbols with GLAD.
+  // ..in a separate compilation unit to avoid duplicate symbols with x.
   initializeGlew();
 #endif
 #ifdef USE_GLAD
@@ -121,6 +122,8 @@ void QGLView::initializeGL()
   PRINTDB("GLAD: Loaded OpenGL %d.%d", GLAD_VERSION_MAJOR(version) % GLAD_VERSION_MINOR(version));
 #endif // ifdef USE_GLAD
   GLView::initializeGL();
+
+  this->selector = std::make_unique<MouseSelector>(this);
 }
 
 std::string QGLView::getRendererInfo() const
@@ -562,9 +565,22 @@ std::vector<SelectedObject> QGLView::findObject(int mouse_x,int mouse_y)
 
 void QGLView::selectPoint(int mouse_x, int mouse_y)
 {
-  std::vector<SelectedObject>  obj= findObject(mouse_x, mouse_y);
-  if(obj.size() == 1) {
+  std::vector<SelectedObject> obj= findObject(mouse_x, mouse_y);
+  if (obj.size() == 1) {
     this->selected_obj.push_back(obj[0]);
     update();
   }
 }
+
+int QGLView::pickObject(QPoint position)
+{
+  if (!isValid()) return -1;
+
+// FIXME: If renderer isn't prepared, we cannot call this 
+//  renderer->prepare(&this->selector->shaderinfo);
+
+  // Update the selector with the right image size
+  this->selector->reset(this);
+
+  return this->selector->select(this->getRenderer(), position.x(), position.y());
+}