Skip to content

Conversation

@FezVrasta
Copy link
Contributor

I'm trying to understand why these changes aren't working, as soon I set a pointerSize larger than 0 picking stops working 🤔

@jourdain
Copy link
Collaborator

jourdain commented Aug 3, 2021

Make sure the xy are ints.

@FezVrasta FezVrasta force-pushed the feat/view-picker-tolerance branch from 6411b88 to 1b4759c Compare August 3, 2021 16:04
@FezVrasta
Copy link
Contributor Author

No luck :-(

@FezVrasta FezVrasta force-pushed the feat/view-picker-tolerance branch from 1b4759c to af7e15d Compare August 3, 2021 16:08
@jourdain
Copy link
Collaborator

jourdain commented Aug 3, 2021

What are the symptoms of the issue?

@FezVrasta
Copy link
Contributor Author

No selection happens at all.

@jourdain
Copy link
Collaborator

jourdain commented Aug 3, 2021

Be aware that by changing x1/x2 to be different you will always get a frustrum rather than a ray.

You may need to fix/extend the pick method to still perform the ray. Maybe by adding a bool arg for isFrustrum.

@FezVrasta
Copy link
Contributor Author

Sorry I think I don't have enough knowledge on the matter to be helpful here :-(

@jourdain
Copy link
Collaborator

jourdain commented Aug 3, 2021

 pick(x1, y1, x2, y2, useFrustrum=false) {
      [...]
      if (useFrustrum) { // if (x1 !== x2 || y1 !== y2) {
      [...]
      const ray = [
        Array.from(
          this.openglRenderWindow.displayToWorld(Math.round((x1+x2) / 2), Math.round((y1+y2) / 2), 0, this.renderer)
        ),
        Array.from(
          this.openglRenderWindow.displayToWorld(Math.round((x1+x2) / 2), Math.round((y1+y2) / 2), 1, this.renderer)
        ),
      ];
      [...]
select => useFrustrum=true
click => useFrustrum=false
hover => useFrustrum=false

@FezVrasta FezVrasta force-pushed the feat/view-picker-tolerance branch from af7e15d to 3e2fe54 Compare August 3, 2021 16:40
@jourdain
Copy link
Collaborator

jourdain commented Aug 3, 2021

Same code as before except the condition to detect if it is a click/hover=ray vs select=frustum is not based on the fact that x1 != x2. Otherwise you are changing the behavior of the selection.

@FezVrasta
Copy link
Contributor Author

Thanks, it works now 👍

@jourdain
Copy link
Collaborator

jourdain commented Aug 3, 2021

You still need to fix the ray so it is at the center of the x1/x2/y1/y2 and not on one of the corner of the area.

@FezVrasta FezVrasta force-pushed the feat/view-picker-tolerance branch from 3e2fe54 to b30258a Compare August 3, 2021 16:45
@jourdain
Copy link
Collaborator

jourdain commented Aug 3, 2021

See my code snippet vs original

@FezVrasta FezVrasta force-pushed the feat/view-picker-tolerance branch from b30258a to bd07c37 Compare August 3, 2021 16:46
@FezVrasta FezVrasta force-pushed the feat/view-picker-tolerance branch from bd07c37 to d370e94 Compare August 3, 2021 16:47
@jourdain
Copy link
Collaborator

jourdain commented Aug 3, 2021

All working? Ready to merge?

@FezVrasta
Copy link
Contributor Author

FezVrasta commented Aug 3, 2021

CleanShot 2021-08-03 at 18 48 28

For some reason with the ray change it seems to be not centered. Only the lower area is sensitive.

@jourdain
Copy link
Collaborator

jourdain commented Aug 3, 2021

The ray/frustum part are only info metadata and don't affect selection. Maybe the returned array from hover is bigger than 1 and you always pick the first one? Check what the selection returned array looks like?

@FezVrasta
Copy link
Contributor Author

I think it's just an optical effect due to the pointer shape, I enlarged the pointer area and it actually looks good 😇

@jourdain jourdain merged commit 8aa7318 into Kitware:master Aug 3, 2021
@FezVrasta FezVrasta deleted the feat/view-picker-tolerance branch August 3, 2021 17:01
@jourdain
Copy link
Collaborator

jourdain commented Aug 3, 2021

🎉 This PR is included in version 1.6.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants