Use AABB to better differentiate instances#67
Use AABB to better differentiate instances#67xoxor4d wants to merge 1 commit intoNVIDIAGameWorks:mainfrom
Conversation
|
|
||
| Vector3 newWorldPosition = Vector3(newTransform[3][0], newTransform[3][1], newTransform[3][2]); | ||
| if (RtxOptions::Get()->instanceUseBoundingBox()) { | ||
| newWorldPosition = (newTransform * Vector4(drawCall.getGeometryData().boundingBox.getCentroid(), 1.0f)).xyz(); |
There was a problem hiding this comment.
You follow the pattern of "get position, check RtxOption, replace position with transform * centroid" several times.
You could move all of this logic into the AABB class, and just depend on the whether the AABB is valid for the draw call instead of checking the RtxOption over and over:
Vector3 getTransformedCentroid(const Matrix4& transform) {
if (isValid()) {
return (transform * Vector4(getCentroid(), 1.f)).xyz();
} else {
return transform[3].xyz();
}
}
That way we just default to using the more accurate measurement when it's available. If calculating the AABB ever shows up as a perf bottleneck in the future, we could easily swap to only calculating AABB's that are in a specific texture list or something, instead of doing it for every draw call.
There was a problem hiding this comment.
Fair point! I've included a toggle so that the feature can be tested / validated more easily.
Should I squash again or keep it a separate commit?
|
The build failures look like they may be related to a change that went in last week. Could you make sure you're up to date with the latest code? You may need to change how you construct the vectors in some spots. |
|
Hey Mark! My branch is up to date and the build is failing with the same issues that made the build fail after anon-apple pushed said changes: 7e24a52 |
|
Oh, sorry, hadn't realized the build was already failing. I'll check if that's being looked into yet. |
- add `getCentroid` to `AxisAlignedBoundingBox` - add `getTransformedCentroid` to `AxisAlignedBoundingBox` - use transformed bounding-box-centroid (when AABB is valid) instead of just the world transform of the instance in `InstanceManager::findSimilarInstance` - ^ in `RtInstance::onTransformChanged` - ^ in `RtInstance::teleport` - ^ in `DrawCallCache::get` - add `AABB's For Instance Differentiation` setting to the Heuristics tab - add setting check to `RtxOptions::needsMeshBoundingBox`
838f52e to
fd16e7c
Compare
|
Okay I've now added |
|
Using the setting in needsMeshBoundingBox is fine for now - by centralizing the use of your new setting in one place, it makes it easier to change the behavior later if it winds up causing a perf bottleneck somewhere. This looks good to me, so I'll go ahead and get this merged in. |
|
I've just merged this internally (with a couple of edits - mostly changing the name of the option to The commit is here: 92f5637 Please remember to close this PR when you have a chance. |
|
Thanks Mark! |
The changes in this PR try to improve instance matching by using its AABB instead of just its world position.
This leads to more stable instances and thus in less visual artifacting -> motionblur smearing.
I think these changes can also help with switching LOD's.
The changes in action (on/off and tweaked
Unique Object Search Distance- Call of Duty 4 SP):https://drive.google.com/file/d/1MtYsdSmAw1HzKZhbvBP7VUlRBRjkEte2
I've started working on this because of the issue I've opened here: NVIDIAGameWorks/rtx-remix#450
Thanks to Alex for all the guidance (and previously Mark on discord)!
Changes
Vector3 getCentroid()toAxisAlignedBoundingBoxInstanceManager::findSimilarInstanceRtInstance::onTransformChangedRtInstance::teleportDrawCallCache::getAABB's For Instance Differentiationsetting to the Heuristics tabRtxOptions::needsMeshBoundingBoxto enable boundingbox calculationsThe initial branch with the individual commits leading up to this squished commit is still available:
https://github.com/xoxor4d/dxvk-remix/commits/feature/aabb_inst