Fix standalone batch renderer context deletion.

src/esp/gfx_batch/Renderer.h

@@ -447,7 +447,7 @@ class Renderer {
     create(configuration);
   }
 
-  ~Renderer();
+  virtual ~Renderer();
 
   /**
    * @brief Global renderer flags

src/esp/gfx_batch/RendererStandalone.cpp

@@ -135,8 +135,8 @@ RendererStandalone::RendererStandalone(
 }
 
 RendererStandalone::~RendererStandalone() {
-  /* Yup, shitty, but as we hold the GL context we can't let any GL resources
-     to be destructed after our destructor. Better ideas? */
+  /* As we hold the GL context, GL resources have to be destructed before this
+   * destructor. */
   Renderer::destroy();
 }
 

src/esp/gfx_batch/RendererStandalone.h

@@ -128,7 +128,7 @@ class RendererStandalone : public Renderer {
       const RendererConfiguration& configuration,
       const RendererStandaloneConfiguration& standaloneConfiguration);
 
-  ~RendererStandalone();
+  ~RendererStandalone() override;
 
   /**
    * @brief Global standalone renderer flags

src/tests/BatchReplayRendererTest.cpp

@@ -4,6 +4,7 @@
 
 #include "Corrade/Utility/Assert.h"
 #include "Magnum/DebugTools/Screenshot.h"
+#include "Magnum/GL/Context.h"
 #include "Magnum/Magnum.h"
 #include "Magnum/Trade/AbstractImageConverter.h"
 #include "configure.h"
@@ -157,55 +158,62 @@ void BatchReplayRendererTest::testIntegration() {
   ReplayRendererConfiguration batchRendererConfig;
   batchRendererConfig.sensorSpecifications = std::move(sensorSpecifications);
   batchRendererConfig.numEnvironments = numEnvs;
-  Cr::Containers::Pointer<esp::sim::AbstractReplayRenderer> renderer =
-      data.create(batchRendererConfig);
-
-  std::vector<std::vector<char>> buffers(numEnvs);
-  std::vector<Mn::MutableImageView2D> imageViews;
-
-  for (int envIndex = 0; envIndex < numEnvs; envIndex++) {
-    // TODO pass size as a Vector2i; use an Image instead of a std::vector
-    //  once there's Iterable<MutableImageView2D> that can be implicitly
-    //  converted from a list of Image2D.
-    imageViews.emplace_back(getRGBView(renderer->sensorSize(envIndex).x(),
-                                       renderer->sensorSize(envIndex).y(),
-                                       buffers[envIndex]));
-  }
+  {
+    Cr::Containers::Pointer<esp::sim::AbstractReplayRenderer> renderer =
+        data.create(batchRendererConfig);
+
+    // Check that the context is properly created
+    CORRADE_VERIFY(Mn::GL::Context::hasCurrent());
+
+    std::vector<std::vector<char>> buffers(numEnvs);
+    std::vector<Mn::MutableImageView2D> imageViews;
+
+    for (int envIndex = 0; envIndex < numEnvs; envIndex++) {
+      // TODO pass size as a Vector2i; use an Image instead of a std::vector
+      //  once there's Iterable<MutableImageView2D> that can be implicitly
+      //  converted from a list of Image2D.
+      imageViews.emplace_back(getRGBView(renderer->sensorSize(envIndex).x(),
+                                         renderer->sensorSize(envIndex).y(),
+                                         buffers[envIndex]));
+    }
 
-  for (int envIndex = 0; envIndex < numEnvs; envIndex++) {
-    renderer->setEnvironmentKeyframe(envIndex, serKeyframes[envIndex]);
-    renderer->setSensorTransformsFromKeyframe(envIndex, userPrefix);
-  }
+    for (int envIndex = 0; envIndex < numEnvs; envIndex++) {
+      renderer->setEnvironmentKeyframe(envIndex, serKeyframes[envIndex]);
+      renderer->setSensorTransformsFromKeyframe(envIndex, userPrefix);
+    }
 
-  renderer->render(imageViews);
+    renderer->render(imageViews);
 
-  for (int envIndex = 0; envIndex < numEnvs; envIndex++) {
-    CORRADE_ITERATION(envIndex);
-    std::string groundTruthImageFile =
-        screenshotPrefix + std::to_string(envIndex) + screenshotExtension;
-    CORRADE_COMPARE_WITH(
-        Mn::ImageView2D{imageViews[envIndex]},
-        Cr::Utility::Path::join(screenshotDir, groundTruthImageFile),
-        (Mn::DebugTools::CompareImageToFile{maxThreshold, meanThreshold}));
-  }
+    for (int envIndex = 0; envIndex < numEnvs; envIndex++) {
+      CORRADE_ITERATION(envIndex);
+      std::string groundTruthImageFile =
+          screenshotPrefix + std::to_string(envIndex) + screenshotExtension;
+      CORRADE_COMPARE_WITH(
+          Mn::ImageView2D{imageViews[envIndex]},
+          Cr::Utility::Path::join(screenshotDir, groundTruthImageFile),
+          (Mn::DebugTools::CompareImageToFile{maxThreshold, meanThreshold}));
+    }
 
-  const auto colorPtr = renderer->getCudaColorBufferDevicePointer();
-  const auto depthPtr = renderer->getCudaDepthBufferDevicePointer();
-  bool isBatchRenderer =
-      dynamic_cast<esp::sim::BatchReplayRenderer*>(renderer.get());
+    const auto colorPtr = renderer->getCudaColorBufferDevicePointer();
+    const auto depthPtr = renderer->getCudaDepthBufferDevicePointer();
+    bool isBatchRenderer =
+        dynamic_cast<esp::sim::BatchReplayRenderer*>(renderer.get());
 #ifdef ESP_BUILD_WITH_CUDA
-  if (isBatchRenderer) {
-    CORRADE_VERIFY(colorPtr);
-    CORRADE_VERIFY(depthPtr);
-  } else {
-    // Not implemented in ClassicReplayRenderer
+    if (isBatchRenderer) {
+      CORRADE_VERIFY(colorPtr);
+      CORRADE_VERIFY(depthPtr);
+    } else {
+      // Not implemented in ClassicReplayRenderer
+      CORRADE_VERIFY(!colorPtr);
+      CORRADE_VERIFY(!depthPtr);
+    }
+#else
     CORRADE_VERIFY(!colorPtr);
     CORRADE_VERIFY(!depthPtr);
-  }
-#else
-  CORRADE_VERIFY(!colorPtr);
-  CORRADE_VERIFY(!depthPtr);
 #endif
+  }
+  // Check that the context is properly deleted
+  CORRADE_VERIFY(!Mn::GL::Context::hasCurrent());
 }
 
 }  // namespace