From a6f048e342564206aa7b4d70702e4bdf96aebc5d Mon Sep 17 00:00:00 2001 From: Alexander Bock Date: Sun, 20 Feb 2022 19:17:06 +0100 Subject: [PATCH] Add explicit checks whenever we are accessing another scene graph node (closes #1831) --- modules/base/rendering/renderablenodeline.cpp | 44 +++++++++---------- modules/base/rendering/renderablenodeline.h | 2 +- modules/globebrowsing/src/globerotation.cpp | 2 +- .../globebrowsing/src/globetranslation.cpp | 4 +- modules/globebrowsing/src/renderableglobe.cpp | 16 +++---- modules/globebrowsing/src/ringscomponent.cpp | 17 ++++--- modules/globebrowsing/src/shadowcomponent.cpp | 8 +++- modules/space/rendering/renderablerings.cpp | 10 ++++- .../rendering/renderablemodelprojection.cpp | 13 +++--- .../rendering/renderabledistancelabel.cpp | 16 +++++-- src/interaction/sessionrecording.cpp | 1 - support/coding/codegen | 2 +- 12 files changed, 80 insertions(+), 55 deletions(-) diff --git a/modules/base/rendering/renderablenodeline.cpp b/modules/base/rendering/renderablenodeline.cpp index 64f48ab481..d0aad7ec44 100644 --- a/modules/base/rendering/renderablenodeline.cpp +++ b/modules/base/rendering/renderablenodeline.cpp @@ -117,11 +117,9 @@ RenderableNodeLine::RenderableNodeLine(const ghoul::Dictionary& dictionary) const Parameters p = codegen::bake(dictionary); _start = p.startNode.value_or(_start); - _start.onChange([&]() { validateNodes(); }); addProperty(_start); _end = p.endNode.value_or(_end); - _end.onChange([&]() { validateNodes(); }); addProperty(_end); _lineColor = p.color.value_or(_lineColor); @@ -203,15 +201,26 @@ void RenderableNodeLine::bindGL() { } void RenderableNodeLine::updateVertexData() { + SceneGraphNode* startNode = global::renderEngine->scene()->sceneGraphNode(_start); + SceneGraphNode* endNode = global::renderEngine->scene()->sceneGraphNode(_end); + + if (!startNode || !endNode) { + LERRORC( + "RenderableNodeLine", + fmt::format( + "Could not find starting '{}' or ending '{}'", + _start.value(), _end.value() + ) + ); + + return; + } + _vertexArray.clear(); // Update the positions of the nodes - _startPos = coordinatePosFromAnchorNode( - global::renderEngine->scene()->sceneGraphNode(_start)->worldPosition() - ); - _endPos = coordinatePosFromAnchorNode( - global::renderEngine->scene()->sceneGraphNode(_end)->worldPosition() - ); + _startPos = coordinatePosFromAnchorNode(startNode->worldPosition()); + _endPos = coordinatePosFromAnchorNode(endNode->worldPosition()); _vertexArray.push_back(static_cast(_startPos.x)); _vertexArray.push_back(static_cast(_startPos.y)); @@ -235,9 +244,11 @@ void RenderableNodeLine::updateVertexData() { unbindGL(); } -void RenderableNodeLine::render(const RenderData& data, RendererTasks&) { +void RenderableNodeLine::update(const UpdateData&) { updateVertexData(); +} +void RenderableNodeLine::render(const RenderData& data, RendererTasks&) { _program->activate(); glm::dmat4 anchorTranslation(1.0); @@ -278,19 +289,4 @@ void RenderableNodeLine::render(const RenderData& data, RendererTasks&) { global::renderEngine->openglStateCache().resetLineState(); } -void RenderableNodeLine::validateNodes() { - if (!global::renderEngine->scene()->sceneGraphNode(_start)) { - LERROR(fmt::format( - "There is no scenegraph node with id {}, defaults to 'Root'", _start - )); - _start = Root; - } - if (!global::renderEngine->scene()->sceneGraphNode(_end)) { - LERROR(fmt::format( - "There is no scenegraph node with id {}, defaults to 'Root'", _end - )); - _end = Root; - } -} - } // namespace openspace diff --git a/modules/base/rendering/renderablenodeline.h b/modules/base/rendering/renderablenodeline.h index a8b555943b..23761c5c4a 100644 --- a/modules/base/rendering/renderablenodeline.h +++ b/modules/base/rendering/renderablenodeline.h @@ -62,8 +62,8 @@ private: bool isReady() const override; void updateVertexData(); + void update(const UpdateData& data) override; void render(const RenderData& data, RendererTasks& rendererTask) override; - void validateNodes(); void unbindGL(); void bindGL(); diff --git a/modules/globebrowsing/src/globerotation.cpp b/modules/globebrowsing/src/globerotation.cpp index 5877c369ba..2d3bf8c077 100644 --- a/modules/globebrowsing/src/globerotation.cpp +++ b/modules/globebrowsing/src/globerotation.cpp @@ -130,7 +130,7 @@ GlobeRotation::GlobeRotation(const ghoul::Dictionary& dictionary) void GlobeRotation::findGlobe() { SceneGraphNode* n = sceneGraphNode(_globe); - if (n->renderable() && dynamic_cast(n->renderable())) { + if (n && n->renderable() && dynamic_cast(n->renderable())) { _globeNode = dynamic_cast(n->renderable()); } else { diff --git a/modules/globebrowsing/src/globetranslation.cpp b/modules/globebrowsing/src/globetranslation.cpp index b245d3320f..e6e027fffb 100644 --- a/modules/globebrowsing/src/globetranslation.cpp +++ b/modules/globebrowsing/src/globetranslation.cpp @@ -133,7 +133,7 @@ GlobeTranslation::GlobeTranslation(const ghoul::Dictionary& dictionary) void GlobeTranslation::fillAttachedNode() { SceneGraphNode* n = sceneGraphNode(_globe); - if (n->renderable() && dynamic_cast(n->renderable())) { + if (n && n->renderable() && dynamic_cast(n->renderable())) { _attachedNode = dynamic_cast(n->renderable()); } else { @@ -142,7 +142,7 @@ void GlobeTranslation::fillAttachedNode() { "Could not set attached node as it does not have a RenderableGlobe" ); if (_attachedNode) { - // Reset the globe name to it's previous name + // Reset the globe name to its previous name _globe = _attachedNode->identifier(); } } diff --git a/modules/globebrowsing/src/renderableglobe.cpp b/modules/globebrowsing/src/renderableglobe.cpp index 3458fae95a..07aa2438a0 100644 --- a/modules/globebrowsing/src/renderableglobe.cpp +++ b/modules/globebrowsing/src/renderableglobe.cpp @@ -2033,6 +2033,14 @@ void RenderableGlobe::calculateEclipseShadows(ghoul::opengl::ProgramObject& prog SceneGraphNode* casterNode = global::renderEngine->scene()->sceneGraphNode(caster); + if ((sourceNode == nullptr) || (casterNode == nullptr)) { + LERRORC( + "Renderableglobe", + "Invalid scenegraph node for the shadow's caster or shadow's receiver." + ); + return; + } + const double sourceRadiusScale = std::max( glm::compMax(sourceNode->scale()), 1.0 @@ -2043,14 +2051,6 @@ void RenderableGlobe::calculateEclipseShadows(ghoul::opengl::ProgramObject& prog 1.0 ); - if ((sourceNode == nullptr) || (casterNode == nullptr)) { - LERRORC( - "Renderableglobe", - "Invalid scenegraph node for the shadow's caster or shadow's receiver." - ); - return; - } - // First we determine if the caster is shadowing the current planet (all // calculations in World Coordinates): const glm::dvec3 planetCasterVec = casterPos - data.modelTransform.translation; diff --git a/modules/globebrowsing/src/ringscomponent.cpp b/modules/globebrowsing/src/ringscomponent.cpp index 5ce334a5d1..31e727a7fc 100644 --- a/modules/globebrowsing/src/ringscomponent.cpp +++ b/modules/globebrowsing/src/ringscomponent.cpp @@ -608,10 +608,18 @@ void RingsComponent::update(const UpdateData& data) { _textureIsDirty = false; } - _sunPosition = glm::normalize( - global::renderEngine->scene()->sceneGraphNode("Sun")->worldPosition() - - data.modelTransform.translation - ); + // @TODO (abock, 2022-02-20) This should be replaced with the more general light + // source solution that we are using in other places + SceneGraphNode* sun = global::renderEngine->scene()->sceneGraphNode("Sun"); + if (sun) { + _sunPosition = glm::normalize( + sun->worldPosition() - data.modelTransform.translation + ); + } + else { + // If the Sun node is not found, we assume the light source to be in the origin + _sunPosition = glm::normalize(-data.modelTransform.translation); + } } void RingsComponent::loadTexture() { @@ -619,7 +627,6 @@ void RingsComponent::loadTexture() { using namespace ghoul::opengl; if (!_texturePath.value().empty()) { - std::unique_ptr texture = TextureReader::ref().loadTexture( absPath(_texturePath).string(), 1 diff --git a/modules/globebrowsing/src/shadowcomponent.cpp b/modules/globebrowsing/src/shadowcomponent.cpp index c6be39dabd..bf6458917c 100644 --- a/modules/globebrowsing/src/shadowcomponent.cpp +++ b/modules/globebrowsing/src/shadowcomponent.cpp @@ -356,7 +356,13 @@ void ShadowComponent::end() { void ShadowComponent::update(const UpdateData&) { ZoneScoped - _sunPosition = global::renderEngine->scene()->sceneGraphNode("Sun")->worldPosition(); + SceneGraphNode* sun = global::renderEngine->scene()->sceneGraphNode("Sun"); + if (sun) { + _sunPosition = sun->worldPosition(); + } + else { + _sunPosition = glm::dvec3(0.0); + } glm::ivec2 renderingResolution = global::renderEngine->renderingResolution(); if (_dynamicDepthTextureRes && ((_shadowDepthTextureWidth != renderingResolution.x) || diff --git a/modules/space/rendering/renderablerings.cpp b/modules/space/rendering/renderablerings.cpp index 4e0d3b7fcf..05d7f47d9d 100644 --- a/modules/space/rendering/renderablerings.cpp +++ b/modules/space/rendering/renderablerings.cpp @@ -226,8 +226,14 @@ void RenderableRings::update(const UpdateData& data) { _textureIsDirty = false; } - _sunPosition = global::renderEngine->scene()->sceneGraphNode("Sun")->worldPosition() - - data.modelTransform.translation; + SceneGraphNode* sun = global::renderEngine->scene()->sceneGraphNode("Sun"); + if (sun) { + _sunPosition = sun->worldPosition() - data.modelTransform.translation; + } + else { + // If the Sun node does not exist, we assume the light source to be in the origin + _sunPosition = -data.modelTransform.translation; + } } void RenderableRings::loadTexture() { diff --git a/modules/spacecraftinstruments/rendering/renderablemodelprojection.cpp b/modules/spacecraftinstruments/rendering/renderablemodelprojection.cpp index baf9843b89..c30c2d6248 100644 --- a/modules/spacecraftinstruments/rendering/renderablemodelprojection.cpp +++ b/modules/spacecraftinstruments/rendering/renderablemodelprojection.cpp @@ -319,11 +319,14 @@ void RenderableModelProjection::update(const UpdateData& data) { } } - glm::dvec3 p = - global::renderEngine->scene()->sceneGraphNode("Sun")->worldPosition() - - data.modelTransform.translation; - - _sunPosition = static_cast(p); + SceneGraphNode* sun = global::renderEngine->scene()->sceneGraphNode("Sun"); + if (sun) { + _sunPosition = sun->worldPosition() - data.modelTransform.translation; + } + else { + // If the Sun node doesn't exist, we assume that the light source is in the origin + _sunPosition = -data.modelTransform.translation; + } } void RenderableModelProjection::imageProjectGPU( diff --git a/modules/vislab/rendering/renderabledistancelabel.cpp b/modules/vislab/rendering/renderabledistancelabel.cpp index d78cf93d6b..8226b2809a 100644 --- a/modules/vislab/rendering/renderabledistancelabel.cpp +++ b/modules/vislab/rendering/renderabledistancelabel.cpp @@ -136,11 +136,19 @@ void RenderableDistanceLabel::update(const UpdateData&) { // Update placement of label with transformation matrix SceneGraphNode* startNode = RE.scene()->sceneGraphNode(nodeline->start()); - glm::dvec3 start = startNode->worldPosition(); SceneGraphNode* endNode = RE.scene()->sceneGraphNode(nodeline->end()); - glm::dvec3 end = endNode->worldPosition(); - glm::dvec3 goalPos = start + (end - start) / 2.0; - _transformationMatrix = glm::translate(glm::dmat4(1.0), goalPos); + if (startNode && endNode) { + glm::dvec3 start = startNode->worldPosition(); + glm::dvec3 end = endNode->worldPosition(); + glm::dvec3 goalPos = start + (end - start) / 2.0; + _transformationMatrix = glm::translate(glm::dmat4(1.0), goalPos); + } + else { + LERROR(fmt::format( + "Could not find scene graph node {} or {}", + nodeline->start(), nodeline->end() + )); + } } else { LERROR(fmt::format("There is no scenegraph node with id {}", _nodelineId)); diff --git a/src/interaction/sessionrecording.cpp b/src/interaction/sessionrecording.cpp index 4b4d8e9b5d..76c5f9dcd0 100644 --- a/src/interaction/sessionrecording.cpp +++ b/src/interaction/sessionrecording.cpp @@ -2004,7 +2004,6 @@ bool SessionRecording::processCameraKeyframe(double now) { Scene* scene = camera->parent()->scene(); const SceneGraphNode* n = scene->sceneGraphNode(_keyframesCamera[prevIdx].focusNode); - if (n) { global::navigationHandler->orbitalNavigator().setFocusNode(n->identifier()); } diff --git a/support/coding/codegen b/support/coding/codegen index 815e975020..ad3aa1e5e1 160000 --- a/support/coding/codegen +++ b/support/coding/codegen @@ -1 +1 @@ -Subproject commit 815e9750201a6bcf40b1b8768c88d4dc4ac9674a +Subproject commit ad3aa1e5e1a3656316f2fc94bf60f31090872e4a