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

Landmarks and Constraints interaction bugs #358

Closed
5 tasks done
annehaley opened this issue Feb 9, 2024 · 12 comments · Fixed by #355
Closed
5 tasks done

Landmarks and Constraints interaction bugs #358

annehaley opened this issue Feb 9, 2024 · 12 comments · Fixed by #355

Comments

@annehaley
Copy link
Collaborator

annehaley commented Feb 9, 2024

During a usability test of the landmarks and constraints, the following interaction bugs were identified:

  • The new simplified syncCameras function, intended to solve the plane overlap bug, causes a worse clipping problem. When using multiple renderers with shapes with different offsets (like the ellipsoid multiple domain), there is noticeable clipping of shapes. We should go back to applying camera deltas. This will mean that we will continue to have the plane overlap bug, but that would be the lesser evil between the two problems.
  • Attempting to save landmarks after deleting a landmark raises a server-side error: NoneType object is not iterable
  • Getting widget is undefined error when attempting to add a landmark or constraint if no subjects are currently shown

The following other improvements could be made. These are not bugs, just usability considerations:

  • After clicking the "Show subject" button, there is no option to hide the subject again. The button should change, not disappear.
  • Add the ability to name constraints (these names would not carry over to Studio)

@JakeWags please add any other bugs or possible usability improvements you find

@JakeWags
Copy link
Collaborator

JakeWags commented Feb 23, 2024

I'm no longer able to get the widget is undefined behavior when no subject is shown, but there is a minor bug when trying to add a landmark with no subject shown.
Browser: Mozilla Firefox Version 123.0
Note: this only seems to occur on the first time attempting after refreshing.
landmarks_error_no_subjects

It seems like the landmark is "wiped" after I try to begin placement on the first subject.

@JakeWags
Copy link
Collaborator

Also:

While attempting to add landmarks after deleting a previous one, I am unable to place the landmarks.

Browser: Mozilla Firefox version 123.0
See gif below:
landmarks-error-on-delete

@JakeWags
Copy link
Collaborator

Constraints:

After placing a constraint, and then deleting the object, the plane placement state is saved. I am unable to reset the plane on the subject. The plane does not show visually, and it looks like the constraint location is default halfway through the object.

Browser: Mozilla Firefox version 123.0

See gif below:
constraints-error-1

@JakeWags
Copy link
Collaborator

Constraints:

After adding a plane or seemingly loading and manipulating any plane, showing a second subject and attempting to manipulate the plane on that subject does not work. It seems like the second subject is unresponsive to any changes of the constraint.

Browser: Mozilla Firefox version 123.0

See gif below:
constraints-error-2

@annehaley
Copy link
Collaborator Author

Thanks, @JakeWags! I've addressed each of these items in these commits: aea4c4b, 23c79e5, 1f50bad. When you have the chance, please pull these changes, try these behaviors again, and report on any more bugs you find.

@JakeWags
Copy link
Collaborator

JakeWags commented Mar 8, 2024

This error occurs when the user clicks "New landmark" before the data is loaded. This seems to create a sort of race condition. The loading will indefinitely stall.

EDIT: This also happens with constraints, with the same kind of actions.

Stack trace:

Uncaught (in promise) TypeError: landmarksObject.anatomy_type is undefined
    lInfos methods.ts:496
    getLandmarks methods.ts:496
    refreshRender Main.vue:165
    Lodash 8
    VueJS 12
    fetchData DataList.vue:57
    setup DataList.vue:104
    VueJS 19
    updateItem VItemGroup.ts:176
methods.ts:496
    refreshRender Main.vue:334
    AsyncFunctionThrow self-hosted:856
    Lodash 8
    VueJS 12
    fetchData DataList.vue:57
    InterpretGeneratorResume self-hosted:1465
    AsyncFunctionNext self-hosted:852
    (Async: async)
    setup DataList.vue:104
    VueJS 19
    updateItem VItemGroup.ts:176

Using same browser and version as above.

constraints-error-3-8--1

@JakeWags
Copy link
Collaborator

JakeWags commented Mar 8, 2024

This is fairly minor: changing landmark color does not have an effect until the shape viewer is interacted with.

constraints-error-3-8--2

@JakeWags
Copy link
Collaborator

JakeWags commented Mar 8, 2024

constraints-error-3-8--3

While trying to add a new landmark, after clicking "begin placement" I could no longer interact. This may be due to fast user actions causing race conditions? This one seems to be inconsistent.

@JakeWags
Copy link
Collaborator

JakeWags commented Mar 8, 2024

It seems like when deleting the first landmark, it deletes correctly, but then the 2nd takes its place rather than remaining in the same position (spacially).

constraints-error-3-8--4

@JakeWags
Copy link
Collaborator

JakeWags commented Mar 8, 2024

Similar to the landmarks/constraints race condition; this error occurs when you try to load multiple layers quickly. I notice this is also happening when just trying to load particles. This could be unrelated to this PR.

constraints-error-3-8--5

@annehaley
Copy link
Collaborator Author

@JakeWags I was able to find fixes for all except this one:

While trying to add a new landmark, after clicking "begin placement" I could no longer interact. This may be due to fast user actions causing race conditions? This one seems to be inconsistent.

Could you provide more instructions on how to replicate? When I copied the actions in your video, I wasn't able to get the error. Is it just a matter of doing this fast enough?

Also, for the other bug that seemed to be a race condition, I added loading flags to prevent any user action before the fetching of landmarks/constraints is complete. Could you try out the new changes and let me know if the loading flag also prevents the bug above?

@JakeWags
Copy link
Collaborator

All errors look resolved by adding those loading flags. The landmark deletion error looks good as well.

As far as the "race condition" error goes, I actually believe this is something to do with the dataset having particles loaded on upload, but they don't match our expected datatype. I was able to get this error outside of this PR. It only occurs when loading "particles" from this dataset. *Note: I haven't done any grooming or optimization steps

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 a pull request may close this issue.

2 participants