Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor Celestial Anchors #298

Merged
merged 75 commits into from
Oct 4, 2022
Merged

Conversation

Schneegans
Copy link
Member

@Schneegans Schneegans commented Aug 22, 2022

This major refactoring reduces the CPU load of scenes containing many celestial bodies significantly. The primary change is a shift from inheritance to composition when it comes to the celestial anchors. For the default scene, the time to update the SolarSystem has been reduced by more than 50% (on my machine from 11 ms to 5 ms).

Primary Changes

  • The Settings class now has a map of immutable CelestialObjects which are instantiated when CosmoScout is started. Before it used to have a list of anchor configurations which could be used to initialize CelestialObjects.
  • CelestialObjects now have four new properties:
    • bodyCullingRadius: This can be used to determine the visiblity of an object. If the object is too far away to be seen, plugins may want to skip updating or drawing attached things.
    • orbitCullingRadius:This can be used to determine the visiblity of an object's trajectory. If the object is really far away, plugins may want to skip updating or drawing attached things (like trajectories or anchor labels).
    • trackable: If set to false, the observer will not automatically follow this object.
    • collidable: If set to false, the observer can travel through the object.
  • The observer-relative transformation of each CelestialObject is updated once each frame by the SolarSystem.
  • All classes which previously derived from CelestialAnchor, CelestialObject, or CelestialBody now get the observer-relative transformation from the CelestialObjects in the Settings.
  • Classes which previously derived from CelestialBody now have to derive from the new class CelestialSurface. A CelestialSurface can then be assigned to a CelestialObject.
  • CelestialAnchorNodes do not exist anymore. Classes which used to use these, now have to use a VistaTransformNode instead and update its transformation once each frame with the observer-relative transformation from the respective CelestialObject.
  • Much per-frame logic used to be executed in overrides of CelestialObject::update(). This method does not exist any more; plugins now need to update their objects manually.
  • The Windows CI jobs do not use clcache anymore. See Use buildcache instead of clcache #299.

Other Changes

  • The mouse navigation has been made a bit smoother. This makes it possible to update the scene scale only once each frame. Before, it had to be updated multiple times each frame which introduced a significant CPU overhead during rapid movements.
  • All plugins now save their configuration before being unloaded. This makes it possible that plugins keep their current state when being reloaded at run-time.
  • Test source files are now only compiled into the executable and into the libraries if COSMOSCOUT_UNIT_TESTS is set to true.

csp-anchor-labels

  • Anchor Labels are now shown for each configured object per default. A blacklist has been added to the plugin configuration to remove labels selectively.
  • Anchor Labels now show the name of the configured object rather than the SPICE center name.
  • The maximum size of Anchor Labels has been increased to prevent line breaking for longer object names.
  • Anchor Labels now use the new orbitCullingRadius of the object for deciding whether they should be shown or not.

csp-measurement-tools

  • Some common code has been moved from the measurement tools to the base classes cs::core::tools::Tool and cs::core::tools::Mark. This simplifies some code in the measurement tools.

csp-satellites

  • It's now possible to reload this plugin at run-time.

csp-wms-overlays

  • A regression was fixed which led to low precision close to a bodies surface.

Future Work

  • The method SolarSystem::getObject() is called pretty frequently. Performance-wise it could be better to store weak_ptrs to the CelestialObjects instead of calling this method every frame. For now, most objects only store the name of the CelestialObject they are attached to and get the corresponding CelestialObject from the settings each frame.
  • If a plugin references an object which is actually not available (e.g. due to a typo in the scene configuration), CosmoScout will crash (it prints a error message though).
  • Adding and removing CelestialObjects at run-time could work now (like adding a completely new planet at run-time), but has not been tested. Plugins may need some tweaking to properly respond to this.

Schneegans added 30 commits July 4, 2022 14:39
@Schneegans
Copy link
Member Author

A tip for testing this: The DLR-internal configuration repository also contains a feature/refactor-celestial-anchors branch with updated configuration files.

@Schneegans Schneegans marked this pull request as ready for review August 31, 2022 10:05
@Schneegans Schneegans changed the title Refactor celestial anchors Refactor Celestial Anchors Sep 2, 2022
@JonasGilg
Copy link
Member

The csp-satellites plugin throws the following OpenGL errors:

[I] csp-satellites .... Loading plugin...
[W] cs-graphics ....... From vista-gltf: OpenGL error in "createGPUsampler": 1280 (1280) C:/C++/cs-gh/src/cs-graphics/internal/gltfmodel.cpp:916
[W] cs-graphics ....... From vista-gltf: OpenGL error in "createGPUsampler": 1280 (1280) C:/C++/cs-gh/src/cs-graphics/internal/gltfmodel.cpp:916
[W] cs-graphics ....... From vista-gltf: OpenGL error in "createGPUsampler": 1280 (1280) C:/C++/cs-gh/src/cs-graphics/internal/gltfmodel.cpp:916
[I] csp-satellites .... Loading done.

@Schneegans
Copy link
Member Author

The csp-satellites plugin throws the following OpenGL errors:

[I] csp-satellites .... Loading plugin...
[W] cs-graphics ....... From vista-gltf: OpenGL error in "createGPUsampler": 1280 (1280) C:/C++/cs-gh/src/cs-graphics/internal/gltfmodel.cpp:916
[W] cs-graphics ....... From vista-gltf: OpenGL error in "createGPUsampler": 1280 (1280) C:/C++/cs-gh/src/cs-graphics/internal/gltfmodel.cpp:916
[W] cs-graphics ....... From vista-gltf: OpenGL error in "createGPUsampler": 1280 (1280) C:/C++/cs-gh/src/cs-graphics/internal/gltfmodel.cpp:916
[I] csp-satellites .... Loading done.

This is true, but it should be a separate issue, as it also happens on the current develop branch.

Copy link
Member

@JonasGilg JonasGilg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Amazing work! The performance improvements are incredible. I couldn't find any major problems in the changes, I just have some minor suggestions for improvements.

plugins/csp-anchor-labels/src/AnchorLabel.cpp Outdated Show resolved Hide resolved
plugins/csp-anchor-labels/src/Plugin.cpp Outdated Show resolved Hide resolved
plugins/csp-anchor-labels/src/Plugin.hpp Outdated Show resolved Hide resolved
plugins/csp-atmospheres/src/Shaders.cpp Outdated Show resolved Hide resolved
plugins/csp-lod-bodies/src/PlanetShader.hpp Outdated Show resolved Hide resolved
src/cs-core/tools/Mark.cpp Outdated Show resolved Hide resolved
src/cs-core/tools/MultiPointTool.cpp Outdated Show resolved Hide resolved
src/cs-core/tools/MultiPointTool.cpp Outdated Show resolved Hide resolved
src/cs-core/tools/MultiPointTool.cpp Outdated Show resolved Hide resolved
src/cs-core/tools/MultiPointTool.hpp Outdated Show resolved Hide resolved
Copy link
Member

@JonasGilg JonasGilg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just some final adjustments.

src/cs-scene/CelestialObject.hpp Outdated Show resolved Hide resolved
src/cs-utils/ObservableMap.hpp Outdated Show resolved Hide resolved
src/cs-utils/ObservableMap.hpp Outdated Show resolved Hide resolved
@Schneegans Schneegans merged commit 8327fd3 into develop Oct 4, 2022
@Schneegans Schneegans deleted the feature/refactor-celestial-anchors branch October 4, 2022 07:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactoring This is mainly about code restructuring
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants