Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Input simulation indicators interfere with graphics raycasts #6106

Closed
julenka opened this issue Sep 26, 2019 · 1 comment
Closed

Input simulation indicators interfere with graphics raycasts #6106

julenka opened this issue Sep 26, 2019 · 1 comment
Labels

Comments

@julenka
Copy link
Contributor

julenka commented Sep 26, 2019

Overview

Input simulation indicators show the state of hands which is nice, but placing them in the center of the screen introduces visual clutter. However, when you move the indicators to the sides, they interfere with graphics raycasts.

This bug is preventing us from turning on the input simulation indicators. Once it is fixed we can show the blue viewport indicators.

image

See PR #6062

Steps to reproduce

Update the input simulation indicators to be 50 px from the bottom left corner for left indicator, 50 px from bottom right for right indicator. Set indicators on in the default profile.

Run FocusProviderRaycastTests.RaycastProxies test

Expected Behavior

The test passes

Actual Behavior

The test fails. You will see also if you try to raycast against Unity UI, the raycast will fail.

@julenka julenka added the Bug label Sep 26, 2019
julenka pushed a commit that referenced this issue Sep 26, 2019
## Overview
UPDATE: This PR originally moved input simulation indicators to the sides and updates the reset button to look like a 'refresh' button. However, after moving indicators to the side, raycasts related to Unity UI began failing. Since the indicators as they are are too visually distracting from interaction, check them in as off by default. Filed #6106 to track turning them back on. The indicators as they are checked in will be on the sides, with correct refresh buttons.

![image](https://user-images.githubusercontent.com/168492/65546816-198f6280-decd-11e9-8b7f-39b123f19dc4.png)


## Changes
- Fixes: #6023
mrtk-bld added a commit that referenced this issue Sep 27, 2019
… input simulation indicators to the sides and updates the reset button to look like a 'refresh' button. However, after moving indicators to the side, raycasts related to Unity UI began failing. Since the indicators as they are are too visually distracting from interaction, check them in as off by default. Filed #6106 to track turning them back on. The indicators as they are checked in will be on the sides, with correct refresh buttons.  ![image](https://user-images.githubusercontent.com/168492/65546816-198f6280-decd-11e9-8b7f-39b123f19dc4.png)   ## Changes - Fixes: #6023  )
@julenka julenka closed this as completed Sep 27, 2019
tarukosu added a commit to HoloLabInc/MixedRealityToolkit-Unity that referenced this issue Oct 22, 2019
* Added doc comments

* Updated touchable collider comment and typos

* Reassigned toggle buttons with proper interactable profiles.

* Fix broken image links

* Fixed broken filename

* Change request - remove comment

* Recreated HandMenuExamples scene, deleted original corrupt scene file.

* Reassigned buttons.

* Button position adjustment

* Updated Scene System Profile for the Examples Hub

* add desc panel to input tool, rename tool

* re-update tool name to be more descriptive

* minor description panel updates

* add tool document and images, update input data provider doc to ref tool

* add controller mapping tool to toc

* Update Documentation/Tools/ControllerMappingTool.md

Co-Authored-By: Julia Schwarz <jschwarz@microsoft.com>

* Updated OnDisable and SetDisable in Interactable

* progress

* cleanup

* Fix other unit test

* Fix broken unit test

* Revert "Revert "Fix a few issues with PressableButton.""

This reverts commit a60ae5d.

* Revert previous changes, they didn't work. Instead add second collider, passed 20/20 tests

* remove extra fixedupdates

* Doccomments

* commit delete code that i missed in merge

* Remove empty returns

* One more empty return

* Merge pull request microsoft#6040 from andreiborodin/uses/anborod/nugetDocumentation

Uses/anborod/nuget documentation

* use new hand behaviors, make scale handles bigger

* remove bulk tests

* remove magic number

* Add second cube to fix issue where sometimes ray doesn't hit colliders (seems like Unity issue). Tests pass 100/100

* Fix ble TestInputActionMenuInput test by adding function that ensures input system updates. Also make test easier to diagnose by also verifying button was pressed, and released.

* Fix black piano key not responding to hand tracking input. adjusted Box Collider

* Adjust hand positions a bit in a test.  Also add better error text to know which one failed

* fix tmp installation

* Fix intermittent voice command failure by waiting for input system

* Remove extra wait

* Move the hand down/right by 5cm to aim the hand ray closer to the center of the button.

* add log message if tmpro package cannot be found

* Updated if spaces, removed SetDisabled from OnEnable and OnDisable

* More reliable one hand move far

* fix comments

* Update to CoreServices

* Comments

* Start deprecation and conslidation of state properties/setters

* Update states and move related items into deprecated, clean up code, and add appropriate guards to get/set properties

* Fix from merge, cleaning up code a bit

* Minor tweaks

* Update selectionmode code and disable interactable GUI on play

* polish steps

* Start working on tests

* Update tests

* Add deprecated item back

* Documentation fix for the Examples Hub

* Missing private field

* Merge pull request microsoft#6097 from Railboy/tooltip_inspector_fixes

Stops ToolTip inspectors from continuously refreshing

* Update Documentation/README_BoundingBox.md

* Account for script in .anim file

* Improve logging

* Turn off input simulation indicators (microsoft#6062)

## Overview
UPDATE: This PR originally moved input simulation indicators to the sides and updates the reset button to look like a 'refresh' button. However, after moving indicators to the side, raycasts related to Unity UI began failing. Since the indicators as they are are too visually distracting from interaction, check them in as off by default. Filed microsoft#6106 to track turning them back on. The indicators as they are checked in will be on the sides, with correct refresh buttons.

![image](https://user-images.githubusercontent.com/168492/65546816-198f6280-decd-11e9-8b7f-39b123f19dc4.png)


## Changes
- Fixes: microsoft#6023

* Docs update for the Button

* Remove IconEditor

* Delete MRTKExamplesHub.meta

* Image update

* Updated default simulation mouse rotation speed for improved usability (microsoft#6029)

* Updated default simulation mouse rotation speed for improved usability

* Move indicators to the sides

* Updated reset icon.

* Updating all existing input simulation profiles

* Revert indicators to old positions, disable indicators in default profile

* Update docs

* Typo

* Merged Julia's updates microsoft#6062, updated mouse hand rotation value to 30

* Properly remap regardless of m_script or script

* Updates for PR

* Update buffer size debug output

* removed patch for scaling issue while animating because it's breaking scaling of bounding box children (microsoft#6119)

fix for microsoft#6088

I removed a fix I did to compensate for a messed up scaling caused by animating the localscale of the bounding box target object. ( microsoft#6017 )
This fix broke scaling behavior for bounding box children.
Seems like scale changes to children are not propagated anymore once I set the localScale manually. This didn't show in the objects I tested with because the scaled objects were the target object itself. The broken cheese however has the scaled mesh as a child (same goes for slate, coffee, etc).

I tried to add a different workaround for the messed up scaling (eg https://answers.unity.com/questions/770240/changing-the-scale-of-the-parent-messes-up-the-chi.html) but that didn't work with the test file I'm using from one of the partners.
While testing with my own animation files this behavior won't show.

I don't think the original issue microsoft#5006 is something we can fix in MRTK code (the exploding part, not the updating part -> that's already fixed).

nit: added missing parentheses to if

* Camera fix and test

* Minor tweaks

* Added CanRouteInput() to PhysicalPressEventRouter and added a test for the change

* Update Documentation/README_ExampleHub.md

Co-Authored-By: Troy Ferrell <trferrel@microsoft.com>

* Update Documentation/README_ExampleHub.md

Co-Authored-By: Troy Ferrell <trferrel@microsoft.com>

* Update Documentation/README_Button.md

Co-Authored-By: Bernadette Thalhammer <36998103+thalbern@users.noreply.github.com>

* Update Documentation/README_Button.md

Co-Authored-By: Bernadette Thalhammer <36998103+thalbern@users.noreply.github.com>

* Update Assets/MixedRealityToolkit.Tests/PlayModeTests/PressableButtonTests.cs

Fixed typo

Co-Authored-By: Julia Schwarz <julenka.schwarz@gmail.com>

* Added test to InteractableTests that checks focus state when enabling/disabling Interactable

* Fix for PR

* fix ?. usages for obvious UnityEngine.Component derived objects

* Animation Controller / Radial View timing adjustment to remove initial delay

* Mark ColliderNearInteractionTouchable as obsolete (microsoft#6139)

* Mark ColliderNearInteractionTouchable as obsolete

* Fix docfx errors

* Updates for PR

* Remove #IF UNITY_EDITOR from MSBuild scripts

which are in an Editor-only assembly anyway

* Clean up some UNITY_EDITOR usage

* Update for merge

* fix null reference exception when re-enabling input system

* add disable/enable input systm test

* Fix docs building

* Fix typos

* Init of docs update

* More updates

* Updated test based on feedback

* Updates for docs

* More updates

* Fix

* Fix header

* Minor interface fix

* Docfx fixes

* Fix serialized property name in the inspector that was broken due to property renaming

* More docfx fixes

* pr fix

* Update menu/start button mapping for Oculus Touch

* Change TriggerPress -> Select for WMR controller button-based select

* Minor optimization

* Update Select for Vive and GenericOpenVR

* Rename MixedRealityControllerMappingProfiles -> MixedRealityControllerMappings

* Use built in profile method

* Improve profile migration

* Formatting

* Revert change to make Dimensions field lower-case to fix microsoft#6169

* Update toggle state of the near menu pin buttons

* Revert handinteractionexamples change

* Revert near menu, will do it in prefabs

* Modify 4x1 prefab pin to be toggled.

* Formatting

* Update test icons

* Fixed recursive property setting. Yay tests!

* Update WMROpenVRController to initially account for the angle offset

* Add controller mapping tests

* Deleted extra MixedRealityPlaypace objects from example scenes

* Disable camera controls when an xr device is present

* first pass release notes for 2.1.0, modify add updating guide

* Add comment for change

* add new features and known issues content

* add interactable deprecation list to updating article

* Updating from RC2 -> Updating from earlier versions

* fix typing errors and bad links

* Docs - Getting Started page - Fix broken links, updated and polished contents.

* Formatting and spelling

* More test icons

* Add menu item to add icons to tests automatically

* Spelling and move menu item to be consistent

* Update UnitTests.md

* Fix tests

* Add assert

* Wrap #if UNITY_EDITOR

* Fix toggle buttons and other multi-dimension interactable themes  (microsoft#6195)

Prior to microsoft#6104 Interactable would create all themes for all dimensions at startup, and then just select the appropriate list for the current dimension.

microsoft#6104  accidentally re-wrote that to recreate the list of current themes every time the dimension changed. When a theme is created and inits, it looks at the current offset/position/rotation etc as the starting position from which to base offset. In case of microsoft#6178 and microsoft#6187 the buttons had not reset to their original position before the new theme is created, causing new themes to be based not on the original 'resting' transform, but on some transform in middle of animation. This caused things like toggle buttons to move forward steadily as toggle was triggered.

Big thank you to @ritijain and @gilbdev for finding these issues!


## Changes
- Fixes: microsoft#6178
- Fix this by reverting to old behavior -- creating list of all themes per dimension whenever interactable is created, or when profiles change. 
- Add test to catch this regression in future, thank you @CDiaz-MS for helping write the test.

## Verification
Manually verified toggles behave correctly
Verified that test did not pass before changes, and that the test passes after changes.

> This optional section is a place where you can detail the specific type of verification 
> you want from reviewers. For example, if you want reviewers to checkout the PR locally
> and validate the functionality of specific scenarios, provide instructions
> on the specific scenarios and what you want verified.
>
> If there are specific areas of concern or question feel free to highlight them here so
> that reviewers can watch out for those issues.
>
> As a reviewer, it is possible to check out this change locally by using the following
> commands (substituting {PR_ID} with the ID of this pull request):
>
> git fetch origin pull/{PR_ID}/head:name_of_local_branch
>
> git checkout name_of_local_branch

* Auto generate signing configs in release pipeline

The signing configs will go out of sync any time add, remove or rename assemblies. We previously created a script to generate these configs, as writing them manually was a pain. It makes sense that we use this script in the pipelines as it's one less thing to think about.

* Add missing dir

* fix link

* Formatting and spelling

* Formatting and spelling

* All-caps !NOTE and !IMPORTANT

* Add notes on controller mapping profile

* move example hub to the experimental folder, update release notes

* More new stuff

* Spacing

* Add mesh outlining

* We don't want to publish Tests package, but we should include it in the signing before deleting it, otherwise an error

* Remove alpha tag

* temprarilly disabling mesh vertex count tracking

* restore some of the preciously commented code

* try calling recalc when the mesh changes

* restore 0 check

* refine mesh change check

* readd the vertex count and add a null check

* Remove duplicate metas from being packaged

* Update Examples Hub README to provide information on Extension package

* Update README_ExampleHub.md

* Update Documentation/README_ExampleHub.md

Co-Authored-By: Kurtis <kurtie@microsoft.com>

* Update README_ExampleHub.md

* add nuget package information to release notes

* improve nuget update instructions

* Update Documentation/ReleaseNotes.md

Co-Authored-By: Kurtis <kurtie@microsoft.com>

* Update Documentation/ReleaseNotes.md

Co-Authored-By: CDiaz-MS <53493796+CDiaz-MS@users.noreply.github.com>

* Updating nuspecs to remove source from 2.1 release due to N4U long path issue. This source will return as part of 2.2 release

* update roadmap

* Update .gitignore

Co-Authored-By: Kurtis <kurtie@microsoft.com>
@Alexees
Copy link
Contributor

Alexees commented May 28, 2020

@julenka seems this is fixed but the documentation is not updated still stating this bug as pending :
https://microsoft.github.io/MixedRealityToolkit-Unity/Documentation/InputSimulation/InputSimulationService.html

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants