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

[BUG FIX] Mouse drag crashing the viewer in edge cases #644

Merged

Conversation

eratc
Copy link
Contributor

@eratc eratc commented Jan 27, 2025

Description

This change initializes the _pdown instance variable of the Trackball object, which stores the x, y values of the initial mouse press for the camera drag in the viewer. It ensures _pdown is initialized to zeroes instead of None, preventing crashes in edge cases. Additionally, it removes the use of getattr to access _pdown, as it is always initialized.

Related Issue

No related existing issue.

Motivation and Context

Previously, viewer would crash if the on_mouse_drag callback was triggered before on_mouse_click. This happened because when the initial mouse press that should set _pdown attribute was missing, it had original value None from when the Trackball instance is created, resulting in a TypeError during camera drag calculations. This might happen in couple of edge cases, one way I am able to reliably reproduce it in my Ubuntu 24.04 system is listed below.

Steps to reproduce

  1. Launch a genesis script with the viewer.
  2. Don't click inside the window.
  3. Click and drag the top bar of the window and snap it to one of the screen edges to trigger edge snap resizing.
  4. Move the mouse back inside the window.

How Has This Been / Can This Be Tested?

Screenshots (if appropriate):

Screencast.from.2025-01-27.21-26-23.webm

Checklist:

  • I read the CONTRIBUTING document.
  • I followed the Submitting Code Changes section of CONTRIBUTING document.
  • I tagged the title correctly (including BUG FIX/FEATURE/MISC/BREAKING)
  • I updated the documentation accordingly or no change is needed.
  • I tested my changes and added instructions on how to test it for reviewers.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

eratc added 2 commits January 27, 2025 18:02
Previously, viewer would crash if the `on_mouse_drag` callback was
triggered before `on_mouse_click`. This happened because the initial
mouse press would set `_pdown` attribute, which was initialized to
`None` when the `Trackball` instance is created, resulting in a
`TypeError` during camera drag calculations.

This fix ensures the `_pdown` value is properly initialized to
prevent the crash in some edge cases like window snap resizing.
It also removes the attribute access to `_pdown` with getattr
since it is always initialized.
@YilingQiao YilingQiao merged commit f0417dc into Genesis-Embodied-AI:main Jan 30, 2025
1 check passed
@YilingQiao
Copy link
Collaborator

thanks!

@eratc eratc deleted the fix/viewer-mouse-drag-crash branch January 30, 2025 14:52
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