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

Time-track movements in the 3D viewport #4876

Merged
merged 8 commits into from
Oct 20, 2020
Merged

Conversation

daniel-wer
Copy link
Member

@daniel-wer daniel-wer commented Oct 15, 2020

  • Added a new updateTdCamera update action which is triggered if the user modified the 3D view, so that these actions can be time tracked. The action is empty otherwise.
  • While I was at it I spent some love on the version view, added a lot of missing descriptions for certain update actions (tree/group visibility, user bounding boxes, fallback layer removal, node updates, edge deletion/addition) and extended the tree deletion message as by now multiple trees can be deleted at once.
  • I added save queue compaction for updateNode update actions as otherwise lots of new versions are created if the radius of a node is changed or a node is moved. updateTdCamera actions are also compacted and only the last one within one save interval is kept.
  • I've also noticed that the Undo of a tree deletion is misinterpreted as an NML upload by the version view (as multiple nodes are created at once which is not possible otherwise). Looking at the update actions one cannot be distinguished from the other, so I'm not sure how to fix this.

TODO:

  • Rotation in the 3D view is not picked up as a change yet, because the action that is used for that is also used when hovering over the 3D view and when initializing the tracing (those should not be counted as actions). I'll probably dispatch another action for the 3D view rotation so that it can be recognized and told apart from the other actions. (Solved by introducing a duplicate action that doesn't trigger the save queue diffing which is used accordingly)

URL of deployed dev instance (used for testing):

  • https://___.webknossos.xyz

Steps to test:

  • Move/Zoom/Rotate the 3D view - the save button should indicate that there are changes which need to be saved.
  • Create a task and exclusively do stuff in the 3D view - time should be tracked.
  • Check the version view to see whether the action is correctly labeled and also that subsequent movements in the 3D view within one save interval are compacted to only one version.
  • Do some of the other mentioned actions with new descriptions/compactions and check the version view afterwards.

Issues:


…dd many missing actions, improve save queue compaction
@daniel-wer daniel-wer self-assigned this Oct 15, 2020
@daniel-wer daniel-wer changed the title [WIP] Time-track movements in the 3D viewport Time-track movements in the 3D viewport Oct 19, 2020
Copy link
Member

@philippotto philippotto left a comment

Choose a reason for hiding this comment

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

Great stuff & clean up 👍 Worked well for me!

One thing I noticed during testing: Merely opening the version-restore-view seems to clear the undo stack. After opening and closing the version-restore-view, I cannot undo an action. But that's been like this probably before this PR? I cannot really see why this is happening. Maybe you know anything about this?

@@ -96,6 +157,7 @@ function getDescriptionForBatch(actions: Array<ServerUpdateAction>): Description

// If more than one createNode update actions are part of one batch, that is not a tree merge or split
// an NML was uploaded.
// TODO: This could've also been an undo/redo action.
Copy link
Member

Choose a reason for hiding this comment

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

Hmm, tricky. A simple solution would be to generalize the description message (e.g., "X nodes were added.") or to just leave this as is. Figuring out what the correct reason is would obviously be better, but I can't think of a good solution right now :/

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, I think we cannot do much about it easily apart from generalizing the message as you suggested. I'll create an issue for that.

Copy link
Member Author

Choose a reason for hiding this comment

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

For reference #4883

@daniel-wer
Copy link
Member Author

daniel-wer commented Oct 20, 2020

One thing I noticed during testing: Merely opening the version-restore-view seems to clear the undo stack. After opening and closing the version-restore-view, I cannot undo an action. But that's been like this probably before this PR? I cannot really see why this is happening. Maybe you know anything about this?

I'm afraid this is by design (so far). When a version is previewed (or the version view is closed) the tracing is restarted (api.tracing.restart) which also restarts all sagas to get a clean state. One could probably skip this if the newest version is already loaded but I'm not entirely sure about that and would do it in another PR as it's been like that, before :)

@daniel-wer daniel-wer merged commit e9982a0 into master Oct 20, 2020
@daniel-wer daniel-wer deleted the timetrack-td-view branch October 20, 2020 13:25
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.

Time-track movements in the 3D viewport
3 participants