Skip to content

PokePointer TouchableVolume fix touch up when in middle of volume#6026

Merged
thalbern merged 5 commits into
microsoft:mrtk_developmentfrom
julenka:issue/5873
Sep 23, 2019
Merged

PokePointer TouchableVolume fix touch up when in middle of volume#6026
thalbern merged 5 commits into
microsoft:mrtk_developmentfrom
julenka:issue/5873

Conversation

@julenka

@julenka julenka commented Sep 20, 2019

Copy link
Copy Markdown
Contributor

Overview

PokePointers would actually raise up events when in the middle of a touchable volume, for two reasons:

  1. The ray that pokepointer was building was too short. When inside a volume, the ray would never actually hit any bounds of the volume, and therefore FocusProvider would thing that the pointer was no longer colliding with something, causing touch up.
  2. The distance checks in OnPostSceneQuery were looking at the length of the poke to hit point, which can make sense for touchable planes (you maybe want to raise touch up after you've gone far enough behind a plane), but not for volumes (you only really want to exit when you exit the volume).

In both cases I fixed this by special-casing touchable volume in poke pointer, and making the ray longer as well as changing the distance check.

Changes

Verification

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

@julenka julenka requested review from a user, MenelvagorMilsom and thalbern September 20, 2019 21:08

@Troy-Ferrell Troy-Ferrell left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, some nit elaboration on comments

@julenka

julenka commented Sep 21, 2019

Copy link
Copy Markdown
Contributor Author

/azp run

@julenka julenka closed this Sep 21, 2019
@julenka julenka reopened this Sep 21, 2019
@julenka

julenka commented Sep 21, 2019

Copy link
Copy Markdown
Contributor Author

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 2 pipeline(s).

@julenka

julenka commented Sep 23, 2019

Copy link
Copy Markdown
Contributor Author

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 2 pipeline(s).

@thalbern thalbern left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

@thalbern thalbern merged commit 4b72ac4 into microsoft:mrtk_development Sep 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants