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

Extend JS API with createNode functionality and refactor #7998

Merged
merged 13 commits into from
Aug 19, 2024

Conversation

philippotto
Copy link
Member

@philippotto philippotto commented Aug 15, 2024

  • Adds api.tracing.createNode(position, options) to the front-end API.
  • Refactors skeleton handler related code
  • Refactors some maybe-accessors away (see Refactor data.maybe away #7999)

URL of deployed dev instance (used for testing):

  • https://___.webknossos.xyz

Steps to test:

  • create an annotation
  • run something like this in the console: webknossos.DEV.api.tracing.createNode([1596, 1596, 1538], {center: false, activate: true, branchpoint: true})

Issues:

  • no issue (was a user request)

(Please delete unneeded items, merge only when none are left open)

@philippotto philippotto self-assigned this Aug 15, 2024
@philippotto philippotto marked this pull request as ready for review August 15, 2024 11:07
@philippotto philippotto changed the title [WIP] Extend JS API with createNode functionality and refactor Extend JS API with createNode functionality and refactor Aug 15, 2024
Copy link
Contributor

@MichaelBuessemeyer MichaelBuessemeyer left a comment

Choose a reason for hiding this comment

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

Thanks for the new API and refactoring. The code looks good to me and I was unable to come up with an suggestions :)

Testing also went well without any problems 🎉

In short: Good Stuff

@philippotto
Copy link
Member Author

thank you! fyi, I added additionalCoordinates as optional parameters.

@MichaelBuessemeyer
Copy link
Contributor

Commit a646bd5 looks alright

But sadly during testing I found that centerin doesn't properly work in the current version of the dev branch (deployed friday evening).

Here is what I tested as input

// Creating nodes at specific coordinates with varying options

webknossos.DEV.api.tracing.createNode([1596, 1596, 1538], {center: false, activate: true, branchpoint: true, additionalAxes: []});

webknossos.DEV.api.tracing.createNode([1596, 1596, 1538], {center: true, activate: true, branchpoint: true});

// Ignoring a ResizeObserver loop warning
console.js:13 Ignoring ResizeObserver loop completed with undelivered notifications.

// More node creations at different coordinates with specified options

webknossos.DEV.api.tracing.createNode([3184, 3514, 1024], {center: false, activate: true, branchpoint: true});

webknossos.DEV.api.tracing.createNode([3184, 3114, 1024], {center: true, activate: true, branchpoint: true});

webknossos.DEV.api.tracing.createNode([3290, 3622, 1024], {center: true, activate: true, branchpoint: true});

webknossos.DEV.api.tracing.createNode([3295, 3622, 1024], {center: true, activate: true, branchpoint: false});

webknossos.DEV.api.tracing.createNode([3718, 3491, 1024], {center: true, activate: true, branchpoint: false});

webknossos.DEV.api.tracing.createNode([3718, 3591, 1024], {center: true, activate: true, branchpoint: false});

webknossos.DEV.api.tracing.createNode([3718, 3691, 1024], {center: false, activate: true, branchpoint: false});

webknossos.DEV.api.tracing.createNode([3718, 3720, 1024], {center: true, activate: true, branchpoint: false});

webknossos.DEV.api.tracing.createNode([3778, 3720, 1024], {center: true, activate: true, branchpoint: false});

webknossos.DEV.api.tracing.createNode([3778, 3620, 1024], {center: true, activate: true, branchpoint: false});

webknossos.DEV.api.tracing.createNode([3778, 3420, 1024], {center: true, activate: true, branchpoint: false});

webknossos.DEV.api.tracing.createNode([3878, 3420, 1024], {center: true, activate: true, branchpoint: false});

When changing the y coordinate, the centering only worked on the y axis. When changing the x axis no centering / flycam moment happened :/

Could you please have another look at this 🙏 ?

@philippotto philippotto merged commit 630532a into master Aug 19, 2024
2 checks passed
@philippotto philippotto deleted the skeleton-js-api branch August 19, 2024 09:06
dieknolle3333 pushed a commit that referenced this pull request Sep 2, 2024
* rename setWaypoint to handleCreateNode in arbitrary controller

* rename setWaypoint to handleNodeCreation

* refactor

* implement createNode in api_latest; refactor to allow better parameteriziation; fix that centering node did not work when new node was not activated

* refactor getActiveNode accessor to not use a Maybe

* refactor getActiveTree accessor to not use a Maybe

* update changelog

* remove unused import

* allow to pass additionalCoordinates when creating nodes via API

* set skipCenteringAnimationInThirdDimension to false when using api
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants