From a1778638c895c6d21fbd41b93bb927f9988ca056 Mon Sep 17 00:00:00 2001
From: Evgenii Startcev <slayer-gurgen@yandex.ru>
Date: Fri, 14 Mar 2025 22:03:00 +0100
Subject: [PATCH 1/7] context: Add exception handling
---
application/F3DStarter.cxx | 51 +++++++++++++++++++++++++++-----------
1 file changed, 37 insertions(+), 14 deletions(-)
diff --git a/application/F3DStarter.cxx b/application/F3DStarter.cxx
index 3995f1cd4d..b72c5509e7 100644
--- a/application/F3DStarter.cxx
+++ b/application/F3DStarter.cxx
@@ -936,29 +936,52 @@ int F3DStarter::Start(int argc, char** argv)
bool offscreen = !this->Internals->AppOptions.Reference.empty() ||
!this->Internals->AppOptions.Output.empty() || this->Internals->AppOptions.BindingsList;
- if (this->Internals->AppOptions.RenderingBackend == "egl")
+ try
{
- this->Internals->Engine = std::make_unique<f3d::engine>(f3d::engine::createEGL());
+ if (this->Internals->AppOptions.RenderingBackend == "egl")
+ {
+ this->Internals->Engine = std::make_unique<f3d::engine>(f3d::engine::createEGL());
+ }
+ else if (this->Internals->AppOptions.RenderingBackend == "osmesa")
+ {
+ this->Internals->Engine = std::make_unique<f3d::engine>(f3d::engine::createOSMesa());
+ }
+ else if (this->Internals->AppOptions.RenderingBackend == "glx")
+ {
+ this->Internals->Engine = std::make_unique<f3d::engine>(f3d::engine::createGLX(offscreen));
+ }
+ else if (this->Internals->AppOptions.RenderingBackend == "wgl")
+ {
+ this->Internals->Engine = std::make_unique<f3d::engine>(f3d::engine::createWGL(offscreen));
+ }
+ else
+ {
+ if (this->Internals->AppOptions.RenderingBackend != "auto")
+ {
+ f3d::log::warn("--rendering-backend value is invalid, falling back to \"auto\"");
+ }
+ this->Internals->Engine = std::make_unique<f3d::engine>(f3d::engine::create(offscreen));
+ }
}
- else if (this->Internals->AppOptions.RenderingBackend == "osmesa")
+ catch (const f3d::context::loading_exception& ex)
{
- this->Internals->Engine = std::make_unique<f3d::engine>(f3d::engine::createOSMesa());
+ f3d::log::error("Could not load graphic library: ", ex.what());
+ return EXIT_FAILURE;
}
- else if (this->Internals->AppOptions.RenderingBackend == "glx")
+ catch (const f3d::context::symbol_exception& ex)
{
- this->Internals->Engine = std::make_unique<f3d::engine>(f3d::engine::createGLX(offscreen));
+ f3d::log::error("Could not find needed symbol in graphic library: ", ex.what());
+ return EXIT_FAILURE;
}
- else if (this->Internals->AppOptions.RenderingBackend == "wgl")
+ catch (const f3d::engine::no_window_exception& ex)
{
- this->Internals->Engine = std::make_unique<f3d::engine>(f3d::engine::createWGL(offscreen));
+ f3d::log::error("Could not create the window: ", ex.what());
+ return EXIT_FAILURE;
}
- else
+ catch (const f3d::engine::cache_exception& ex)
{
- if (this->Internals->AppOptions.RenderingBackend != "auto")
- {
- f3d::log::warn("--rendering-backend value is invalid, falling back to \"auto\"");
- }
- this->Internals->Engine = std::make_unique<f3d::engine>(f3d::engine::create(offscreen));
+ f3d::log::error("Could not use default cache directory: ", ex.what());
+ return EXIT_FAILURE;
}
this->ResetWindowName();
From 58f8310a0f5b9c1b8ce98b21e6cefe6d5e5a4f73 Mon Sep 17 00:00:00 2001
From: Evgenii Startcev <slayer-gurgen@yandex.ru>
Date: Fri, 14 Mar 2025 23:22:17 +0100
Subject: [PATCH 2/7] engine: Fix memory leak
Essentially changed internals to unique_ptr
---
library/public/engine.h | 2 +-
library/src/engine.cxx | 10 +++-------
2 files changed, 4 insertions(+), 8 deletions(-)
diff --git a/library/public/engine.h b/library/public/engine.h
index 32760408f9..e9b8313b43 100644
--- a/library/public/engine.h
+++ b/library/public/engine.h
@@ -352,7 +352,7 @@ class F3D_EXPORT engine
private:
class internals;
- internals* Internals;
+ std::unique_ptr<internals> Internals;
/**
* Engine constructor. This is a private method.
diff --git a/library/src/engine.cxx b/library/src/engine.cxx
index 3557b0bc38..0d5ddeb675 100644
--- a/library/src/engine.cxx
+++ b/library/src/engine.cxx
@@ -47,7 +47,7 @@ class engine::internals
//----------------------------------------------------------------------------
engine::engine(
const std::optional<window::Type>& windowType, bool offscreen, const context::function& loader)
- : Internals(new engine::internals)
+ : Internals(std::make_unique<engine::internals>())
{
// Ensure all lib initialization is done (once)
detail::init::initialize();
@@ -182,22 +182,18 @@ engine engine::createExternalOSMesa()
//----------------------------------------------------------------------------
engine::~engine()
{
- delete this->Internals;
}
//----------------------------------------------------------------------------
engine::engine(engine&& other) noexcept
{
- this->Internals = other.Internals;
- other.Internals = nullptr;
+ this->Internals = std::move(other.Internals);
}
//----------------------------------------------------------------------------
engine& engine::operator=(engine&& other) noexcept
{
- delete this->Internals;
- this->Internals = other.Internals;
- other.Internals = nullptr;
+ this->Internals = std::move(other.Internals);
return *this;
}
From 6167dbef678a01c016bc84084cf1c35a10b6ac59 Mon Sep 17 00:00:00 2001
From: Evgenii Startcev <slayer-gurgen@yandex.ru>
Date: Fri, 14 Mar 2025 23:28:57 +0100
Subject: [PATCH 3/7] context: Add no cache test case
Also remove symbol loading exception catch as it should not be possible
to happen in this place unless GL wrapper library is faulty somehow
---
application/F3DStarter.cxx | 5 -----
application/testing/CMakeLists.txt | 1 +
2 files changed, 1 insertion(+), 5 deletions(-)
diff --git a/application/F3DStarter.cxx b/application/F3DStarter.cxx
index b72c5509e7..fe30c0ad1d 100644
--- a/application/F3DStarter.cxx
+++ b/application/F3DStarter.cxx
@@ -968,11 +968,6 @@ int F3DStarter::Start(int argc, char** argv)
f3d::log::error("Could not load graphic library: ", ex.what());
return EXIT_FAILURE;
}
- catch (const f3d::context::symbol_exception& ex)
- {
- f3d::log::error("Could not find needed symbol in graphic library: ", ex.what());
- return EXIT_FAILURE;
- }
catch (const f3d::engine::no_window_exception& ex)
{
f3d::log::error("Could not create the window: ", ex.what());
diff --git a/application/testing/CMakeLists.txt b/application/testing/CMakeLists.txt
index 55f8fec39a..286c9d041a 100644
--- a/application/testing/CMakeLists.txt
+++ b/application/testing/CMakeLists.txt
@@ -1135,6 +1135,7 @@ f3d_test(NAME TestBindingsList ARGS --list-bindings REGEXP "Any.5 Toggle
# Test rendering backends
# For some reason the sanitizer detects leaks because of EGL and OSMesa
+f3d_test(NAME TestInvalidCache ENV "HOME=" REGEXP "Could not use default cache directory" NO_BASELINE)
f3d_test(NAME TestRenderingBackendAuto DATA cow.vtp RENDERING_BACKEND auto)
if(UNIX AND NOT APPLE AND VTK_VERSION VERSION_GREATER_EQUAL 9.3.20240914 AND F3D_SANITIZER STREQUAL "none")
if(F3D_TESTING_ENABLE_EGL_TESTS)
From f5929773a55f770f0c2290ebfa26e1ec9278013f Mon Sep 17 00:00:00 2001
From: Evgenii Startcev <slayer-gurgen@yandex.ru>
Date: Fri, 14 Mar 2025 23:43:45 +0100
Subject: [PATCH 4/7] Revert back to raw pointer still fixing the leak
---
library/public/engine.h | 2 +-
library/src/engine.cxx | 11 ++++++++---
2 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/library/public/engine.h b/library/public/engine.h
index e9b8313b43..32760408f9 100644
--- a/library/public/engine.h
+++ b/library/public/engine.h
@@ -352,7 +352,7 @@ class F3D_EXPORT engine
private:
class internals;
- std::unique_ptr<internals> Internals;
+ internals* Internals;
/**
* Engine constructor. This is a private method.
diff --git a/library/src/engine.cxx b/library/src/engine.cxx
index 0d5ddeb675..8d106273c5 100644
--- a/library/src/engine.cxx
+++ b/library/src/engine.cxx
@@ -47,7 +47,7 @@ class engine::internals
//----------------------------------------------------------------------------
engine::engine(
const std::optional<window::Type>& windowType, bool offscreen, const context::function& loader)
- : Internals(std::make_unique<engine::internals>())
+ : Internals(new engine::internals)
{
// Ensure all lib initialization is done (once)
detail::init::initialize();
@@ -85,6 +85,7 @@ engine::engine(
#endif
if (cachePath.empty())
{
+ delete Internals;
throw engine::cache_exception(
"Could not setup cache, please set the appropriate environment variable");
}
@@ -182,18 +183,22 @@ engine engine::createExternalOSMesa()
//----------------------------------------------------------------------------
engine::~engine()
{
+ delete this->Internals;
}
//----------------------------------------------------------------------------
engine::engine(engine&& other) noexcept
{
- this->Internals = std::move(other.Internals);
+ this->Internals = other.Internals;
+ other.Internals = nullptr;
}
//----------------------------------------------------------------------------
engine& engine::operator=(engine&& other) noexcept
{
- this->Internals = std::move(other.Internals);
+ delete this->Internals;
+ this->Internals = other.Internals;
+ other.Internals = nullptr;
return *this;
}
From a389f35b2e0b81cb41f453038431121d1b5c2760 Mon Sep 17 00:00:00 2001
From: Evgenii Startcev <slayer-gurgen@yandex.ru>
Date: Sat, 15 Mar 2025 00:15:25 +0100
Subject: [PATCH 5/7] context: Move test to non-Windows
---
application/testing/CMakeLists.txt | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/application/testing/CMakeLists.txt b/application/testing/CMakeLists.txt
index 286c9d041a..f3fcbb3528 100644
--- a/application/testing/CMakeLists.txt
+++ b/application/testing/CMakeLists.txt
@@ -1135,8 +1135,10 @@ f3d_test(NAME TestBindingsList ARGS --list-bindings REGEXP "Any.5 Toggle
# Test rendering backends
# For some reason the sanitizer detects leaks because of EGL and OSMesa
-f3d_test(NAME TestInvalidCache ENV "HOME=" REGEXP "Could not use default cache directory" NO_BASELINE)
f3d_test(NAME TestRenderingBackendAuto DATA cow.vtp RENDERING_BACKEND auto)
+if(NOT WIN32)
+ f3d_test(NAME TestInvalidCache ENV "HOME=" REGEXP "Could not use default cache directory" NO_BASELINE)
+endif()
if(UNIX AND NOT APPLE AND VTK_VERSION VERSION_GREATER_EQUAL 9.3.20240914 AND F3D_SANITIZER STREQUAL "none")
if(F3D_TESTING_ENABLE_EGL_TESTS)
f3d_test(NAME TestRenderingBackendEGL DATA cow.vtp RENDERING_BACKEND egl)
From 74491284e83256f30bef562031804db469183b6a Mon Sep 17 00:00:00 2001
From: Evgenii Startcev <slayer-gurgen@yandex.ru>
Date: Sat, 15 Mar 2025 10:23:20 +0100
Subject: [PATCH 6/7] context: bring back symbol exception handling
Also move test case location
---
application/F3DStarter.cxx | 5 +++++
application/testing/CMakeLists.txt | 4 +---
2 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/application/F3DStarter.cxx b/application/F3DStarter.cxx
index fe30c0ad1d..b72c5509e7 100644
--- a/application/F3DStarter.cxx
+++ b/application/F3DStarter.cxx
@@ -968,6 +968,11 @@ int F3DStarter::Start(int argc, char** argv)
f3d::log::error("Could not load graphic library: ", ex.what());
return EXIT_FAILURE;
}
+ catch (const f3d::context::symbol_exception& ex)
+ {
+ f3d::log::error("Could not find needed symbol in graphic library: ", ex.what());
+ return EXIT_FAILURE;
+ }
catch (const f3d::engine::no_window_exception& ex)
{
f3d::log::error("Could not create the window: ", ex.what());
diff --git a/application/testing/CMakeLists.txt b/application/testing/CMakeLists.txt
index f3fcbb3528..d1c7ffca7c 100644
--- a/application/testing/CMakeLists.txt
+++ b/application/testing/CMakeLists.txt
@@ -1136,9 +1136,6 @@ f3d_test(NAME TestBindingsList ARGS --list-bindings REGEXP "Any.5 Toggle
# Test rendering backends
# For some reason the sanitizer detects leaks because of EGL and OSMesa
f3d_test(NAME TestRenderingBackendAuto DATA cow.vtp RENDERING_BACKEND auto)
-if(NOT WIN32)
- f3d_test(NAME TestInvalidCache ENV "HOME=" REGEXP "Could not use default cache directory" NO_BASELINE)
-endif()
if(UNIX AND NOT APPLE AND VTK_VERSION VERSION_GREATER_EQUAL 9.3.20240914 AND F3D_SANITIZER STREQUAL "none")
if(F3D_TESTING_ENABLE_EGL_TESTS)
f3d_test(NAME TestRenderingBackendEGL DATA cow.vtp RENDERING_BACKEND egl)
@@ -1190,6 +1187,7 @@ set_tests_properties(f3d::TestNoFileFileNameTemplate PROPERTIES PASS_REGULAR_EXP
set_tests_properties(f3d::TestNoFileFileNameTemplate PROPERTIES ENVIRONMENT "CTEST_F3D_NO_DATA_FORCE_RENDER=1")
if(NOT WIN32)
+ f3d_test(NAME TestInvalidCache ENV "HOME=" REGEXP "Could not use default cache directory" NO_BASELINE)
add_test(NAME f3d::TestHOMEOutput COMMAND $<TARGET_FILE:f3d> ${F3D_SOURCE_DIR}/testing/data/suzanne.stl --output=~/Testing/Temporary/TestHOMEOutput.png --resolution=300,300 --no-config)
set_tests_properties(f3d::TestHOMEOutput PROPERTIES ENVIRONMENT "HOME=${CMAKE_BINARY_DIR}")
set_tests_properties(f3d::TestHOMEOutput PROPERTIES FIXTURES_SETUP f3d::TestHOMEOutput_FIXTURE)
From 4f5465b5a842afcb83bc2250965b4959d985180c Mon Sep 17 00:00:00 2001
From: Evgenii Startcev <slayer-gurgen@yandex.ru>
Date: Sat, 15 Mar 2025 10:57:13 +0100
Subject: [PATCH 7/7] context: Clearer test declaration
---
application/testing/CMakeLists.txt | 1 +
1 file changed, 1 insertion(+)
diff --git a/application/testing/CMakeLists.txt b/application/testing/CMakeLists.txt
index d1c7ffca7c..df948e74a2 100644
--- a/application/testing/CMakeLists.txt
+++ b/application/testing/CMakeLists.txt
@@ -1188,6 +1188,7 @@ set_tests_properties(f3d::TestNoFileFileNameTemplate PROPERTIES ENVIRONMENT "CTE
if(NOT WIN32)
f3d_test(NAME TestInvalidCache ENV "HOME=" REGEXP "Could not use default cache directory" NO_BASELINE)
+
add_test(NAME f3d::TestHOMEOutput COMMAND $<TARGET_FILE:f3d> ${F3D_SOURCE_DIR}/testing/data/suzanne.stl --output=~/Testing/Temporary/TestHOMEOutput.png --resolution=300,300 --no-config)
set_tests_properties(f3d::TestHOMEOutput PROPERTIES ENVIRONMENT "HOME=${CMAKE_BINARY_DIR}")
set_tests_properties(f3d::TestHOMEOutput PROPERTIES FIXTURES_SETUP f3d::TestHOMEOutput_FIXTURE)