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

RayClicks integration #106

Merged
merged 11 commits into from
Oct 11, 2023
Merged

RayClicks integration #106

merged 11 commits into from
Oct 11, 2023

Conversation

chungmin99
Copy link
Collaborator

Similar to "ClickMessage"s supported in the previous Nerfstudio viewer, where a doubleclick sends a ray to the client.

The ray is specified in world coordinates, where:

  • the origin is the camera origin, and
  • the direction is based on the vector drawn from the origin to the clicked pixel on the viewport camera's image plane.

I named it "RayClicking" here, to disambiguate it with the "Clicks" on the SceneNodeHandles.

@chungmin99 chungmin99 requested a review from brentyi September 27, 2023 05:13
@brentyi
Copy link
Collaborator

brentyi commented Sep 27, 2023

Notes from chat:

  • Start with single click instead of double click.
  • Pass a single event object to callbacks. This could be like:
@dataclasses.dataclass
class ScenePointerEvent:
    client_id: int
    type: Literal["click"]  # Later we can add `double_click`, `move`, `down`, `up`, etc
    ray_origin: Tuple[float, float, float]
    ray_direction: Tuple[float, float, float]

We can also rename the existing viser.ClickEvent to viser.SceneNodePointerEvent?

  • Set up another message to turn on pointer events? So we don't send them unless a callback is actually attached.
  • Check if stopPropagation() in scene node click callbacks prevents scene clicks. (I'd prefer to send both events if a node is clicked but also if only the node one is clicked I think that's OK)
  • We can see if we can borrow naming etc from Javascript? Here's a test page for pointer events: https://devtools.terrymorse.com/pointerevents/index.html
  • Have a function for removing callbacks. I think a nice pattern would be:
add_sphere_button = server.add_gui_button("Add sphere")

@add_sphere_button.on_click
def _(_) -> None:
    @viser_server.on_scene_click # Or whatever this is called
    def add_sphere(event: viser.ScenePointerEvent) -> None:
        add_the_sphere(event)
        viser_server.remove_scene_click_cb(add_sphere)

Separate PRs:

  • Consider passing in intersected objects.
  • Add ray_origin / ray_direction to the scene node clicks.
  • Enable/disable camera controls.
  • Plots!

@chungmin99
Copy link
Collaborator Author

chungmin99 commented Sep 28, 2023

Moved everything to a separate file ScenePointerControls.tsx because it was originally inside CameraControls.tsx. Hopefully this is not only the right thing to do (ScenePointer is not camera controls!), but also will facilitate adding further ScenePointer actions in this file.

Also, with regards to the prior notes:

Start with single click instead of double click.
Pass a single event object to callbacks.

Both should be there now.

Check if stopPropagation() in scene node click callbacks prevents scene clicks. (I'd prefer to send both events if a node is clicked but also if only the node one is clicked I think that's OK)

I experimented a bit with the example test file, and it seems that both the scene node clicks and the scene pointer events are registered. :)

Set up another message to turn on pointer events? So we don't send them unless a callback is actually attached.

This should be implemented now with EnableScenePointerMessage. This state is stored inside the ViewerContext, and now can be set from the python side. I recall that you mentioned a strong preference to avoid hardcoding stuff though.

Have a function for removing callbacks.

... still thinking about it, I think it's worth trying the approach in the code snippit in the previous convo. Will try soon!

src/viser/_message_api.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@brentyi brentyi left a comment

Choose a reason for hiding this comment

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

let's gooo

@brentyi brentyi enabled auto-merge (squash) October 11, 2023 05:26
@brentyi brentyi merged commit b41eb17 into main Oct 11, 2023
12 checks passed
@brentyi brentyi deleted the cmk/rayclick branch October 11, 2023 05:31
yzslab pushed a commit to yzslab/viser that referenced this pull request Oct 20, 2024
* initial rayclick imp

* Example code bug

* Move scene pointer logic to diff file, add pointer on/off logic

* Updated names + example

* Remove pointer listener if removed all callbacks

* Formatting changes

* Tweaks

* Docs + backwards compatibility

* Update cursor for scene clicks

* Add events.md

---------

Co-authored-by: Brent Yi <[email protected]>
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.

2 participants