Line pointer refactor for performance #6863
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… instead of every getpoint()
This comment has been minimized.
This comment has been minimized.
Troy-Ferrell
requested review from
CDiaz-MS,
julenka and
thalbern
as code owners
|
/azp run |
|
Azure Pipelines successfully started running 2 pipeline(s). |
|
mrtk_docs fix on way |
I've added a ticket for tracking the 2.3 breaking changes: #6922 |
thalbern
approved these changes
Dec 17, 2019
keveleigh
approved these changes
Jan 2, 2020
Assets/MixedRealityToolkit.SDK/Features/UX/Scripts/Pointers/ShellHandRayPointer.cs
Outdated
Show resolved
Hide resolved
…ellHandRayPointer.cs Co-Authored-By: Kurtis <kurtie@microsoft.com>
…hub.com/Troy-Ferrell/MixedRealityToolkit-Unity into users/trferrel/line-pointer-refactor-v2 # Please enter a commit message to explain why this merge is necessary, # especially if it merges an updated upstream into a topic branch. # # Lines starting with '#' will be ignored, and an empty message aborts # the commit.
|
/azp run |
|
Azure Pipelines successfully started running 2 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 2 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 2 pipeline(s). |
|
@Troy-Ferrell, this looks to be blocked by a merge conflict and a test failure (related?) |
…ctor-v2 # Conflicts: # Assets/MixedRealityToolkit.Tests/PlayModeTests/PointerTests.cs
|
/azp run |
|
Azure Pipelines successfully started running 2 pipeline(s). |
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Overview
Change notes
This change refactors the default hand ray pointer prefabs and classes for more optimal performance. Key impacts
LinePointerclass has been simplified to only support raycast along a 2-point line (one raystep). Support for multi-raystep has been pushed down to a new inherited class,CurvePointer. Teleport pointers now inherit from this class. Simplifying LinePointer reduces some overhead in calculating points along the curve and number of raysteps involved.The
ShellHandRayPointerhas been re-worked to alternate the line renderers material instead of doing weird things using twoMixedRealityLineRenderercomponents. The dual render components were weirdly setup and doing duplicated work to the unity LineRender. If the old line renderer properties are used, a warning is logged during start alerting user to switch to new propertyDefaultControllerPointer prefab has removed the dual MRLineRenderers in favor of one. Also LineStepCount on the MRLineRenderer left is lowered from 16 to 10.
Fixed line renderer issues (did not respond to FadeLineOnEnable toggle, gradient value setting and material tiling)
Breaking changes
ShellHandRayPointerhas been removed of it's MRLineRenderer properties (lineRendererSelectedandlineRendererNoTarget)IMixedRealityPointernow requires aReset()method in it's interfaceUpdated #6922 for migration in properties change on ShellHandRayPointer
Added test under PointerTest.cs to confirm line pointer raycasts straight and curve pointer works with teleport pointer collisions
BEFORE
AFTER
Changes
Verification
Tested
HandsInteractionExampleScene in editor
HandsInteractionExampleScene VR on Device
Teleport system VR on Device
ExamplesHub app on HL2 device (IN PROGRESS)