enh(viz): make viz::scene3d::vr great again
Originally: vr::RayTracingVolumeRenderer calls virtual function from constructor
Description
Resource management
-
SVolumeRenderer
is actually much more than a service. It also manages the same resources as the lib it is supposed to interface, but... Not all of them. Some are simply duplicated for no valid reason. - Some flags exist for the same purpose in
SVolumeRenderer
andRayTracingVolumeRenderer
... But they are never updated at the same time. This causes subtle bugs, such as: - RAM leaks due to some objects being allocated with
new
but nodelete
Safety
- Some functions in
RayTracingVolumeRenderer
do not check anything before allocating objects, and thus would reallocate without deleting the previous instances if the user is not careful enough.
Performance
- Some allocation functions are called when it is useless, so the reallocate everything once in a while for no reason.
Polymorphism
-
sight::viz::scene3D::RayTracingVolumeRenderer
has quite a lot ofvirtual
members. Among themcreateRayTracingMaterial
,setSampling
,computeRayTracingDefines
, andsetRayCastingPassTextureUnits
are called from the constructor. This discards the qualifier when instancing an object from subclasses. Currently, it works because we only use this class as a standalone, and experimental subclasses use the same setup. -
This is not related to polymorphism directly, but there is really no reason to not give access to the
string
s below to potential subclasses. I'm sure there are numerous times it would be relevant to know their values, and they should thus bestatic
members of the class, eitherprotected
orpublic
.
static const std::string s_AUTOSTEREO_DEFINE = "AUTOSTEREO=1";
static const std::string s_AO_DEFINE = "AMBIENT_OCCLUSION=1";
static const std::string s_COLOR_BLEEDING_DEFINE = "COLOR_BLEEDING=1";
static const std::string s_SHADOWS_DEFINE = "SHADOWS=1";
static const std::string s_PREINTEGRATION_DEFINE = "PREINTEGRATION=1";
static const std::string s_VOLUME_TF_TEXUNIT_NAME = "volumeTransferFunction";
Proposal
- Properly split the service and the libraries. The service should only call the library functions with the proper arguments and manage slots.
- Fix those random RAM leaks with smart pointers.
- Remove duplicate flags.
- Make
vr::RayTracingVolumeRenderer
polymorphic, and for real.
And also:
- Group some attributes in
struct
s. They need each other to work anyway.
Outcomes
- Proper service-library operation.
- Proper use of polymorphism.
- No warnings related to
virtual
methods called in constructor. - Possibility to create properly formed subclasses.