Skip to content

Commit eea3102

Browse files
prybickiprybicki
authored
Remove internal usage of multiple scenes (#213)
* Remove internal usage of multiple scenes * Review fixes
1 parent ae14a87 commit eea3102

File tree

10 files changed

+47
-71
lines changed

10 files changed

+47
-71
lines changed

‎extensions/ros2/src/graph/Ros2PublishPointsNode.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -80,8 +80,8 @@ void Ros2PublishPointsNode::enqueueExecImpl()
8080
ros2Message.row_step = ros2Message.point_step * ros2Message.width;
8181
// TODO(msz-rai): Assign scene to the Graph.
8282
// For now, only default scene is supported.
83-
ros2Message.header.stamp = Scene::defaultInstance()->getTime().has_value() ?
84-
Scene::defaultInstance()->getTime()->asRos2Msg() :
83+
ros2Message.header.stamp = Scene::instance().getTime().has_value() ?
84+
Scene::instance().getTime()->asRos2Msg() :
8585
static_cast<builtin_interfaces::msg::Time>(ros2Node->get_clock()->now());
8686
if (!rclcpp::ok()) {
8787
throw std::runtime_error("Unable to publish a message because ROS2 has been shut down.");

‎src/api/apiCore.cpp

Lines changed: 15 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,7 @@ RGL_API rgl_status_t rgl_cleanup(void)
147147
Entity::instances.clear();
148148
Mesh::instances.clear();
149149
Texture::instances.clear();
150-
Scene::defaultInstance()->clear();
150+
Scene::instance().clear();
151151
});
152152
TAPE_HOOK();
153153
return status;
@@ -256,11 +256,9 @@ RGL_API rgl_status_t rgl_entity_create(rgl_entity_t* out_entity, rgl_scene_t sce
256256
RGL_API_LOG("rgl_entity_create(out_entity={}, scene={}, mesh={})", (void*) out_entity, (void*) scene, (void*) mesh);
257257
CHECK_ARG(out_entity != nullptr);
258258
CHECK_ARG(mesh != nullptr);
259+
CHECK_ARG(scene == nullptr); // TODO: remove once rgl_scene_t param is removed
259260
GraphRunCtx::synchronizeAll(); // Prevent races with graph threads
260-
if (scene == nullptr) {
261-
scene = Scene::defaultInstance().get();
262-
}
263-
*out_entity = Entity::create(Mesh::validatePtr(mesh), Scene::validatePtr(scene)).get();
261+
*out_entity = Entity::create(Mesh::validatePtr(mesh)).get();
264262
});
265263
TAPE_HOOK(out_entity, scene, mesh);
266264
return status;
@@ -269,9 +267,7 @@ RGL_API rgl_status_t rgl_entity_create(rgl_entity_t* out_entity, rgl_scene_t sce
269267
void TapePlayer::tape_entity_create(const YAML::Node& yamlNode)
270268
{
271269
rgl_entity_t entity = nullptr;
272-
rgl_entity_create(&entity,
273-
nullptr, // TODO(msz-rai) support multiple scenes
274-
tapeMeshes.at(yamlNode[2].as<TapeAPIObjectID>()));
270+
rgl_entity_create(&entity, nullptr, tapeMeshes.at(yamlNode[2].as<TapeAPIObjectID>()));
275271
tapeEntities.insert(std::make_pair(yamlNode[0].as<TapeAPIObjectID>(), entity));
276272
}
277273

@@ -282,7 +278,7 @@ RGL_API rgl_status_t rgl_entity_destroy(rgl_entity_t entity)
282278
CHECK_ARG(entity != nullptr);
283279
GraphRunCtx::synchronizeAll(); // Prevent races with graph threads
284280
auto entitySafe = Entity::validatePtr(entity);
285-
entitySafe->getScene()->removeEntity(entitySafe);
281+
Scene::instance().removeEntity(entitySafe);
286282
Entity::release(entity);
287283
});
288284
TAPE_HOOK(entity);
@@ -402,21 +398,16 @@ RGL_API rgl_status_t rgl_scene_set_time(rgl_scene_t scene, uint64_t nanoseconds)
402398
{
403399
auto status = rglSafeCall([&]() {
404400
RGL_API_LOG("rgl_scene_set_time(scene={}, nanoseconds={})", (void*) scene, nanoseconds);
401+
CHECK_ARG(scene == nullptr); // TODO: remove once rgl_scene_t param is removed
405402
GraphRunCtx::synchronizeAll(); // Prevent races with graph threads
406-
if (scene == nullptr) {
407-
scene = Scene::defaultInstance().get();
408-
}
409-
Scene::validatePtr(scene)->setTime(Time::nanoseconds(nanoseconds));
403+
404+
Scene::instance().setTime(Time::nanoseconds(nanoseconds));
410405
});
411406
TAPE_HOOK(scene, nanoseconds);
412407
return status;
413408
}
414409

415-
void TapePlayer::tape_scene_set_time(const YAML::Node& yamlNode)
416-
{
417-
rgl_scene_set_time(nullptr, // TODO(msz-rai) support multiple scenes
418-
yamlNode[1].as<uint64_t>());
419-
}
410+
void TapePlayer::tape_scene_set_time(const YAML::Node& yamlNode) { rgl_scene_set_time(nullptr, yamlNode[1].as<uint64_t>()); }
420411

421412
RGL_API rgl_status_t rgl_graph_run(rgl_node_t raw_node)
422413
{
@@ -810,12 +801,9 @@ RGL_API rgl_status_t rgl_node_raytrace(rgl_node_t* node, rgl_scene_t scene)
810801
auto status = rglSafeCall([&]() {
811802
RGL_API_LOG("rgl_node_raytrace(node={}, scene={})", repr(node), (void*) scene);
812803
CHECK_ARG(node != nullptr);
804+
CHECK_ARG(scene == nullptr); // TODO: remove once rgl_scene_t param is removed
813805

814-
if (scene == nullptr) {
815-
scene = Scene::defaultInstance().get();
816-
}
817-
818-
createOrUpdateNode<RaytraceNode>(node, Scene::validatePtr(scene));
806+
createOrUpdateNode<RaytraceNode>(node);
819807
// Clear velocity that could be set by rgl_node_raytrace_with_distortion
820808
Node::validatePtr<RaytraceNode>(*node)->setVelocity(nullptr, nullptr);
821809
});
@@ -827,7 +815,7 @@ void TapePlayer::tape_node_raytrace(const YAML::Node& yamlNode)
827815
{
828816
auto nodeId = yamlNode[0].as<TapeAPIObjectID>();
829817
rgl_node_t node = tapeNodes.contains(nodeId) ? tapeNodes.at(nodeId) : nullptr;
830-
rgl_node_raytrace(&node, nullptr); // TODO(msz-rai) support multiple scenes
818+
rgl_node_raytrace(&node, nullptr);
831819
tapeNodes.insert({nodeId, node});
832820
}
833821

@@ -840,12 +828,9 @@ RGL_API rgl_status_t rgl_node_raytrace_with_distortion(rgl_node_t* node, rgl_sce
840828
CHECK_ARG(node != nullptr);
841829
CHECK_ARG(linear_velocity != nullptr);
842830
CHECK_ARG(angular_velocity != nullptr);
831+
CHECK_ARG(scene == nullptr);
843832

844-
if (scene == nullptr) {
845-
scene = Scene::defaultInstance().get();
846-
}
847-
848-
createOrUpdateNode<RaytraceNode>(node, Scene::validatePtr(scene));
833+
createOrUpdateNode<RaytraceNode>(node);
849834
Node::validatePtr<RaytraceNode>(*node)->setVelocity(reinterpret_cast<const Vec3f*>(linear_velocity),
850835
reinterpret_cast<const Vec3f*>(angular_velocity));
851836
});
@@ -857,9 +842,7 @@ void TapePlayer::tape_node_raytrace_with_distortion(const YAML::Node& yamlNode)
857842
{
858843
auto nodeId = yamlNode[0].as<TapeAPIObjectID>();
859844
rgl_node_t node = tapeNodes.contains(nodeId) ? tapeNodes.at(nodeId) : nullptr;
860-
rgl_node_raytrace_with_distortion(&node,
861-
nullptr, // TODO(msz-rai) support multiple scenes
862-
reinterpret_cast<const rgl_vec3f*>(fileMmap + yamlNode[2].as<size_t>()),
845+
rgl_node_raytrace_with_distortion(&node, nullptr, reinterpret_cast<const rgl_vec3f*>(fileMmap + yamlNode[2].as<size_t>()),
863846
reinterpret_cast<const rgl_vec3f*>(fileMmap + yamlNode[3].as<size_t>()));
864847
tapeNodes.insert({nodeId, node});
865848
}

‎src/graph/NodesCore.hpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ struct CompactPointsNode : IPointsNodeSingleInput
9797
struct RaytraceNode : IPointsNode
9898
{
9999
using Ptr = std::shared_ptr<RaytraceNode>;
100-
void setParameters(std::shared_ptr<Scene> scene);
100+
void setParameters();
101101

102102
// Node
103103
void validateImpl() override;
@@ -122,7 +122,6 @@ struct RaytraceNode : IPointsNode
122122

123123
private:
124124
IRaysNode::Ptr raysNode;
125-
std::shared_ptr<Scene> scene;
126125

127126
DeviceAsyncArray<Vec2f>::Ptr defaultRange = DeviceAsyncArray<Vec2f>::create(arrayMgr);
128127
bool doApplyDistortion{false};

‎src/graph/RaytraceNode.cpp

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,8 @@
2020
#include <macros/optix.hpp>
2121
#include <RGLFields.hpp>
2222

23-
void RaytraceNode::setParameters(std::shared_ptr<Scene> scene)
23+
void RaytraceNode::setParameters()
2424
{
25-
this->scene = scene;
2625
const static Vec2f defaultRangeValue = Vec2f(0.0f, FLT_MAX);
2726
defaultRange->copyFromExternal(&defaultRangeValue, 1);
2827
}
@@ -39,7 +38,7 @@ void RaytraceNode::validateImpl()
3938
throw InvalidPipeline(msg);
4039
}
4140

42-
if (fieldData.contains(TIME_STAMP_F64) && !scene->getTime().has_value()) {
41+
if (fieldData.contains(TIME_STAMP_F64) && !Scene::instance().getTime().has_value()) {
4342
auto msg = fmt::format("requested for field TIME_STAMP_F64, but RaytraceNode cannot get time from scene");
4443
throw InvalidPipeline(msg);
4544
}
@@ -68,8 +67,8 @@ void RaytraceNode::enqueueExecImpl()
6867

6968
// Even though we are in graph thread here, we can access Scene class (see comment there)
7069
const Mat3x4f* raysPtr = raysNode->getRays()->asSubclass<DeviceAsyncArray>()->getReadPtr();
71-
auto sceneAS = scene->getASLocked();
72-
auto sceneSBT = scene->getSBTLocked();
70+
auto sceneAS = Scene::instance().getASLocked();
71+
auto sceneSBT = Scene::instance().getSBTLocked();
7372
dim3 launchDims = {static_cast<unsigned int>(raysNode->getRayCount()), 1, 1};
7473

7574
// Optional
@@ -94,7 +93,7 @@ void RaytraceNode::enqueueExecImpl()
9493
.rayTimeOffsets = timeOffsets.has_value() ? (*timeOffsets)->asSubclass<DeviceAsyncArray>()->getReadPtr() : nullptr,
9594
.rayTimeOffsetsCount = timeOffsets.has_value() ? (*timeOffsets)->getCount() : 0,
9695
.scene = sceneAS,
97-
.sceneTime = scene->getTime().has_value() ? scene->getTime()->asSeconds() : 0,
96+
.sceneTime = Scene::instance().getTime().value_or(Time::zero()).asSeconds(),
9897
.xyz = getPtrTo<XYZ_VEC3_F32>(),
9998
.isHit = getPtrTo<IS_HIT_I32>(),
10099
.rayIdx = getPtrTo<RAY_IDX_U32>(),

‎src/scene/Entity.cpp

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,10 @@
1616

1717
API_OBJECT_INSTANCE(Entity);
1818

19-
std::shared_ptr<Entity> Entity::create(std::shared_ptr<Mesh> mesh, std::shared_ptr<Scene> scene)
19+
std::shared_ptr<Entity> Entity::create(std::shared_ptr<Mesh> mesh)
2020
{
2121
auto entity = APIObject<Entity>::create(mesh);
22-
scene->addEntity(entity);
22+
Scene::instance().addEntity(entity);
2323
return entity;
2424
}
2525

@@ -28,7 +28,7 @@ Entity::Entity(std::shared_ptr<Mesh> mesh) : mesh(std::move(mesh)) {}
2828
void Entity::setTransform(Mat3x4f newTransform)
2929
{
3030
transform = newTransform;
31-
scene->requestASRebuild();
31+
Scene::instance().requestASRebuild();
3232
}
3333

3434
void Entity::setId(int newId)
@@ -39,13 +39,13 @@ void Entity::setId(int newId)
3939
throw std::invalid_argument(msg);
4040
}
4141
id = newId;
42-
scene->requestASRebuild();
42+
Scene::instance().requestASRebuild();
4343
}
4444

4545
void Entity::setIntensityTexture(std::shared_ptr<Texture> texture)
4646
{
4747
intensityTexture = texture;
48-
scene->requestSBTRebuild();
48+
Scene::instance().requestSBTRebuild();
4949
}
5050

5151
Mat3x4f Entity::getVelocity() const { throw std::runtime_error("unimplemented"); }

‎src/scene/Entity.hpp

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ struct Entity : APIObject<Entity>
3939
* Factory methods which creates an Entity and adds it to the given Scene.
4040
* See constructor docs for more details.
4141
*/
42-
static std::shared_ptr<Entity> create(std::shared_ptr<Mesh> mesh, std::shared_ptr<Scene> scene);
42+
static std::shared_ptr<Entity> create(std::shared_ptr<Mesh> mesh);
4343

4444
/**
4545
* Sets ID that will be used as a point attribute ENTITY_ID_I32 when a ray hits this entity.
@@ -61,11 +61,6 @@ struct Entity : APIObject<Entity>
6161
*/
6262
Mat3x4f getVelocity() const;
6363

64-
/**
65-
* @return The Scene in which this Entity is present.
66-
*/
67-
std::shared_ptr<Scene> getScene() const { return scene; }
68-
6964
private:
7065
/**
7166
* Creates Entity with given mesh and identity transform.
@@ -82,5 +77,4 @@ struct Entity : APIObject<Entity>
8277

8378
std::shared_ptr<Mesh> mesh{};
8479
std::shared_ptr<Texture> intensityTexture{};
85-
std::shared_ptr<Scene> scene{};
8680
};

‎src/scene/Mesh.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
// limitations under the License.
1414

1515
#include <scene/Mesh.hpp>
16+
#include <scene/Scene.hpp>
1617

1718
#include <filesystem>
1819

@@ -35,6 +36,7 @@ void Mesh::updateVertices(const Vec3f* vertices, std::size_t vertexCount)
3536
}
3637
dVertices->copyFromExternal(vertices, vertexCount);
3738
gasNeedsUpdate = true;
39+
Scene::instance().requestASRebuild();
3840
}
3941

4042
OptixTraversableHandle Mesh::getGAS(CudaStream::Ptr stream)
@@ -142,4 +144,5 @@ void Mesh::setTexCoords(const Vec2f* texCoords, std::size_t texCoordCount)
142144

143145
dTextureCoords.value()->copyFromExternal(texCoords, texCoordCount);
144146
gasNeedsUpdate = true;
147+
Scene::instance().requestSBTRebuild();
145148
}

‎src/scene/Scene.cpp

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,9 @@
1717
#include <scene/Texture.hpp>
1818
#include <memory/Array.hpp>
1919

20-
API_OBJECT_INSTANCE(Scene);
21-
22-
std::shared_ptr<Scene> Scene::defaultInstance()
20+
Scene& Scene::instance()
2321
{
24-
static auto scene = Scene::create();
22+
static Scene scene;
2523
return scene;
2624
}
2725

@@ -32,24 +30,20 @@ std::size_t Scene::getObjectCount() { return entities.size(); }
3230
void Scene::clear()
3331
{
3432
entities.clear();
35-
requestFullRebuild();
33+
requestASRebuild();
34+
requestSBTRebuild();
3635
}
3736

3837
void Scene::addEntity(std::shared_ptr<Entity> entity)
3938
{
40-
entity->scene = shared_from_this();
4139
entities.insert(entity);
42-
requestFullRebuild();
40+
requestASRebuild();
41+
requestSBTRebuild();
4342
}
4443

4544
void Scene::removeEntity(std::shared_ptr<Entity> entity)
4645
{
4746
entities.erase(entity);
48-
requestFullRebuild();
49-
}
50-
51-
void Scene::requestFullRebuild()
52-
{
5347
requestASRebuild();
5448
requestSBTRebuild();
5549
}

‎src/scene/Scene.hpp

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -43,11 +43,14 @@ struct Entity;
4343
* The only case when graph thread accesses scene is getAS() and getSBT(), which are locked.
4444
*
4545
*/
46-
struct Scene : APIObject<Scene>, std::enable_shared_from_this<Scene>
46+
struct Scene
4747
{
48-
static std::shared_ptr<Scene> defaultInstance();
48+
static Scene& instance();
4949

50-
Scene();
50+
Scene(const Scene&) = delete;
51+
Scene(Scene&&) = delete;
52+
Scene& operator=(const Scene&) = delete;
53+
Scene& operator=(Scene&&) = delete;
5154

5255
void addEntity(std::shared_ptr<Entity> entity);
5356
void removeEntity(std::shared_ptr<Entity> entity);
@@ -63,11 +66,12 @@ struct Scene : APIObject<Scene>, std::enable_shared_from_this<Scene>
6366
OptixTraversableHandle getASLocked();
6467
OptixShaderBindingTable getSBTLocked();
6568

66-
void requestFullRebuild();
6769
void requestASRebuild();
6870
void requestSBTRebuild();
6971

7072
private:
73+
Scene();
74+
7175
OptixShaderBindingTable buildSBT();
7276
OptixTraversableHandle buildAS();
7377

‎test/src/apiSurfaceTests.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ TEST_F(APISurfaceTests, rgl_entity_create_destroy)
7878
// Invalid args, note: scene can be nullptr here.
7979
EXPECT_RGL_INVALID_ARGUMENT(rgl_entity_create(nullptr, nullptr, nullptr), "entity != nullptr");
8080
EXPECT_RGL_INVALID_ARGUMENT(rgl_entity_create(&entity, nullptr, nullptr), "mesh != nullptr");
81-
EXPECT_RGL_INVALID_OBJECT(rgl_entity_create(&entity, (rgl_scene_t) 0x1234, mesh), "Scene 0x1234");
81+
EXPECT_RGL_INVALID_ARGUMENT(rgl_entity_create(&entity, (rgl_scene_t) 0x1234, mesh), "scene == nullptr");
8282
EXPECT_RGL_INVALID_OBJECT(rgl_entity_create(&entity, nullptr, (rgl_mesh_t) 0x1234), "Mesh 0x1234");
8383

8484
// Correct create

0 commit comments

Comments
 (0)