This PR aims to solve all issues with `Groundcover` view distance handling in a satisfying way while preserving other optimisations that benefit other features. The main idea here is not to rely on `ViewData` updates for distance culling calculations because we can not accurately determine distance against lazily updated views. Instead, we perform an accurate measurement in a cull callback we can run every frame in `Groundcover` itself. While we do have to add some code to handle this feature, it is quite loosely coupled code that could be useful elsewhere in the future. These changes should address a performance regression @akortunov experienced.
This PR aims to solve `uniform block LightBufferBinding has no binding` messages @glassmancody has reportedly encountered since PR #3110 due to an apparent bug in OSG. While we do have to add a workaround here that adds a bit of clunkiness, #3216 should allow us to clean up these interactions a bit in the future.
This PR removes unneeded `lowerCaseInPlace` calls in in a hot path of `objectpaging.cpp` that are no longer necessary after PR #3197. In addition, I have been informed that these changes should by coincidence address a compiler specific compilation error we currently experience.
* resets state updater to apply light settings (#3141)
resets state updater to apply light settings
With this PR we achieve the same effect with fewer lines of code.
* fixes LightSource logic errors
We currently update `LightSource::setActorFade` in `TransparencyUpdater`. There are several logic errors inherent in this approach:
1. We fail to update `LightSource::setActorFade` for off screen actors because their `TransparencyUpdater` cull callback is not invoked.
2. We fail to update `LightSource::setActorFade` in the instant that a `TransparencyUpdater` is removed.
3. We fail to update `setActorFade` when an `mExtraLightSource` is created after calling `Animation::setAlpha`.
With this PR we avoid such issues by updating `LightSource::setActorFade` in `Animation::setAlpha` and `Animation::addExtraLightSource` instead.
With this PR we refactor `SceneUtil::KeyframeController` not to require `virtual osg::Callback` inheritance. I suppose such `virtual` overhead is not justified here because it negatively impacts many other classes we derive from `osg::Callback`.
With this PR we refactor a `premultiplied alpha` user string set by `characterpreview.cpp` into a more flexible mechanism allowing us to assign any state to GUI textures. We can consider these changes more future proof than the previous approach.
This PR aims to spark the retirement of a questionable pattern I have found all over our code base. I will illustrate how this pattern encourages code duplication, lacks type safety, requires documentation and can be prone to bugs.
```
std::map<std::string, Object> mMap; // Stored in all lowercase for a case-insensitive lookup
std::string lowerKey = Misc::StringUtils::lowerCase(key);
mMap.emplace(lowerKey, object);
std::string lowerKey = Misc::StringUtils::lowerCase(key);
mMap.find(lowerKey);
mMap.find(key); // Not found. Oops!
```
An alternative approach produces no such issues.
```
std::unordered_map<std::string, Object, Misc::StringUtils::CiHash, Misc::StringUtils::CiEqual> mMap;
mMap.emplace(key, object);
mMap.find(key);
```
Of course, such an alternative will work for a `map` as well, but an `unordered_map` is generally preferable over a `map` with these changes because we have moved `lowerCase` into the comparison operator.
In this PR I have refactored `Animation::mNodeMap` accordingly. I have reviewed and adapted all direct and indirect usage of `Animation::mNodeMap` to ensure we do not change behaviour with this PR.
This PR fixes a crash caused by the improperly ensured lifetime of RigGeometry::mSourceGeometry. mSourceGeometry was not adequate to ensure mSourceGeometry would outlive mGeometry because we extend mGeometrys lifetime beyond this lifetime by passing mGeometry to the draw traversal instead of this.
In addition,
We add important comments.
We detect and prevent generally unsafe operations in high level code.
We add a sprinkling of const to help clarify intentions.
Currently, we use an `UnrefQueue` which supposedly aims to transfer destruction costs to another thread. The implications of this unusual pattern can not be well understood because some allocators might free resources more efficiently if they are freed by the same thread that allocated them. In addition, `UnrefQueue` complicates the validation of thread safety in our engine. Lastly, our current usage of `UnrefQueue` triggers `ref()`, `unref()` atomic operations as objects are passed into the queue. These operations could be more expensive than the actual destruction.
With this PR we thus remove `UnrefQueue`. We can expect these changes to have a minor impact at most because we free most resources elsewhere in `ResourceSystem::updateCache`.
Currently, we use a peculiar mapping of ESM classes by their std::type_info::name. This mapping is an undefined behaviour because std::type_info::name is strictly implementation defined. It could return a non-unique value on some platforms. With this PR we use the unsigned int sRecordId of the ESM class as a more efficient lookup type that does not build on undefined behaviour. We can expect marginally faster save-game loading with these changes as well.
Currently, we always traverse the scene graph an additional time with a ComputeLightSpaceBounds visitor during shadow casting. ComputeLightSpaceBounds is only useful when the shadow casting mask allows us to shrink the bounds of the rendered scene, so we guard its traversal with a check against getCastsShadowTraversalMask. In practice, this guard never works because we build the traversal mask inclusively.
With this PR we limit the getCastsShadowTraversalMask check to relevant masks. This new check allows us to skip a superfluous ComputeLightSpaceBounds traversal with most settings.
We now use PositionAttitudeTransform for unmerged nodes because I have been informed PositionAttitudeTransform's scene graph performance is measurably faster than MatrixTransform's. We still need to use MatrixTransform for merged nodes because the Optimizer has limited support for non-MatrixTransform nodes. These MatrixTransforms will be removed in the merging process anyway.
As we discovered in #3148, `Transform` nodes and their low level equivalence `pushModelViewMatrix` are somewhat costly involving a `Matrix::invert` operation per frame.
With this PR we avoid one `Transform` node for sun flashes and avoid another `pushModelViewMatrix` call in case the sun is fully visible.
With this PR we test out osg's shader define system for a somewhat harmless feature. As we can see, our code becomes more concise and efficient in this case. Most importantly, we no longer create unneeded vertex shader objects.
* avoids creating empty statesets on drawables
Currently, we attempt to skip creating state on drawable nodes when this state matches the default state. This attempt is incomplete because we still create an avoidable empty stateset in the default case.
* renderingmanager.cpp
* nifloader.cpp
* nifloader.cpp
* shadervisitor.cpp
* avoids creating empty statesets on drawables
Currently, we attempt to skip creating state on drawable nodes when this state matches the default state. This attempt is incomplete because we still create an avoidable empty stateset in the default case.
* renderingmanager.cpp
* nifloader.cpp
