Remove allocs from idle and selecting ray interactor #11344
Conversation
com.microsoft.mrtk.spatialmanipulation/ObjectManipulator/NewObjectManipulator.cs
Outdated
Show resolved
Hide resolved
| @@ -367,14 +367,14 @@ private void UpdateLineVisual() | |||
| { | |||
| lineRenderer.colorGradient = ColorUtilities.GradientLerp(ValidColorGradient, SelectActiveColorGradient, rayInteractor.hasSelection ? 1 : 0); | |||
| } | |||
|
|
|||
| var compressionAmount = Mathf.Clamp(rayInteractor.maxRaycastDistance * MaxGradientLength / hitDistance, 0.0f, 1.0f); | |||
| lineRenderer.colorGradient = ColorUtilities.GradientCompress(lineRenderer.colorGradient, 0.0f, compressionAmount); | |||
There was a problem hiding this comment.
instead of only (or in addition to) calling this less frequently, can we make this method not alloc at all?
There was a problem hiding this comment.
After much investigation, I learned a few things.
- calls to retrieve lineRender.colorGradient allocs a new instance of Gradient.
- Calls to retrieve gradient.colorKeys and gradient.alpha keys allocs new arrays.
With these considerations, we can't do much to the fundamental implementation of our utility functions to avoid these GC allocs. I've made other adjustments to the LineVisuals implementation to avoid calling these functions when not necessary, but fundamentally doing modifications of gradients will end up being costly due to the underlying native implemenation.
There was a problem hiding this comment.
If it's fundamentally unsolvable, we should be doing this in the shader.
There was a problem hiding this comment.
Though I agree, I think that the performance hit (a few allocs during inbetween time of selection) doesn't quite justify the engineering effort for a rework at this particular juncture. It's not regression from what we've had previously in MRTK2, so I don't think it should have much customer facing impact.
RogPodge
force-pushed
the
LineCompressNoAlloc
branch
from
985875b
to
be96102
Compare
…or has hit an object
RogPodge
force-pushed
the
LineCompressNoAlloc
branch
from
be96102
to
c064393
Compare
…tilities calls when not needed
RogPodge
changed the title
| @@ -267,6 +267,10 @@ protected void OnDisable() | |||
| #region LineVisual Updates | |||
|
|
|||
| private static readonly ProfilerMarker UpdateLinePerfMarker = new ProfilerMarker("[MRTK] MRTKLineVisual.UpdateLineVisual"); | |||
| private static readonly ProfilerMarker UpdateLinePerfMarker2 = new ProfilerMarker("[MRTK] MRTKLineVisual.UpdateLineVisual2"); | |||
There was a problem hiding this comment.
Leftover while debugging, will remove
| [Range(0.01f, 1)] | ||
| float maxGradientLength = 0.1f; | ||
| float maxGradientLength = 0.3f; |
There was a problem hiding this comment.
Could you explain a bit more about changing the default value?
There was a problem hiding this comment.
The previous value was a little too aggressive and would end up compressing the gradient in situations where it didn't really improve usability. Increasing the value makes it so we don't unnecessarily apply the compression effect (saving us on performance)
* Line visual now only applies compression effect when the ray interactor has hit an object * adjusted gradient compress algorithm to be more compact and efficient * added deadzone parameter to the input simulator so axis values would reach 0 or 1 * reorganized line visuals script, made adjustments to minimize color utilities calls when not needed * adjusted prefab values to be less aggressive with the compression effect * removing unused perf marker Co-authored-by: Finn Sinclair <finnnorth@gmail.com>
Overview
Fixes an issue where
ColorUtilities.GradientCompresswas called every frame. The GradientCompress effect only needs to be applied when the ray interactor is registering a hit.This investigation also revealed that the Gradient lerp function also allocs. It may be worthwhile to create in-place Gradient functions in the future to remove allocs which can occur while selecting or hovering over valid targets.
Changes