From 7bcb4c7adb67483b94fd5ad4a30f9da7ba8d933c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20B=C3=BC=C3=9Femeyer?= <39529669+MichaelBuessemeyer@users.noreply.github.com> Date: Tue, 1 Apr 2025 18:39:07 +0200 Subject: [PATCH 01/41] add more supported update actions for user bounding box updates --- .../annotation/UpdateActions.scala | 8 ++ .../tracings/NamedBoundingBox.scala | 2 +- .../updating/SkeletonUpdateActions.scala | 134 ++++++++++++++++++ .../tracings/volume/VolumeUpdateActions.scala | 134 ++++++++++++++++++ 4 files changed, 277 insertions(+), 1 deletion(-) diff --git a/webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/annotation/UpdateActions.scala b/webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/annotation/UpdateActions.scala index f720e240aec..fe0873f863b 100644 --- a/webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/annotation/UpdateActions.scala +++ b/webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/annotation/UpdateActions.scala @@ -54,8 +54,16 @@ object UpdateAction { case "updateTreeEdgesVisibility" => deserialize[UpdateTreeEdgesVisibilitySkeletonAction](jsonValue) case "updateUserBoundingBoxesInSkeletonTracing" => deserialize[UpdateUserBoundingBoxesSkeletonAction](jsonValue) + case "addUserBoundingBoxSkeletonAction" => deserialize[AddUserBoundingBoxSkeletonAction](jsonValue) + case "deleteUserBoundingBoxSkeletonAction" => deserialize[DeleteUserBoundingBoxSkeletonAction](jsonValue) + case "updateUserBoundingBoxBoundsSkeletonAction" => + deserialize[UpdateUserBoundingBoxBoundsSkeletonAction](jsonValue) case "updateUserBoundingBoxVisibilityInSkeletonTracing" => deserialize[UpdateUserBoundingBoxVisibilitySkeletonAction](jsonValue) + case "updateUserBoundingBoxNameSkeletonAction" => + deserialize[UpdateUserBoundingBoxNameSkeletonAction](jsonValue) + case "updateUserBoundingBoxColorSkeletonAction" => + deserialize[UpdateUserBoundingBoxColorSkeletonAction](jsonValue) // Volume case "updateBucket" => deserialize[UpdateBucketVolumeAction](jsonValue) diff --git a/webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/tracings/NamedBoundingBox.scala b/webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/tracings/NamedBoundingBox.scala index 526af171b67..cb8753a09fa 100644 --- a/webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/tracings/NamedBoundingBox.scala +++ b/webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/tracings/NamedBoundingBox.scala @@ -3,7 +3,7 @@ package com.scalableminds.webknossos.tracingstore.tracings import play.api.libs.json.{Json, OFormat} import com.scalableminds.util.geometry.BoundingBox import com.scalableminds.util.image.Color -import com.scalableminds.webknossos.datastore.geometry.{NamedBoundingBoxProto => ProtoBoundingBox} +import com.scalableminds.webknossos.datastore.geometry.{BoundingBoxProto, NamedBoundingBoxProto => ProtoBoundingBox} import com.scalableminds.webknossos.datastore.helpers.ProtoGeometryImplicits import com.scalableminds.webknossos.tracingstore.tracings.skeleton.updating.SkeletonUpdateActionHelper diff --git a/webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/tracings/skeleton/updating/SkeletonUpdateActions.scala b/webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/tracings/skeleton/updating/SkeletonUpdateActions.scala index 07757abd896..f3e6f62c0a4 100644 --- a/webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/tracings/skeleton/updating/SkeletonUpdateActions.scala +++ b/webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/tracings/skeleton/updating/SkeletonUpdateActions.scala @@ -539,6 +539,68 @@ case class UpdateUserBoundingBoxesSkeletonAction(boundingBoxes: List[NamedBoundi this.copy(actionTracingId = newTracingId) } +case class AddUserBoundingBoxSkeletonAction(boundingBox: NamedBoundingBox, + actionTracingId: String, + actionTimestamp: Option[Long] = None, + actionAuthorId: Option[String] = None, + info: Option[String] = None) + extends SkeletonUpdateAction { + override def applyOn(tracing: SkeletonTracing): SkeletonTracing = + tracing.withUserBoundingBoxes(tracing.userBoundingBoxes :+ boundingBox.toProto) + + override def addTimestamp(timestamp: Long): UpdateAction = + this.copy(actionTimestamp = Some(timestamp)) + override def addInfo(info: Option[String]): UpdateAction = this.copy(info = info) + override def addAuthorId(authorId: Option[String]): UpdateAction = + this.copy(actionAuthorId = authorId) + override def withActionTracingId(newTracingId: String): LayerUpdateAction = + this.copy(actionTracingId = newTracingId) +} + +case class DeleteUserBoundingBoxSkeletonAction(boundingBoxId: Int, + actionTracingId: String, + actionTimestamp: Option[Long] = None, + actionAuthorId: Option[String] = None, + info: Option[String] = None) + extends SkeletonUpdateAction { + override def applyOn(tracing: SkeletonTracing): SkeletonTracing = + tracing.withUserBoundingBoxes(tracing.userBoundingBoxes.filter(_.id != boundingBoxId)) + + override def addTimestamp(timestamp: Long): UpdateAction = + this.copy(actionTimestamp = Some(timestamp)) + override def addInfo(info: Option[String]): UpdateAction = this.copy(info = info) + override def addAuthorId(authorId: Option[String]): UpdateAction = + this.copy(actionAuthorId = authorId) + override def withActionTracingId(newTracingId: String): LayerUpdateAction = + this.copy(actionTracingId = newTracingId) +} + +case class UpdateUserBoundingBoxBoundsSkeletonAction(boundingBox: NamedBoundingBox, + actionTracingId: String, + actionTimestamp: Option[Long] = None, + actionAuthorId: Option[String] = None, + info: Option[String] = None) + extends SkeletonUpdateAction { + override def applyOn(tracing: SkeletonTracing): SkeletonTracing = { + def updateUserBoundingBoxes() = + tracing.userBoundingBoxes.map { currentBoundingBox => + if (boundingBox.id == currentBoundingBox.id) + currentBoundingBox.copy(boundingBox = boundingBox.toProto.boundingBox) + else + currentBoundingBox + } + tracing.withUserBoundingBoxes(updateUserBoundingBoxes()) + } + + override def addTimestamp(timestamp: Long): UpdateAction = + this.copy(actionTimestamp = Some(timestamp)) + override def addInfo(info: Option[String]): UpdateAction = this.copy(info = info) + override def addAuthorId(authorId: Option[String]): UpdateAction = + this.copy(actionAuthorId = authorId) + override def withActionTracingId(newTracingId: String): LayerUpdateAction = + this.copy(actionTracingId = newTracingId) +} + case class UpdateUserBoundingBoxVisibilitySkeletonAction(boundingBoxId: Option[Int], isVisible: Boolean, actionTracingId: String, @@ -569,6 +631,58 @@ case class UpdateUserBoundingBoxVisibilitySkeletonAction(boundingBoxId: Option[I override def isViewOnlyChange: Boolean = true } +case class UpdateUserBoundingBoxNameSkeletonAction(boundingBox: NamedBoundingBox, + actionTracingId: String, + actionTimestamp: Option[Long] = None, + actionAuthorId: Option[String] = None, + info: Option[String] = None) + extends SkeletonUpdateAction { + override def applyOn(tracing: SkeletonTracing): SkeletonTracing = { + def updateUserBoundingBoxes() = + tracing.userBoundingBoxes.map { currentBoundingBox => + if (boundingBox.id == currentBoundingBox.id) + currentBoundingBox.copy(name = boundingBox.name) + else + currentBoundingBox + } + tracing.withUserBoundingBoxes(updateUserBoundingBoxes()) + } + + override def addTimestamp(timestamp: Long): UpdateAction = + this.copy(actionTimestamp = Some(timestamp)) + override def addInfo(info: Option[String]): UpdateAction = this.copy(info = info) + override def addAuthorId(authorId: Option[String]): UpdateAction = + this.copy(actionAuthorId = authorId) + override def withActionTracingId(newTracingId: String): LayerUpdateAction = + this.copy(actionTracingId = newTracingId) +} + +case class UpdateUserBoundingBoxColorSkeletonAction(boundingBox: NamedBoundingBox, + actionTracingId: String, + actionTimestamp: Option[Long] = None, + actionAuthorId: Option[String] = None, + info: Option[String] = None) + extends SkeletonUpdateAction { + override def applyOn(tracing: SkeletonTracing): SkeletonTracing = { + def updateUserBoundingBoxes() = + tracing.userBoundingBoxes.map { currentBoundingBox => + if (boundingBox.id == currentBoundingBox.id) + currentBoundingBox.copy(color = boundingBox.toProto.color) + else + currentBoundingBox + } + tracing.withUserBoundingBoxes(updateUserBoundingBoxes()) + } + + override def addTimestamp(timestamp: Long): UpdateAction = + this.copy(actionTimestamp = Some(timestamp)) + override def addInfo(info: Option[String]): UpdateAction = this.copy(info = info) + override def addAuthorId(authorId: Option[String]): UpdateAction = + this.copy(actionAuthorId = authorId) + override def withActionTracingId(newTracingId: String): LayerUpdateAction = + this.copy(actionTracingId = newTracingId) +} + object CreateTreeSkeletonAction { implicit val jsonFormat: OFormat[CreateTreeSkeletonAction] = Json.format[CreateTreeSkeletonAction] } @@ -620,7 +734,27 @@ object UpdateUserBoundingBoxesSkeletonAction { implicit val jsonFormat: OFormat[UpdateUserBoundingBoxesSkeletonAction] = Json.format[UpdateUserBoundingBoxesSkeletonAction] } +object AddUserBoundingBoxSkeletonAction { + implicit val jsonFormat: OFormat[AddUserBoundingBoxSkeletonAction] = + Json.format[AddUserBoundingBoxSkeletonAction] +} +object DeleteUserBoundingBoxSkeletonAction { + implicit val jsonFormat: OFormat[DeleteUserBoundingBoxSkeletonAction] = + Json.format[DeleteUserBoundingBoxSkeletonAction] +} +object UpdateUserBoundingBoxBoundsSkeletonAction { + implicit val jsonFormat: OFormat[UpdateUserBoundingBoxBoundsSkeletonAction] = + Json.format[UpdateUserBoundingBoxBoundsSkeletonAction] +} object UpdateUserBoundingBoxVisibilitySkeletonAction { implicit val jsonFormat: OFormat[UpdateUserBoundingBoxVisibilitySkeletonAction] = Json.format[UpdateUserBoundingBoxVisibilitySkeletonAction] } +object UpdateUserBoundingBoxNameSkeletonAction { + implicit val jsonFormat: OFormat[UpdateUserBoundingBoxNameSkeletonAction] = + Json.format[UpdateUserBoundingBoxNameSkeletonAction] +} +object UpdateUserBoundingBoxColorSkeletonAction { + implicit val jsonFormat: OFormat[UpdateUserBoundingBoxColorSkeletonAction] = + Json.format[UpdateUserBoundingBoxColorSkeletonAction] +} diff --git a/webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/tracings/volume/VolumeUpdateActions.scala b/webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/tracings/volume/VolumeUpdateActions.scala index 9307df81d1c..68156d2d773 100644 --- a/webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/tracings/volume/VolumeUpdateActions.scala +++ b/webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/tracings/volume/VolumeUpdateActions.scala @@ -106,6 +106,68 @@ case class UpdateUserBoundingBoxesVolumeAction(boundingBoxes: List[NamedBounding tracing.withUserBoundingBoxes(boundingBoxes.map(_.toProto)) } +case class AddUserBoundingBoxSkeletonAction(boundingBox: NamedBoundingBox, + actionTracingId: String, + actionTimestamp: Option[Long] = None, + actionAuthorId: Option[String] = None, + info: Option[String] = None) + extends ApplyableVolumeUpdateAction { + override def applyOn(tracing: VolumeTracing): VolumeTracing = + tracing.withUserBoundingBoxes(tracing.userBoundingBoxes :+ boundingBox.toProto) + + override def addTimestamp(timestamp: Long): UpdateAction = + this.copy(actionTimestamp = Some(timestamp)) + override def addInfo(info: Option[String]): UpdateAction = this.copy(info = info) + override def addAuthorId(authorId: Option[String]): UpdateAction = + this.copy(actionAuthorId = authorId) + override def withActionTracingId(newTracingId: String): LayerUpdateAction = + this.copy(actionTracingId = newTracingId) +} + +case class DeleteUserBoundingBoxSkeletonAction(boundingBoxId: Int, + actionTracingId: String, + actionTimestamp: Option[Long] = None, + actionAuthorId: Option[String] = None, + info: Option[String] = None) + extends ApplyableVolumeUpdateAction { + override def applyOn(tracing: VolumeTracing): VolumeTracing = + tracing.withUserBoundingBoxes(tracing.userBoundingBoxes.filter(_.id != boundingBoxId)) + + override def addTimestamp(timestamp: Long): UpdateAction = + this.copy(actionTimestamp = Some(timestamp)) + override def addInfo(info: Option[String]): UpdateAction = this.copy(info = info) + override def addAuthorId(authorId: Option[String]): UpdateAction = + this.copy(actionAuthorId = authorId) + override def withActionTracingId(newTracingId: String): LayerUpdateAction = + this.copy(actionTracingId = newTracingId) +} + +case class UpdateUserBoundingBoxBoundsSkeletonAction(boundingBox: NamedBoundingBox, + actionTracingId: String, + actionTimestamp: Option[Long] = None, + actionAuthorId: Option[String] = None, + info: Option[String] = None) + extends ApplyableVolumeUpdateAction { + override def applyOn(tracing: VolumeTracing): VolumeTracing = { + def updateUserBoundingBoxes() = + tracing.userBoundingBoxes.map { currentBoundingBox => + if (boundingBox.id == currentBoundingBox.id) + currentBoundingBox.copy(boundingBox = boundingBox.toProto.boundingBox) + else + currentBoundingBox + } + tracing.withUserBoundingBoxes(updateUserBoundingBoxes()) + } + + override def addTimestamp(timestamp: Long): UpdateAction = + this.copy(actionTimestamp = Some(timestamp)) + override def addInfo(info: Option[String]): UpdateAction = this.copy(info = info) + override def addAuthorId(authorId: Option[String]): UpdateAction = + this.copy(actionAuthorId = authorId) + override def withActionTracingId(newTracingId: String): LayerUpdateAction = + this.copy(actionTracingId = newTracingId) +} + case class UpdateUserBoundingBoxVisibilityVolumeAction(boundingBoxId: Option[Int], isVisible: Boolean, actionTracingId: String, @@ -136,6 +198,58 @@ case class UpdateUserBoundingBoxVisibilityVolumeAction(boundingBoxId: Option[Int override def isViewOnlyChange: Boolean = true } +case class UpdateUserBoundingBoxNameSkeletonAction(boundingBox: NamedBoundingBox, + actionTracingId: String, + actionTimestamp: Option[Long] = None, + actionAuthorId: Option[String] = None, + info: Option[String] = None) + extends ApplyableVolumeUpdateAction { + override def applyOn(tracing: VolumeTracing): VolumeTracing = { + def updateUserBoundingBoxes() = + tracing.userBoundingBoxes.map { currentBoundingBox => + if (boundingBox.id == currentBoundingBox.id) + currentBoundingBox.copy(name = boundingBox.name) + else + currentBoundingBox + } + tracing.withUserBoundingBoxes(updateUserBoundingBoxes()) + } + + override def addTimestamp(timestamp: Long): UpdateAction = + this.copy(actionTimestamp = Some(timestamp)) + override def addInfo(info: Option[String]): UpdateAction = this.copy(info = info) + override def addAuthorId(authorId: Option[String]): UpdateAction = + this.copy(actionAuthorId = authorId) + override def withActionTracingId(newTracingId: String): LayerUpdateAction = + this.copy(actionTracingId = newTracingId) +} + +case class UpdateUserBoundingBoxColorSkeletonAction(boundingBox: NamedBoundingBox, + actionTracingId: String, + actionTimestamp: Option[Long] = None, + actionAuthorId: Option[String] = None, + info: Option[String] = None) + extends ApplyableVolumeUpdateAction { + override def applyOn(tracing: VolumeTracing): VolumeTracing = { + def updateUserBoundingBoxes() = + tracing.userBoundingBoxes.map { currentBoundingBox => + if (boundingBox.id == currentBoundingBox.id) + currentBoundingBox.copy(color = boundingBox.toProto.color) + else + currentBoundingBox + } + tracing.withUserBoundingBoxes(updateUserBoundingBoxes()) + } + + override def addTimestamp(timestamp: Long): UpdateAction = + this.copy(actionTimestamp = Some(timestamp)) + override def addInfo(info: Option[String]): UpdateAction = this.copy(info = info) + override def addAuthorId(authorId: Option[String]): UpdateAction = + this.copy(actionAuthorId = authorId) + override def withActionTracingId(newTracingId: String): LayerUpdateAction = + this.copy(actionTracingId = newTracingId) +} + case class RemoveFallbackLayerVolumeAction(actionTracingId: String, actionTimestamp: Option[Long] = None, actionAuthorId: Option[String] = None, @@ -389,10 +503,30 @@ object UpdateUserBoundingBoxesVolumeAction { implicit val jsonFormat: OFormat[UpdateUserBoundingBoxesVolumeAction] = Json.format[UpdateUserBoundingBoxesVolumeAction] } +object AddUserBoundingBoxSkeletonAction { + implicit val jsonFormat: OFormat[AddUserBoundingBoxSkeletonAction] = + Json.format[AddUserBoundingBoxSkeletonAction] +} +object DeleteUserBoundingBoxSkeletonAction { + implicit val jsonFormat: OFormat[DeleteUserBoundingBoxSkeletonAction] = + Json.format[DeleteUserBoundingBoxSkeletonAction] +} +object UpdateUserBoundingBoxBoundsSkeletonAction { + implicit val jsonFormat: OFormat[UpdateUserBoundingBoxBoundsSkeletonAction] = + Json.format[UpdateUserBoundingBoxBoundsSkeletonAction] +} object UpdateUserBoundingBoxVisibilityVolumeAction { implicit val jsonFormat: OFormat[UpdateUserBoundingBoxVisibilityVolumeAction] = Json.format[UpdateUserBoundingBoxVisibilityVolumeAction] } +object UpdateUserBoundingBoxNameSkeletonAction { + implicit val jsonFormat: OFormat[UpdateUserBoundingBoxNameSkeletonAction] = + Json.format[UpdateUserBoundingBoxNameSkeletonAction] +} +object UpdateUserBoundingBoxColorSkeletonAction { + implicit val jsonFormat: OFormat[UpdateUserBoundingBoxColorSkeletonAction] = + Json.format[UpdateUserBoundingBoxColorSkeletonAction] +} object RemoveFallbackLayerVolumeAction { implicit val jsonFormat: OFormat[RemoveFallbackLayerVolumeAction] = Json.format[RemoveFallbackLayerVolumeAction] } From 09de1567955fcbfbdd5d613097ab0ddb527b38c2 Mon Sep 17 00:00:00 2001 From: Charlie Meister Date: Thu, 3 Apr 2025 18:57:11 +0200 Subject: [PATCH 02/41] [frontend] add new updateActions to add, remove and update individual bounding boxes --- .../oxalis/model/sagas/update_actions.ts | 123 ++++++++++++++++++ 1 file changed, 123 insertions(+) diff --git a/frontend/javascripts/oxalis/model/sagas/update_actions.ts b/frontend/javascripts/oxalis/model/sagas/update_actions.ts index 7d6d351466d..d29101a39ee 100644 --- a/frontend/javascripts/oxalis/model/sagas/update_actions.ts +++ b/frontend/javascripts/oxalis/model/sagas/update_actions.ts @@ -377,6 +377,129 @@ export function updateVolumeTracing( }, } as const; } + +export function addUserBoundingBoxSkeletonAction( + boundingBox: UserBoundingBox, + actionTracingId: string, + timestamp: number | null, + authorId: string | null, + info: string | null, +) { + return { + name: "addUserBoundingBoxSkeleton", + value: { + boundingBox: convertUserBoundingBoxesFromFrontendToServer([boundingBox]), + actionTracingId, + actionTimestamp: timestamp, + actionAuthorId: authorId, + info, + }, + } as const; +} + +export function addUserBoundingBoxInVolumeTracingAction( + boundingBox: UserBoundingBox, + actionTracingId: string, + timestamp: number | null, + authorId: string | null, + info: string | null, +) { + return { + name: "addUserBoundingBoxSkeleton", + value: { + boundingBox: convertUserBoundingBoxesFromFrontendToServer([boundingBox]), + actionTracingId, + actionTimestamp: timestamp, + actionAuthorId: authorId, + info, + }, + } as const; +} + +export function deleteUserBoundingBoxInSkeletonTracingAction( + boundingBoxId: number, + actionTracingId: string, + timestamp: number | null, + authorId: string | null, + info: string | null, +) { + return { + name: "deleteUserBoundingBoxSkeletonAction", + value: { + boundingBoxId, + actionTracingId, + actionTimestamp: timestamp, + actionAuthorId: authorId, + info, + }, + } as const; +} + +export function deleteUserBoundingBoxInVolumeTracingAction( + boundingBoxId: number, + actionTracingId: string, + timestamp: number | null, + authorId: string | null, + info: string | null, +) { + return { + name: "deleteUserBoundingBoxVolumeAction", + value: { + boundingBoxId, + actionTracingId, + actionTimestamp: timestamp, + actionAuthorId: authorId, + info, + }, + } as const; +} + +export function updateUserBoundingBoxInSkeletonTracingAction( + boundingBoxId: number, + updatedProps: Partial, + actionTracingId: string, + timestamp: number | null, + authorId: string | null, + info: string | null, +) { + const updatedPropKeys = Object.keys(updatedProps); + return { + name: "updateUserBoundingBoxSkeletonAction", + value: { + boundingBoxId, + actionTracingId, + updatedProps, + updatedPropKeys, + actionTimestamp: timestamp, + actionAuthorId: authorId, + info, + }, + } as const; +} + +export function updateUserBoundingBoxInVolumeTracingAction( + boundingBoxId: number, + updatedProps: Partial, + actionTracingId: string, + timestamp: number | null, + authorId: string | null, + info: string | null, +) { + const updatedPropKeys = Object.keys(updatedProps); + return { + name: "updateUserBoundingBoxVolumeAction", + value: { + boundingBoxId, + actionTracingId, + updatedProps, + updatedPropKeys, + actionTimestamp: timestamp, + actionAuthorId: authorId, + info, + }, + } as const; +} + export function updateUserBoundingBoxesInSkeletonTracing( userBoundingBoxes: Array, actionTracingId: string, From 7a187fd1e1dd24d7dd62a6052dc8a41d3048e195 Mon Sep 17 00:00:00 2001 From: Charlie Meister Date: Thu, 3 Apr 2025 20:45:15 +0200 Subject: [PATCH 03/41] WIP: adjust saga code to use new update actions and fixing typing in some places --- .../oxalis/model/sagas/save_saga.ts | 7 ++- .../model/sagas/skeletontracing_saga.ts | 54 ++++++++++++++++-- .../oxalis/model/sagas/update_actions.ts | 26 ++++++++- .../oxalis/model/sagas/volumetracing_saga.tsx | 56 +++++++++++++++++-- .../javascripts/oxalis/view/version_entry.tsx | 24 ++++++++ .../test/sagas/compact_toggle_actions.spec.ts | 1 + .../test/sagas/skeletontracing_saga.spec.ts | 1 + 7 files changed, 157 insertions(+), 12 deletions(-) diff --git a/frontend/javascripts/oxalis/model/sagas/save_saga.ts b/frontend/javascripts/oxalis/model/sagas/save_saga.ts index f0b306608e5..f8918a1f499 100644 --- a/frontend/javascripts/oxalis/model/sagas/save_saga.ts +++ b/frontend/javascripts/oxalis/model/sagas/save_saga.ts @@ -350,18 +350,19 @@ export function performDiffTracing( flycam: Flycam, prevTdCamera: CameraData, tdCamera: CameraData, + activeUserId: string | null, ): Array { let actions: Array = []; if (prevTracing.type === "skeleton" && tracing.type === "skeleton") { actions = actions.concat( - Array.from(diffSkeletonTracing(prevTracing, tracing, prevFlycam, flycam)), + Array.from(diffSkeletonTracing(prevTracing, tracing, prevFlycam, flycam, activeUserId)), ); } if (prevTracing.type === "volume" && tracing.type === "volume") { actions = actions.concat( - Array.from(diffVolumeTracing(prevTracing, tracing, prevFlycam, flycam)), + Array.from(diffVolumeTracing(prevTracing, tracing, prevFlycam, flycam, activeUserId)), ); } @@ -394,6 +395,7 @@ export function* setupSavingForTracingType( | SkeletonTracing; let prevFlycam = yield* select((state) => state.flycam); let prevTdCamera = yield* select((state) => state.viewModeData.plane.tdCamera); + const activeUserId = yield* select((state) => state.activeUser?.id || null); yield* call(ensureWkReady); while (true) { @@ -434,6 +436,7 @@ export function* setupSavingForTracingType( flycam, prevTdCamera, tdCamera, + activeUserId, ), ), tracing, diff --git a/frontend/javascripts/oxalis/model/sagas/skeletontracing_saga.ts b/frontend/javascripts/oxalis/model/sagas/skeletontracing_saga.ts index c9013bb8093..da71568d5da 100644 --- a/frontend/javascripts/oxalis/model/sagas/skeletontracing_saga.ts +++ b/frontend/javascripts/oxalis/model/sagas/skeletontracing_saga.ts @@ -48,19 +48,21 @@ import type { Saga } from "oxalis/model/sagas/effect-generators"; import { select } from "oxalis/model/sagas/effect-generators"; import type { UpdateActionWithoutIsolationRequirement } from "oxalis/model/sagas/update_actions"; import { + addUserBoundingBoxSkeletonAction, createEdge, createNode, createTree, deleteEdge, deleteNode, deleteTree, + deleteUserBoundingBoxInSkeletonTracingAction, updateNode, updateSkeletonTracing, updateTree, updateTreeEdgesVisibility, updateTreeGroups, updateTreeVisibility, - updateUserBoundingBoxesInSkeletonTracing, + updateUserBoundingBoxInSkeletonTracingAction, } from "oxalis/model/sagas/update_actions"; import { api } from "oxalis/singletons"; import type { @@ -612,6 +614,7 @@ export function* diffSkeletonTracing( skeletonTracing: SkeletonTracing, prevFlycam: Flycam, flycam: Flycam, + activeUserId: string | null, ): Generator { if (prevSkeletonTracing !== skeletonTracing) { for (const action of cachedDiffTrees( @@ -638,10 +641,53 @@ export function* diffSkeletonTracing( } if (!_.isEqual(prevSkeletonTracing.userBoundingBoxes, skeletonTracing.userBoundingBoxes)) { - yield updateUserBoundingBoxesInSkeletonTracing( - skeletonTracing.userBoundingBoxes, - skeletonTracing.tracingId, + const { + onlyA: deletedBBoxIds, + onlyB: addedBBoxIds, + both: changedBBoxIds, + } = Utils.diffArrays( + _.map(prevSkeletonTracing.userBoundingBoxes, (bbox) => bbox.id), + _.map(skeletonTracing.userBoundingBoxes, (bbox) => bbox.id), ); + for (const id of deletedBBoxIds) { + yield deleteUserBoundingBoxInSkeletonTracingAction( + id, + skeletonTracing.tracingId, + Date.now(), + activeUserId, + null, + ); + } + for (const id of addedBBoxIds) { + const bbox = skeletonTracing.userBoundingBoxes.find((bbox) => bbox.id === id); + if (bbox) { + yield addUserBoundingBoxSkeletonAction( + bbox, + skeletonTracing.tracingId, + Date.now(), + activeUserId, + null, + ); + } else { + Toast.error(`User bounding box with id ${id} not found in skeleton tracing.`); + } + } + for (const id of changedBBoxIds) { + const bbox = skeletonTracing.userBoundingBoxes.find((bbox) => bbox.id === id); + if (bbox) { + // TODO_charlie only update changed props + yield updateUserBoundingBoxInSkeletonTracingAction( + bbox.id, + bbox, + skeletonTracing.tracingId, + Date.now(), + activeUserId, + null, + ); + } else { + Toast.error(`User bounding box with id ${id} not found in skeleton tracing.`); + } + } } } export default [ diff --git a/frontend/javascripts/oxalis/model/sagas/update_actions.ts b/frontend/javascripts/oxalis/model/sagas/update_actions.ts index d29101a39ee..2a0e70394ff 100644 --- a/frontend/javascripts/oxalis/model/sagas/update_actions.ts +++ b/frontend/javascripts/oxalis/model/sagas/update_actions.ts @@ -38,6 +38,22 @@ export type CreateSegmentUpdateAction = ReturnType; export type DeleteSegmentUpdateAction = ReturnType; export type DeleteSegmentDataUpdateAction = ReturnType; +export type AddUserBoundingBoxSkeletonAction = ReturnType; +export type AddUserBoundingBoxInVolumeTracingAction = ReturnType< + typeof addUserBoundingBoxInVolumeTracingAction +>; +export type DeleteUserBoundingBoxInSkeletonTracingAction = ReturnType< + typeof deleteUserBoundingBoxInSkeletonTracingAction +>; +export type DeleteUserBoundingBoxInVolumeTracingAction = ReturnType< + typeof deleteUserBoundingBoxInVolumeTracingAction +>; +export type UpdateUserBoundingBoxInSkeletonTracingAction = ReturnType< + typeof updateUserBoundingBoxInSkeletonTracingAction +>; +export type UpdateUserBoundingBoxInVolumeTracingAction = ReturnType< + typeof updateUserBoundingBoxInVolumeTracingAction +>; type UpdateUserBoundingBoxesInSkeletonTracingUpdateAction = ReturnType< typeof updateUserBoundingBoxesInSkeletonTracing >; @@ -83,6 +99,12 @@ export type UpdateActionWithoutIsolationRequirement = | DeleteEdgeUpdateAction | UpdateSkeletonTracingUpdateAction | UpdateVolumeTracingUpdateAction + | AddUserBoundingBoxSkeletonAction + | AddUserBoundingBoxInVolumeTracingAction + | DeleteUserBoundingBoxInSkeletonTracingAction + | DeleteUserBoundingBoxInVolumeTracingAction + | UpdateUserBoundingBoxInSkeletonTracingAction + | UpdateUserBoundingBoxInVolumeTracingAction | UpdateUserBoundingBoxesInSkeletonTracingUpdateAction | UpdateUserBoundingBoxesInVolumeTracingUpdateAction | CreateSegmentUpdateAction @@ -386,7 +408,7 @@ export function addUserBoundingBoxSkeletonAction( info: string | null, ) { return { - name: "addUserBoundingBoxSkeleton", + name: "addUserBoundingBoxSkeletonAction", value: { boundingBox: convertUserBoundingBoxesFromFrontendToServer([boundingBox]), actionTracingId, @@ -405,7 +427,7 @@ export function addUserBoundingBoxInVolumeTracingAction( info: string | null, ) { return { - name: "addUserBoundingBoxSkeleton", + name: "addUserBoundingBoxVolumeAction", value: { boundingBox: convertUserBoundingBoxesFromFrontendToServer([boundingBox]), actionTracingId, diff --git a/frontend/javascripts/oxalis/model/sagas/volumetracing_saga.tsx b/frontend/javascripts/oxalis/model/sagas/volumetracing_saga.tsx index 8a379725061..302e8ef1e6e 100644 --- a/frontend/javascripts/oxalis/model/sagas/volumetracing_saga.tsx +++ b/frontend/javascripts/oxalis/model/sagas/volumetracing_saga.tsx @@ -74,14 +74,16 @@ import { } from "oxalis/model/sagas/saga_helpers"; import { type UpdateActionWithoutIsolationRequirement, + addUserBoundingBoxInVolumeTracingAction, createSegmentVolumeAction, deleteSegmentDataVolumeAction, deleteSegmentVolumeAction, + deleteUserBoundingBoxInVolumeTracingAction, removeFallbackLayer, updateMappingName, updateSegmentGroups, updateSegmentVolumeAction, - updateUserBoundingBoxesInVolumeTracing, + updateUserBoundingBoxInVolumeTracingAction, updateVolumeTracing, } from "oxalis/model/sagas/update_actions"; import type VolumeLayer from "oxalis/model/volumetracing/volumelayer"; @@ -95,6 +97,8 @@ import { floodFill } from "./volume/floodfill_saga"; import { type BooleanBox, createVolumeLayer, labelWithVoxelBuffer2D } from "./volume/helpers"; import maybeInterpolateSegmentationLayer from "./volume/volume_interpolation_saga"; +import * as Utils from "libs/utils"; + const OVERWRITE_EMPTY_WARNING_KEY = "OVERWRITE-EMPTY-WARNING"; export function* watchVolumeTracingAsync(): Saga { @@ -472,6 +476,7 @@ export function* diffVolumeTracing( volumeTracing: VolumeTracing, prevFlycam: Flycam, flycam: Flycam, + activeUserId: string | null, ): Generator { if (updateTracingPredicate(prevVolumeTracing, volumeTracing, prevFlycam, flycam)) { yield updateVolumeTracing( @@ -484,10 +489,53 @@ export function* diffVolumeTracing( } if (!_.isEqual(prevVolumeTracing.userBoundingBoxes, volumeTracing.userBoundingBoxes)) { - yield updateUserBoundingBoxesInVolumeTracing( - volumeTracing.userBoundingBoxes, - volumeTracing.tracingId, + const { + onlyA: deletedBBoxIds, + onlyB: addedBBoxIds, + both: changedBBoxIds, + } = Utils.diffArrays( + _.map(prevVolumeTracing.userBoundingBoxes, (bbox) => bbox.id), + _.map(volumeTracing.userBoundingBoxes, (bbox) => bbox.id), ); + for (const id of deletedBBoxIds) { + yield deleteUserBoundingBoxInVolumeTracingAction( + id, + volumeTracing.tracingId, + Date.now(), + activeUserId, + null, + ); + } + for (const id of addedBBoxIds) { + const bbox = volumeTracing.userBoundingBoxes.find((bbox) => bbox.id === id); + if (bbox) { + yield addUserBoundingBoxInVolumeTracingAction( + bbox, + volumeTracing.tracingId, + Date.now(), + activeUserId, + null, + ); + } else { + Toast.error(`User bounding box with id ${id} not found in volume tracing.`); + } + } + for (const id of changedBBoxIds) { + const bbox = volumeTracing.userBoundingBoxes.find((bbox) => bbox.id === id); + if (bbox) { + // TODO_charlie only update changed props + yield updateUserBoundingBoxInVolumeTracingAction( + bbox.id, + bbox, + volumeTracing.tracingId, + Date.now(), + activeUserId, + null, + ); + } else { + Toast.error(`User bounding box with id ${id} not found in volume tracing.`); + } + } } if (prevVolumeTracing !== volumeTracing) { diff --git a/frontend/javascripts/oxalis/view/version_entry.tsx b/frontend/javascripts/oxalis/view/version_entry.tsx index 7a7b92c0042..e763db4450f 100644 --- a/frontend/javascripts/oxalis/view/version_entry.tsx +++ b/frontend/javascripts/oxalis/view/version_entry.tsx @@ -84,6 +84,30 @@ const descriptionFns: Record< description: "Updated a bounding box.", icon: , }), + addUserBoundingBoxSkeletonAction: (): Description => ({ + description: "Added a bounding box.", + icon: , + }), + addUserBoundingBoxVolumeAction: (): Description => ({ + description: "Added a bounding box.", + icon: , + }), + deleteUserBoundingBoxSkeletonAction: (): Description => ({ + description: "Deleted a bounding box.", + icon: , + }), + deleteUserBoundingBoxVolumeAction: (): Description => ({ + description: "Deleted a bounding box.", + icon: , + }), + updateUserBoundingBoxSkeletonAction: (): Description => ({ + description: "Updated a bounding box.", + icon: , + }), + updateUserBoundingBoxVolumeAction: (): Description => ({ + description: "Updated a bounding box.", + icon: , + }), removeFallbackLayer: (): Description => ({ description: "Removed the segmentation fallback layer.", icon: , diff --git a/frontend/javascripts/test/sagas/compact_toggle_actions.spec.ts b/frontend/javascripts/test/sagas/compact_toggle_actions.spec.ts index f4f17661d20..6614a2e948f 100644 --- a/frontend/javascripts/test/sagas/compact_toggle_actions.spec.ts +++ b/frontend/javascripts/test/sagas/compact_toggle_actions.spec.ts @@ -107,6 +107,7 @@ function testDiffing(prevState: OxalisState, nextState: OxalisState) { enforceSkeletonTracing(nextState.tracing), flycamMock, flycamMock, + nextState.activeUser?.id || null, ), ), ), diff --git a/frontend/javascripts/test/sagas/skeletontracing_saga.spec.ts b/frontend/javascripts/test/sagas/skeletontracing_saga.spec.ts index e55b0919f93..81429127a0b 100644 --- a/frontend/javascripts/test/sagas/skeletontracing_saga.spec.ts +++ b/frontend/javascripts/test/sagas/skeletontracing_saga.spec.ts @@ -63,6 +63,7 @@ function testDiffing( enforceSkeletonTracing(nextTracing), prevFlycam, flycam, + null, ), ), ); From cdf546f69b94d4485b695cac3619dd0515e7d340 Mon Sep 17 00:00:00 2001 From: Charlie Meister Date: Fri, 4 Apr 2025 11:21:11 +0200 Subject: [PATCH 04/41] [frontend] remove activeUser, timestamp and info from updateActions --- .../oxalis/model/sagas/save_saga.ts | 7 ++-- .../model/sagas/skeletontracing_saga.ts | 20 ++--------- .../oxalis/model/sagas/update_actions.ts | 36 ------------------- .../oxalis/model/sagas/volumetracing_saga.tsx | 26 ++------------ 4 files changed, 7 insertions(+), 82 deletions(-) diff --git a/frontend/javascripts/oxalis/model/sagas/save_saga.ts b/frontend/javascripts/oxalis/model/sagas/save_saga.ts index f8918a1f499..f0b306608e5 100644 --- a/frontend/javascripts/oxalis/model/sagas/save_saga.ts +++ b/frontend/javascripts/oxalis/model/sagas/save_saga.ts @@ -350,19 +350,18 @@ export function performDiffTracing( flycam: Flycam, prevTdCamera: CameraData, tdCamera: CameraData, - activeUserId: string | null, ): Array { let actions: Array = []; if (prevTracing.type === "skeleton" && tracing.type === "skeleton") { actions = actions.concat( - Array.from(diffSkeletonTracing(prevTracing, tracing, prevFlycam, flycam, activeUserId)), + Array.from(diffSkeletonTracing(prevTracing, tracing, prevFlycam, flycam)), ); } if (prevTracing.type === "volume" && tracing.type === "volume") { actions = actions.concat( - Array.from(diffVolumeTracing(prevTracing, tracing, prevFlycam, flycam, activeUserId)), + Array.from(diffVolumeTracing(prevTracing, tracing, prevFlycam, flycam)), ); } @@ -395,7 +394,6 @@ export function* setupSavingForTracingType( | SkeletonTracing; let prevFlycam = yield* select((state) => state.flycam); let prevTdCamera = yield* select((state) => state.viewModeData.plane.tdCamera); - const activeUserId = yield* select((state) => state.activeUser?.id || null); yield* call(ensureWkReady); while (true) { @@ -436,7 +434,6 @@ export function* setupSavingForTracingType( flycam, prevTdCamera, tdCamera, - activeUserId, ), ), tracing, diff --git a/frontend/javascripts/oxalis/model/sagas/skeletontracing_saga.ts b/frontend/javascripts/oxalis/model/sagas/skeletontracing_saga.ts index da71568d5da..1f379f3e784 100644 --- a/frontend/javascripts/oxalis/model/sagas/skeletontracing_saga.ts +++ b/frontend/javascripts/oxalis/model/sagas/skeletontracing_saga.ts @@ -614,7 +614,6 @@ export function* diffSkeletonTracing( skeletonTracing: SkeletonTracing, prevFlycam: Flycam, flycam: Flycam, - activeUserId: string | null, ): Generator { if (prevSkeletonTracing !== skeletonTracing) { for (const action of cachedDiffTrees( @@ -650,24 +649,12 @@ export function* diffSkeletonTracing( _.map(skeletonTracing.userBoundingBoxes, (bbox) => bbox.id), ); for (const id of deletedBBoxIds) { - yield deleteUserBoundingBoxInSkeletonTracingAction( - id, - skeletonTracing.tracingId, - Date.now(), - activeUserId, - null, - ); + yield deleteUserBoundingBoxInSkeletonTracingAction(id, skeletonTracing.tracingId); } for (const id of addedBBoxIds) { const bbox = skeletonTracing.userBoundingBoxes.find((bbox) => bbox.id === id); if (bbox) { - yield addUserBoundingBoxSkeletonAction( - bbox, - skeletonTracing.tracingId, - Date.now(), - activeUserId, - null, - ); + yield addUserBoundingBoxSkeletonAction(bbox, skeletonTracing.tracingId); } else { Toast.error(`User bounding box with id ${id} not found in skeleton tracing.`); } @@ -680,9 +667,6 @@ export function* diffSkeletonTracing( bbox.id, bbox, skeletonTracing.tracingId, - Date.now(), - activeUserId, - null, ); } else { Toast.error(`User bounding box with id ${id} not found in skeleton tracing.`); diff --git a/frontend/javascripts/oxalis/model/sagas/update_actions.ts b/frontend/javascripts/oxalis/model/sagas/update_actions.ts index 2a0e70394ff..7ebe8107349 100644 --- a/frontend/javascripts/oxalis/model/sagas/update_actions.ts +++ b/frontend/javascripts/oxalis/model/sagas/update_actions.ts @@ -403,18 +403,12 @@ export function updateVolumeTracing( export function addUserBoundingBoxSkeletonAction( boundingBox: UserBoundingBox, actionTracingId: string, - timestamp: number | null, - authorId: string | null, - info: string | null, ) { return { name: "addUserBoundingBoxSkeletonAction", value: { boundingBox: convertUserBoundingBoxesFromFrontendToServer([boundingBox]), actionTracingId, - actionTimestamp: timestamp, - actionAuthorId: authorId, - info, }, } as const; } @@ -422,18 +416,12 @@ export function addUserBoundingBoxSkeletonAction( export function addUserBoundingBoxInVolumeTracingAction( boundingBox: UserBoundingBox, actionTracingId: string, - timestamp: number | null, - authorId: string | null, - info: string | null, ) { return { name: "addUserBoundingBoxVolumeAction", value: { boundingBox: convertUserBoundingBoxesFromFrontendToServer([boundingBox]), actionTracingId, - actionTimestamp: timestamp, - actionAuthorId: authorId, - info, }, } as const; } @@ -441,18 +429,12 @@ export function addUserBoundingBoxInVolumeTracingAction( export function deleteUserBoundingBoxInSkeletonTracingAction( boundingBoxId: number, actionTracingId: string, - timestamp: number | null, - authorId: string | null, - info: string | null, ) { return { name: "deleteUserBoundingBoxSkeletonAction", value: { boundingBoxId, actionTracingId, - actionTimestamp: timestamp, - actionAuthorId: authorId, - info, }, } as const; } @@ -460,18 +442,12 @@ export function deleteUserBoundingBoxInSkeletonTracingAction( export function deleteUserBoundingBoxInVolumeTracingAction( boundingBoxId: number, actionTracingId: string, - timestamp: number | null, - authorId: string | null, - info: string | null, ) { return { name: "deleteUserBoundingBoxVolumeAction", value: { boundingBoxId, actionTracingId, - actionTimestamp: timestamp, - actionAuthorId: authorId, - info, }, } as const; } @@ -480,9 +456,6 @@ export function updateUserBoundingBoxInSkeletonTracingAction( boundingBoxId: number, updatedProps: Partial, actionTracingId: string, - timestamp: number | null, - authorId: string | null, - info: string | null, ) { const updatedPropKeys = Object.keys(updatedProps); return { @@ -492,9 +465,6 @@ export function updateUserBoundingBoxInSkeletonTracingAction( actionTracingId, updatedProps, updatedPropKeys, - actionTimestamp: timestamp, - actionAuthorId: authorId, - info, }, } as const; } @@ -503,9 +473,6 @@ export function updateUserBoundingBoxInVolumeTracingAction( boundingBoxId: number, updatedProps: Partial, actionTracingId: string, - timestamp: number | null, - authorId: string | null, - info: string | null, ) { const updatedPropKeys = Object.keys(updatedProps); return { @@ -515,9 +482,6 @@ export function updateUserBoundingBoxInVolumeTracingAction( actionTracingId, updatedProps, updatedPropKeys, - actionTimestamp: timestamp, - actionAuthorId: authorId, - info, }, } as const; } diff --git a/frontend/javascripts/oxalis/model/sagas/volumetracing_saga.tsx b/frontend/javascripts/oxalis/model/sagas/volumetracing_saga.tsx index 302e8ef1e6e..f5c36f64495 100644 --- a/frontend/javascripts/oxalis/model/sagas/volumetracing_saga.tsx +++ b/frontend/javascripts/oxalis/model/sagas/volumetracing_saga.tsx @@ -476,7 +476,6 @@ export function* diffVolumeTracing( volumeTracing: VolumeTracing, prevFlycam: Flycam, flycam: Flycam, - activeUserId: string | null, ): Generator { if (updateTracingPredicate(prevVolumeTracing, volumeTracing, prevFlycam, flycam)) { yield updateVolumeTracing( @@ -498,24 +497,12 @@ export function* diffVolumeTracing( _.map(volumeTracing.userBoundingBoxes, (bbox) => bbox.id), ); for (const id of deletedBBoxIds) { - yield deleteUserBoundingBoxInVolumeTracingAction( - id, - volumeTracing.tracingId, - Date.now(), - activeUserId, - null, - ); + yield deleteUserBoundingBoxInVolumeTracingAction(id, volumeTracing.tracingId); } for (const id of addedBBoxIds) { const bbox = volumeTracing.userBoundingBoxes.find((bbox) => bbox.id === id); if (bbox) { - yield addUserBoundingBoxInVolumeTracingAction( - bbox, - volumeTracing.tracingId, - Date.now(), - activeUserId, - null, - ); + yield addUserBoundingBoxInVolumeTracingAction(bbox, volumeTracing.tracingId); } else { Toast.error(`User bounding box with id ${id} not found in volume tracing.`); } @@ -524,14 +511,7 @@ export function* diffVolumeTracing( const bbox = volumeTracing.userBoundingBoxes.find((bbox) => bbox.id === id); if (bbox) { // TODO_charlie only update changed props - yield updateUserBoundingBoxInVolumeTracingAction( - bbox.id, - bbox, - volumeTracing.tracingId, - Date.now(), - activeUserId, - null, - ); + yield updateUserBoundingBoxInVolumeTracingAction(bbox.id, bbox, volumeTracing.tracingId); } else { Toast.error(`User bounding box with id ${id} not found in volume tracing.`); } From bfb8746af6cf224b1702b01285c48714e5fa734e Mon Sep 17 00:00:00 2001 From: Charlie Meister Date: Fri, 4 Apr 2025 13:56:19 +0200 Subject: [PATCH 05/41] [frontend] make saga code more DRY and only update changed props --- .../model/sagas/skeletontracing_saga.ts | 101 +++++++++++------- .../oxalis/model/sagas/update_actions.ts | 6 +- .../oxalis/model/sagas/volumetracing_saga.tsx | 44 ++------ 3 files changed, 77 insertions(+), 74 deletions(-) diff --git a/frontend/javascripts/oxalis/model/sagas/skeletontracing_saga.ts b/frontend/javascripts/oxalis/model/sagas/skeletontracing_saga.ts index 1f379f3e784..561910de9a7 100644 --- a/frontend/javascripts/oxalis/model/sagas/skeletontracing_saga.ts +++ b/frontend/javascripts/oxalis/model/sagas/skeletontracing_saga.ts @@ -48,7 +48,8 @@ import type { Saga } from "oxalis/model/sagas/effect-generators"; import { select } from "oxalis/model/sagas/effect-generators"; import type { UpdateActionWithoutIsolationRequirement } from "oxalis/model/sagas/update_actions"; import { - addUserBoundingBoxSkeletonAction, + addUserBoundingBoxInSkeletonTracingAction, + addUserBoundingBoxInVolumeTracingAction, createEdge, createNode, createTree, @@ -56,6 +57,7 @@ import { deleteNode, deleteTree, deleteUserBoundingBoxInSkeletonTracingAction, + deleteUserBoundingBoxInVolumeTracingAction, updateNode, updateSkeletonTracing, updateTree, @@ -63,6 +65,7 @@ import { updateTreeGroups, updateTreeVisibility, updateUserBoundingBoxInSkeletonTracingAction, + updateUserBoundingBoxInVolumeTracingAction, } from "oxalis/model/sagas/update_actions"; import { api } from "oxalis/singletons"; import type { @@ -73,6 +76,7 @@ import type { SkeletonTracing, Tree, TreeMap, + UserBoundingBox, } from "oxalis/store"; import Store from "oxalis/store"; import { @@ -86,7 +90,7 @@ import { takeEvery, throttle, } from "typed-redux-saga"; -import type { ServerSkeletonTracing } from "types/api_flow_types"; +import { AnnotationLayerEnum, type ServerSkeletonTracing } from "types/api_flow_types"; import { ensureWkReady } from "./ready_sagas"; import { takeWithBatchActionSupport } from "./saga_helpers"; @@ -609,6 +613,59 @@ export const cachedDiffTrees = memoizeOne((tracingId: string, prevTrees: TreeMap Array.from(diffTrees(tracingId, prevTrees, trees)), ); +export function* diffBoundingBoxes( + prevBoundingBoxes: UserBoundingBox[], + currentBoundingBoxes: UserBoundingBox[], + tracingId: string, + tracingType: AnnotationLayerEnum, +) { + const { + onlyA: deletedBBoxIds, + onlyB: addedBBoxIds, + both: changedBBoxIds, + } = Utils.diffArrays( + _.map(prevBoundingBoxes, (bbox) => bbox.id), + _.map(currentBoundingBoxes, (bbox) => bbox.id), + ); + const addBBoxAction = + tracingType === AnnotationLayerEnum.Skeleton + ? addUserBoundingBoxInSkeletonTracingAction + : addUserBoundingBoxInVolumeTracingAction; + const deleteBBoxAction = + tracingType === AnnotationLayerEnum.Skeleton + ? deleteUserBoundingBoxInSkeletonTracingAction + : deleteUserBoundingBoxInVolumeTracingAction; + const updateBBoxAction = + tracingType === AnnotationLayerEnum.Skeleton + ? updateUserBoundingBoxInSkeletonTracingAction + : updateUserBoundingBoxInVolumeTracingAction; + const getErrorMessage = (id: number) => + `User bounding box with id ${id} not found in ${tracingType} tracing.`; + for (const id of deletedBBoxIds) { + yield deleteBBoxAction(id, tracingId); + } + for (const id of addedBBoxIds) { + const bbox = currentBoundingBoxes.find((bbox) => bbox.id === id); + if (bbox) { + yield addBBoxAction(bbox, tracingId); + } else { + Toast.error(getErrorMessage(id)); + } + } + for (const id of changedBBoxIds) { + const currentBbox = currentBoundingBoxes.find((bbox) => bbox.id === id); + const prevBbox = prevBoundingBoxes.find((bbox) => bbox.id === id); + if (currentBbox == null || prevBbox == null) { + Toast.error(getErrorMessage(id)); + continue; + } + const diffBBox = Utils.diffObjects(currentBbox, prevBbox); + //TODO_C remove + console.log("diffBBox", diffBBox); + yield updateBBoxAction(currentBbox.id, diffBBox, tracingId); + } +} + export function* diffSkeletonTracing( prevSkeletonTracing: SkeletonTracing, skeletonTracing: SkeletonTracing, @@ -639,40 +696,12 @@ export function* diffSkeletonTracing( ); } - if (!_.isEqual(prevSkeletonTracing.userBoundingBoxes, skeletonTracing.userBoundingBoxes)) { - const { - onlyA: deletedBBoxIds, - onlyB: addedBBoxIds, - both: changedBBoxIds, - } = Utils.diffArrays( - _.map(prevSkeletonTracing.userBoundingBoxes, (bbox) => bbox.id), - _.map(skeletonTracing.userBoundingBoxes, (bbox) => bbox.id), - ); - for (const id of deletedBBoxIds) { - yield deleteUserBoundingBoxInSkeletonTracingAction(id, skeletonTracing.tracingId); - } - for (const id of addedBBoxIds) { - const bbox = skeletonTracing.userBoundingBoxes.find((bbox) => bbox.id === id); - if (bbox) { - yield addUserBoundingBoxSkeletonAction(bbox, skeletonTracing.tracingId); - } else { - Toast.error(`User bounding box with id ${id} not found in skeleton tracing.`); - } - } - for (const id of changedBBoxIds) { - const bbox = skeletonTracing.userBoundingBoxes.find((bbox) => bbox.id === id); - if (bbox) { - // TODO_charlie only update changed props - yield updateUserBoundingBoxInSkeletonTracingAction( - bbox.id, - bbox, - skeletonTracing.tracingId, - ); - } else { - Toast.error(`User bounding box with id ${id} not found in skeleton tracing.`); - } - } - } + diffBoundingBoxes( + skeletonTracing.userBoundingBoxes, + prevSkeletonTracing.userBoundingBoxes, + skeletonTracing.tracingId, + AnnotationLayerEnum.Skeleton, + ); } export default [ watchSkeletonTracingAsync, diff --git a/frontend/javascripts/oxalis/model/sagas/update_actions.ts b/frontend/javascripts/oxalis/model/sagas/update_actions.ts index 7ebe8107349..a46d7f335a8 100644 --- a/frontend/javascripts/oxalis/model/sagas/update_actions.ts +++ b/frontend/javascripts/oxalis/model/sagas/update_actions.ts @@ -38,7 +38,9 @@ export type CreateSegmentUpdateAction = ReturnType; export type DeleteSegmentUpdateAction = ReturnType; export type DeleteSegmentDataUpdateAction = ReturnType; -export type AddUserBoundingBoxSkeletonAction = ReturnType; +export type AddUserBoundingBoxSkeletonAction = ReturnType< + typeof addUserBoundingBoxInSkeletonTracingAction +>; export type AddUserBoundingBoxInVolumeTracingAction = ReturnType< typeof addUserBoundingBoxInVolumeTracingAction >; @@ -400,7 +402,7 @@ export function updateVolumeTracing( } as const; } -export function addUserBoundingBoxSkeletonAction( +export function addUserBoundingBoxInSkeletonTracingAction( boundingBox: UserBoundingBox, actionTracingId: string, ) { diff --git a/frontend/javascripts/oxalis/model/sagas/volumetracing_saga.tsx b/frontend/javascripts/oxalis/model/sagas/volumetracing_saga.tsx index f5c36f64495..531af825025 100644 --- a/frontend/javascripts/oxalis/model/sagas/volumetracing_saga.tsx +++ b/frontend/javascripts/oxalis/model/sagas/volumetracing_saga.tsx @@ -1,7 +1,6 @@ import { diffDiffableMaps } from "libs/diffable_map"; import { V3 } from "libs/mjs"; import Toast from "libs/toast"; -import _ from "lodash"; import memoizeOne from "memoize-one"; import type { AnnotationTool, @@ -74,16 +73,13 @@ import { } from "oxalis/model/sagas/saga_helpers"; import { type UpdateActionWithoutIsolationRequirement, - addUserBoundingBoxInVolumeTracingAction, createSegmentVolumeAction, deleteSegmentDataVolumeAction, deleteSegmentVolumeAction, - deleteUserBoundingBoxInVolumeTracingAction, removeFallbackLayer, updateMappingName, updateSegmentGroups, updateSegmentVolumeAction, - updateUserBoundingBoxInVolumeTracingAction, updateVolumeTracing, } from "oxalis/model/sagas/update_actions"; import type VolumeLayer from "oxalis/model/volumetracing/volumelayer"; @@ -91,14 +87,14 @@ import { Model, api } from "oxalis/singletons"; import type { Flycam, SegmentMap, VolumeTracing } from "oxalis/store"; import type { ActionPattern } from "redux-saga/effects"; import { actionChannel, call, fork, put, takeEvery, takeLatest } from "typed-redux-saga"; +import { AnnotationLayerEnum } from "types/api_flow_types"; import { pushSaveQueueTransaction } from "../actions/save_actions"; import { ensureWkReady } from "./ready_sagas"; +import { diffBoundingBoxes } from "./skeletontracing_saga"; import { floodFill } from "./volume/floodfill_saga"; import { type BooleanBox, createVolumeLayer, labelWithVoxelBuffer2D } from "./volume/helpers"; import maybeInterpolateSegmentationLayer from "./volume/volume_interpolation_saga"; -import * as Utils from "libs/utils"; - const OVERWRITE_EMPTY_WARNING_KEY = "OVERWRITE-EMPTY-WARNING"; export function* watchVolumeTracingAsync(): Saga { @@ -487,36 +483,12 @@ export function* diffVolumeTracing( ); } - if (!_.isEqual(prevVolumeTracing.userBoundingBoxes, volumeTracing.userBoundingBoxes)) { - const { - onlyA: deletedBBoxIds, - onlyB: addedBBoxIds, - both: changedBBoxIds, - } = Utils.diffArrays( - _.map(prevVolumeTracing.userBoundingBoxes, (bbox) => bbox.id), - _.map(volumeTracing.userBoundingBoxes, (bbox) => bbox.id), - ); - for (const id of deletedBBoxIds) { - yield deleteUserBoundingBoxInVolumeTracingAction(id, volumeTracing.tracingId); - } - for (const id of addedBBoxIds) { - const bbox = volumeTracing.userBoundingBoxes.find((bbox) => bbox.id === id); - if (bbox) { - yield addUserBoundingBoxInVolumeTracingAction(bbox, volumeTracing.tracingId); - } else { - Toast.error(`User bounding box with id ${id} not found in volume tracing.`); - } - } - for (const id of changedBBoxIds) { - const bbox = volumeTracing.userBoundingBoxes.find((bbox) => bbox.id === id); - if (bbox) { - // TODO_charlie only update changed props - yield updateUserBoundingBoxInVolumeTracingAction(bbox.id, bbox, volumeTracing.tracingId); - } else { - Toast.error(`User bounding box with id ${id} not found in volume tracing.`); - } - } - } + diffBoundingBoxes( + prevVolumeTracing.userBoundingBoxes, + volumeTracing.userBoundingBoxes, + volumeTracing.tracingId, + AnnotationLayerEnum.Volume, + ); if (prevVolumeTracing !== volumeTracing) { if (prevVolumeTracing.segments !== volumeTracing.segments) { From 78d8d1850d86c4bfd47f9058c70a47553f12586f Mon Sep 17 00:00:00 2001 From: Charlie Meister Date: Fri, 4 Apr 2025 15:07:26 +0200 Subject: [PATCH 06/41] [frontend] correctly call helper function --- .../javascripts/oxalis/model/sagas/skeletontracing_saga.ts | 4 ++-- .../javascripts/oxalis/model/sagas/volumetracing_saga.tsx | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/frontend/javascripts/oxalis/model/sagas/skeletontracing_saga.ts b/frontend/javascripts/oxalis/model/sagas/skeletontracing_saga.ts index 561910de9a7..f94075122b6 100644 --- a/frontend/javascripts/oxalis/model/sagas/skeletontracing_saga.ts +++ b/frontend/javascripts/oxalis/model/sagas/skeletontracing_saga.ts @@ -696,9 +696,9 @@ export function* diffSkeletonTracing( ); } - diffBoundingBoxes( - skeletonTracing.userBoundingBoxes, + yield* diffBoundingBoxes( prevSkeletonTracing.userBoundingBoxes, + skeletonTracing.userBoundingBoxes, skeletonTracing.tracingId, AnnotationLayerEnum.Skeleton, ); diff --git a/frontend/javascripts/oxalis/model/sagas/volumetracing_saga.tsx b/frontend/javascripts/oxalis/model/sagas/volumetracing_saga.tsx index 531af825025..1c02ea9554c 100644 --- a/frontend/javascripts/oxalis/model/sagas/volumetracing_saga.tsx +++ b/frontend/javascripts/oxalis/model/sagas/volumetracing_saga.tsx @@ -483,7 +483,7 @@ export function* diffVolumeTracing( ); } - diffBoundingBoxes( + yield* diffBoundingBoxes( prevVolumeTracing.userBoundingBoxes, volumeTracing.userBoundingBoxes, volumeTracing.tracingId, From c680cf285877a430c17ee20c8cd8eef452597b5b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20B=C3=BC=C3=9Femeyer?= <39529669+MichaelBuessemeyer@users.noreply.github.com> Date: Tue, 8 Apr 2025 18:44:44 +0200 Subject: [PATCH 07/41] have single update action for all user bounding box properties --- .../annotation/UpdateActions.scala | 42 +++-- .../tracings/NamedBoundingBox.scala | 19 +- .../updating/SkeletonUpdateActions.scala | 133 +++----------- .../tracings/volume/VolumeUpdateActions.scala | 167 +++++------------- 4 files changed, 116 insertions(+), 245 deletions(-) diff --git a/webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/annotation/UpdateActions.scala b/webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/annotation/UpdateActions.scala index fe0873f863b..a790185c066 100644 --- a/webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/annotation/UpdateActions.scala +++ b/webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/annotation/UpdateActions.scala @@ -5,7 +5,7 @@ import com.scalableminds.webknossos.tracingstore.tracings.editablemapping.{ SplitAgglomerateUpdateAction } import com.scalableminds.webknossos.tracingstore.tracings.skeleton.updating._ -import com.scalableminds.webknossos.tracingstore.tracings.volume._ +import com.scalableminds.webknossos.tracingstore.tracings.volume.{UpdateUserBoundingBoxVolumeAction, _} import play.api.libs.json._ trait UpdateAction { @@ -56,22 +56,18 @@ object UpdateAction { deserialize[UpdateUserBoundingBoxesSkeletonAction](jsonValue) case "addUserBoundingBoxSkeletonAction" => deserialize[AddUserBoundingBoxSkeletonAction](jsonValue) case "deleteUserBoundingBoxSkeletonAction" => deserialize[DeleteUserBoundingBoxSkeletonAction](jsonValue) - case "updateUserBoundingBoxBoundsSkeletonAction" => - deserialize[UpdateUserBoundingBoxBoundsSkeletonAction](jsonValue) - case "updateUserBoundingBoxVisibilityInSkeletonTracing" => - deserialize[UpdateUserBoundingBoxVisibilitySkeletonAction](jsonValue) - case "updateUserBoundingBoxNameSkeletonAction" => - deserialize[UpdateUserBoundingBoxNameSkeletonAction](jsonValue) - case "updateUserBoundingBoxColorSkeletonAction" => - deserialize[UpdateUserBoundingBoxColorSkeletonAction](jsonValue) + case "updateUserBoundingBoxSkeletonAction" => + deserialize[UpdateUserBoundingBoxSkeletonAction](jsonValue) // Volume case "updateBucket" => deserialize[UpdateBucketVolumeAction](jsonValue) case "updateVolumeTracing" => deserialize[UpdateTracingVolumeAction](jsonValue) + case "updateUserBoundingBoxVolumeAction" => + deserialize[UpdateUserBoundingBoxVolumeAction](jsonValue) + case "addUserBoundingBoxVolumeAction" => deserialize[AddUserBoundingBoxVolumeAction](jsonValue) + case "deleteUserBoundingBoxVolumeAction" => deserialize[DeleteUserBoundingBoxVolumeAction](jsonValue) case "updateUserBoundingBoxesInVolumeTracing" => deserialize[UpdateUserBoundingBoxesVolumeAction](jsonValue) - case "updateUserBoundingBoxVisibilityInVolumeTracing" => - deserialize[UpdateUserBoundingBoxVisibilityVolumeAction](jsonValue) case "removeFallbackLayer" => deserialize[RemoveFallbackLayerVolumeAction](jsonValue) case "importVolumeTracing" => deserialize[ImportVolumeDataVolumeAction](jsonValue) case "createSegment" => deserialize[CreateSegmentVolumeAction](jsonValue) @@ -148,9 +144,15 @@ object UpdateAction { case s: UpdateUserBoundingBoxesSkeletonAction => Json.obj("name" -> "updateUserBoundingBoxesInSkeletonTracing", "value" -> Json.toJson(s)(UpdateUserBoundingBoxesSkeletonAction.jsonFormat)) - case s: UpdateUserBoundingBoxVisibilitySkeletonAction => - Json.obj("name" -> "updateUserBoundingBoxVisibilityInSkeletonTracing", - "value" -> Json.toJson(s)(UpdateUserBoundingBoxVisibilitySkeletonAction.jsonFormat)) + case s: UpdateUserBoundingBoxSkeletonAction => + Json.obj("name" -> "updateUserBoundingBoxSkeletonAction", + "value" -> Json.toJson(s)(UpdateUserBoundingBoxSkeletonAction.jsonFormat)) + case s: AddUserBoundingBoxSkeletonAction => + Json.obj("name" -> "addUserBoundingBoxSkeletonAction", + "value" -> Json.toJson(s)(AddUserBoundingBoxSkeletonAction.jsonFormat)) + case s: DeleteUserBoundingBoxSkeletonAction => + Json.obj("name" -> "deleteUserBoundingBoxSkeletonAction", + "value" -> Json.toJson(s)(DeleteUserBoundingBoxSkeletonAction.jsonFormat)) // Volume case s: UpdateBucketVolumeAction => @@ -160,9 +162,15 @@ object UpdateAction { case s: UpdateUserBoundingBoxesVolumeAction => Json.obj("name" -> "updateUserBoundingBoxesInVolumeTracing", "value" -> Json.toJson(s)(UpdateUserBoundingBoxesVolumeAction.jsonFormat)) - case s: UpdateUserBoundingBoxVisibilityVolumeAction => - Json.obj("name" -> "updateUserBoundingBoxVisibilityInVolumeTracing", - "value" -> Json.toJson(s)(UpdateUserBoundingBoxVisibilityVolumeAction.jsonFormat)) + case s: UpdateUserBoundingBoxVolumeAction => + Json.obj("name" -> "updateUserBoundingBoxVolumeAction", + "value" -> Json.toJson(s)(UpdateUserBoundingBoxVolumeAction.jsonFormat)) + case s: AddUserBoundingBoxVolumeAction => + Json.obj("name" -> "addUserBoundingBoxVolumeAction", + "value" -> Json.toJson(s)(AddUserBoundingBoxVolumeAction.jsonFormat)) + case s: DeleteUserBoundingBoxVolumeAction => + Json.obj("name" -> "deleteUserBoundingBoxVolumeAction", + "value" -> Json.toJson(s)(DeleteUserBoundingBoxVolumeAction.jsonFormat)) case s: RemoveFallbackLayerVolumeAction => Json.obj("name" -> "removeFallbackLayer", "value" -> Json.toJson(s)(RemoveFallbackLayerVolumeAction.jsonFormat)) case s: ImportVolumeDataVolumeAction => diff --git a/webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/tracings/NamedBoundingBox.scala b/webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/tracings/NamedBoundingBox.scala index cb8753a09fa..79a114d6ffc 100644 --- a/webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/tracings/NamedBoundingBox.scala +++ b/webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/tracings/NamedBoundingBox.scala @@ -3,7 +3,11 @@ package com.scalableminds.webknossos.tracingstore.tracings import play.api.libs.json.{Json, OFormat} import com.scalableminds.util.geometry.BoundingBox import com.scalableminds.util.image.Color -import com.scalableminds.webknossos.datastore.geometry.{BoundingBoxProto, NamedBoundingBoxProto => ProtoBoundingBox} +import com.scalableminds.webknossos.datastore.geometry.{ + BoundingBoxProto, + ColorProto, + NamedBoundingBoxProto => ProtoBoundingBox +} import com.scalableminds.webknossos.datastore.helpers.ProtoGeometryImplicits import com.scalableminds.webknossos.tracingstore.tracings.skeleton.updating.SkeletonUpdateActionHelper @@ -18,3 +22,16 @@ case class NamedBoundingBox(id: Int, } object NamedBoundingBox { implicit val jsonFormat: OFormat[NamedBoundingBox] = Json.format[NamedBoundingBox] } + +case class NamedBoundingBoxUpdate(id: Option[Int], + name: Option[String], + isVisible: Option[Boolean], + color: Option[Color], + boundingBox: Option[BoundingBox], +) extends ProtoGeometryImplicits { + def colorOptProto: Option[ColorProto] = colorOptToProto(color) + def boundingBoxProto: Option[BoundingBoxProto] = boundingBoxOptToProto(boundingBox) +} +object NamedBoundingBoxUpdate { + implicit val jsonFormat: OFormat[NamedBoundingBoxUpdate] = Json.format[NamedBoundingBoxUpdate] +} diff --git a/webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/tracings/skeleton/updating/SkeletonUpdateActions.scala b/webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/tracings/skeleton/updating/SkeletonUpdateActions.scala index f3e6f62c0a4..39b2313ae85 100644 --- a/webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/tracings/skeleton/updating/SkeletonUpdateActions.scala +++ b/webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/tracings/skeleton/updating/SkeletonUpdateActions.scala @@ -575,100 +575,35 @@ case class DeleteUserBoundingBoxSkeletonAction(boundingBoxId: Int, this.copy(actionTracingId = newTracingId) } -case class UpdateUserBoundingBoxBoundsSkeletonAction(boundingBox: NamedBoundingBox, - actionTracingId: String, - actionTimestamp: Option[Long] = None, - actionAuthorId: Option[String] = None, - info: Option[String] = None) - extends SkeletonUpdateAction { - override def applyOn(tracing: SkeletonTracing): SkeletonTracing = { - def updateUserBoundingBoxes() = - tracing.userBoundingBoxes.map { currentBoundingBox => - if (boundingBox.id == currentBoundingBox.id) - currentBoundingBox.copy(boundingBox = boundingBox.toProto.boundingBox) - else - currentBoundingBox - } - tracing.withUserBoundingBoxes(updateUserBoundingBoxes()) - } - - override def addTimestamp(timestamp: Long): UpdateAction = - this.copy(actionTimestamp = Some(timestamp)) - override def addInfo(info: Option[String]): UpdateAction = this.copy(info = info) - override def addAuthorId(authorId: Option[String]): UpdateAction = - this.copy(actionAuthorId = authorId) - override def withActionTracingId(newTracingId: String): LayerUpdateAction = - this.copy(actionTracingId = newTracingId) -} - -case class UpdateUserBoundingBoxVisibilitySkeletonAction(boundingBoxId: Option[Int], - isVisible: Boolean, - actionTracingId: String, - actionTimestamp: Option[Long] = None, - actionAuthorId: Option[String] = None, - info: Option[String] = None) - extends SkeletonUpdateAction { - override def applyOn(tracing: SkeletonTracing): SkeletonTracing = { - def updateUserBoundingBoxes() = - tracing.userBoundingBoxes.map { boundingBox => - if (boundingBoxId.forall(_ == boundingBox.id)) - boundingBox.copy(isVisible = Some(isVisible)) - else - boundingBox - } - - tracing.withUserBoundingBoxes(updateUserBoundingBoxes()) - } - - override def addTimestamp(timestamp: Long): UpdateAction = - this.copy(actionTimestamp = Some(timestamp)) - override def addInfo(info: Option[String]): UpdateAction = this.copy(info = info) - override def addAuthorId(authorId: Option[String]): UpdateAction = - this.copy(actionAuthorId = authorId) - override def withActionTracingId(newTracingId: String): LayerUpdateAction = - this.copy(actionTracingId = newTracingId) - - override def isViewOnlyChange: Boolean = true -} - -case class UpdateUserBoundingBoxNameSkeletonAction(boundingBox: NamedBoundingBox, - actionTracingId: String, - actionTimestamp: Option[Long] = None, - actionAuthorId: Option[String] = None, - info: Option[String] = None) - extends SkeletonUpdateAction { - override def applyOn(tracing: SkeletonTracing): SkeletonTracing = { - def updateUserBoundingBoxes() = - tracing.userBoundingBoxes.map { currentBoundingBox => - if (boundingBox.id == currentBoundingBox.id) - currentBoundingBox.copy(name = boundingBox.name) - else - currentBoundingBox - } - tracing.withUserBoundingBoxes(updateUserBoundingBoxes()) - } - - override def addTimestamp(timestamp: Long): UpdateAction = - this.copy(actionTimestamp = Some(timestamp)) - override def addInfo(info: Option[String]): UpdateAction = this.copy(info = info) - override def addAuthorId(authorId: Option[String]): UpdateAction = - this.copy(actionAuthorId = authorId) - override def withActionTracingId(newTracingId: String): LayerUpdateAction = - this.copy(actionTracingId = newTracingId) -} - -case class UpdateUserBoundingBoxColorSkeletonAction(boundingBox: NamedBoundingBox, - actionTracingId: String, - actionTimestamp: Option[Long] = None, - actionAuthorId: Option[String] = None, - info: Option[String] = None) +case class UpdateUserBoundingBoxSkeletonAction(boundingBoxId: Int, + updatedPropKeys: List[String], + updatedProps: NamedBoundingBoxUpdate, + actionTracingId: String, + actionTimestamp: Option[Long] = None, + actionAuthorId: Option[String] = None, + info: Option[String] = None) extends SkeletonUpdateAction { override def applyOn(tracing: SkeletonTracing): SkeletonTracing = { def updateUserBoundingBoxes() = tracing.userBoundingBoxes.map { currentBoundingBox => - if (boundingBox.id == currentBoundingBox.id) - currentBoundingBox.copy(color = boundingBox.toProto.color) - else + if (boundingBoxId == currentBoundingBox.id) { + currentBoundingBox.copy( + id = updatedProps.id.getOrElse(currentBoundingBox.id), + name = + if (updatedPropKeys.contains("name") && updatedProps.name.isDefined) updatedProps.name + else currentBoundingBox.name, + isVisible = + if (updatedPropKeys.contains("isVisible") && updatedProps.isVisible.isDefined) updatedProps.isVisible + else currentBoundingBox.isVisible, + color = + if (updatedPropKeys.contains("color") && updatedProps.color.isDefined) updatedProps.colorOptProto + else currentBoundingBox.color, + boundingBox = + if (updatedPropKeys.contains("boundingBox")) + updatedProps.boundingBoxProto.getOrElse(currentBoundingBox.boundingBox) + else currentBoundingBox.boundingBox + ) + } else currentBoundingBox } tracing.withUserBoundingBoxes(updateUserBoundingBoxes()) @@ -742,19 +677,7 @@ object DeleteUserBoundingBoxSkeletonAction { implicit val jsonFormat: OFormat[DeleteUserBoundingBoxSkeletonAction] = Json.format[DeleteUserBoundingBoxSkeletonAction] } -object UpdateUserBoundingBoxBoundsSkeletonAction { - implicit val jsonFormat: OFormat[UpdateUserBoundingBoxBoundsSkeletonAction] = - Json.format[UpdateUserBoundingBoxBoundsSkeletonAction] -} -object UpdateUserBoundingBoxVisibilitySkeletonAction { - implicit val jsonFormat: OFormat[UpdateUserBoundingBoxVisibilitySkeletonAction] = - Json.format[UpdateUserBoundingBoxVisibilitySkeletonAction] -} -object UpdateUserBoundingBoxNameSkeletonAction { - implicit val jsonFormat: OFormat[UpdateUserBoundingBoxNameSkeletonAction] = - Json.format[UpdateUserBoundingBoxNameSkeletonAction] -} -object UpdateUserBoundingBoxColorSkeletonAction { - implicit val jsonFormat: OFormat[UpdateUserBoundingBoxColorSkeletonAction] = - Json.format[UpdateUserBoundingBoxColorSkeletonAction] +object UpdateUserBoundingBoxSkeletonAction { + implicit val jsonFormat: OFormat[UpdateUserBoundingBoxSkeletonAction] = + Json.format[UpdateUserBoundingBoxSkeletonAction] } diff --git a/webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/tracings/volume/VolumeUpdateActions.scala b/webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/tracings/volume/VolumeUpdateActions.scala index f1d07ca87c4..fe51021407f 100644 --- a/webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/tracings/volume/VolumeUpdateActions.scala +++ b/webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/tracings/volume/VolumeUpdateActions.scala @@ -6,7 +6,7 @@ import com.scalableminds.webknossos.datastore.geometry.NamedBoundingBoxProto import com.scalableminds.webknossos.datastore.helpers.ProtoGeometryImplicits import com.scalableminds.webknossos.datastore.models.{AdditionalCoordinate, BucketPosition} import com.scalableminds.webknossos.tracingstore.annotation.{LayerUpdateAction, UpdateAction} -import com.scalableminds.webknossos.tracingstore.tracings.{MetadataEntry, NamedBoundingBox} +import com.scalableminds.webknossos.tracingstore.tracings.{MetadataEntry, NamedBoundingBox, NamedBoundingBoxUpdate} import play.api.libs.json._ trait VolumeUpdateActionHelper { @@ -117,11 +117,11 @@ case class UpdateUserBoundingBoxesVolumeAction(boundingBoxes: List[NamedBounding tracing.withUserBoundingBoxes(boundingBoxes.map(_.toProto)) } -case class AddUserBoundingBoxSkeletonAction(boundingBox: NamedBoundingBox, - actionTracingId: String, - actionTimestamp: Option[Long] = None, - actionAuthorId: Option[String] = None, - info: Option[String] = None) +case class AddUserBoundingBoxVolumeAction(boundingBox: NamedBoundingBox, + actionTracingId: String, + actionTimestamp: Option[Long] = None, + actionAuthorId: Option[String] = None, + info: Option[String] = None) extends ApplyableVolumeUpdateAction { override def applyOn(tracing: VolumeTracing): VolumeTracing = tracing.withUserBoundingBoxes(tracing.userBoundingBoxes :+ boundingBox.toProto) @@ -135,11 +135,11 @@ case class AddUserBoundingBoxSkeletonAction(boundingBox: NamedBoundingBox, this.copy(actionTracingId = newTracingId) } -case class DeleteUserBoundingBoxSkeletonAction(boundingBoxId: Int, - actionTracingId: String, - actionTimestamp: Option[Long] = None, - actionAuthorId: Option[String] = None, - info: Option[String] = None) +case class DeleteUserBoundingBoxVolumeAction(boundingBoxId: Int, + actionTracingId: String, + actionTimestamp: Option[Long] = None, + actionAuthorId: Option[String] = None, + info: Option[String] = None) extends ApplyableVolumeUpdateAction { override def applyOn(tracing: VolumeTracing): VolumeTracing = tracing.withUserBoundingBoxes(tracing.userBoundingBoxes.filter(_.id != boundingBoxId)) @@ -153,100 +153,35 @@ case class DeleteUserBoundingBoxSkeletonAction(boundingBoxId: Int, this.copy(actionTracingId = newTracingId) } -case class UpdateUserBoundingBoxBoundsSkeletonAction(boundingBox: NamedBoundingBox, - actionTracingId: String, - actionTimestamp: Option[Long] = None, - actionAuthorId: Option[String] = None, - info: Option[String] = None) +case class UpdateUserBoundingBoxVolumeAction(boundingBoxId: Int, + updatedPropKeys: List[String], + updatedProps: NamedBoundingBoxUpdate, + actionTracingId: String, + actionTimestamp: Option[Long] = None, + actionAuthorId: Option[String] = None, + info: Option[String] = None) extends ApplyableVolumeUpdateAction { override def applyOn(tracing: VolumeTracing): VolumeTracing = { def updateUserBoundingBoxes() = tracing.userBoundingBoxes.map { currentBoundingBox => - if (boundingBox.id == currentBoundingBox.id) - currentBoundingBox.copy(boundingBox = boundingBox.toProto.boundingBox) - else - currentBoundingBox - } - tracing.withUserBoundingBoxes(updateUserBoundingBoxes()) - } - - override def addTimestamp(timestamp: Long): UpdateAction = - this.copy(actionTimestamp = Some(timestamp)) - override def addInfo(info: Option[String]): UpdateAction = this.copy(info = info) - override def addAuthorId(authorId: Option[String]): UpdateAction = - this.copy(actionAuthorId = authorId) - override def withActionTracingId(newTracingId: String): LayerUpdateAction = - this.copy(actionTracingId = newTracingId) -} - -case class UpdateUserBoundingBoxVisibilityVolumeAction(boundingBoxId: Option[Int], - isVisible: Boolean, - actionTracingId: String, - actionTimestamp: Option[Long] = None, - actionAuthorId: Option[String] = None, - info: Option[String] = None) - extends ApplyableVolumeUpdateAction { - override def addTimestamp(timestamp: Long): VolumeUpdateAction = this.copy(actionTimestamp = Some(timestamp)) - override def addAuthorId(authorId: Option[String]): VolumeUpdateAction = - this.copy(actionAuthorId = authorId) - override def addInfo(info: Option[String]): UpdateAction = this.copy(info = info) - override def withActionTracingId(newTracingId: String): LayerUpdateAction = - this.copy(actionTracingId = newTracingId) - - override def applyOn(tracing: VolumeTracing): VolumeTracing = { - - def updateUserBoundingBoxes(): Seq[NamedBoundingBoxProto] = - tracing.userBoundingBoxes.map { boundingBox => - if (boundingBoxId.forall(_ == boundingBox.id)) - boundingBox.copy(isVisible = Some(isVisible)) - else - boundingBox - } - - tracing.withUserBoundingBoxes(updateUserBoundingBoxes()) - } - - override def isViewOnlyChange: Boolean = true -} - -case class UpdateUserBoundingBoxNameSkeletonAction(boundingBox: NamedBoundingBox, - actionTracingId: String, - actionTimestamp: Option[Long] = None, - actionAuthorId: Option[String] = None, - info: Option[String] = None) - extends ApplyableVolumeUpdateAction { - override def applyOn(tracing: VolumeTracing): VolumeTracing = { - def updateUserBoundingBoxes() = - tracing.userBoundingBoxes.map { currentBoundingBox => - if (boundingBox.id == currentBoundingBox.id) - currentBoundingBox.copy(name = boundingBox.name) - else - currentBoundingBox - } - tracing.withUserBoundingBoxes(updateUserBoundingBoxes()) - } - - override def addTimestamp(timestamp: Long): UpdateAction = - this.copy(actionTimestamp = Some(timestamp)) - override def addInfo(info: Option[String]): UpdateAction = this.copy(info = info) - override def addAuthorId(authorId: Option[String]): UpdateAction = - this.copy(actionAuthorId = authorId) - override def withActionTracingId(newTracingId: String): LayerUpdateAction = - this.copy(actionTracingId = newTracingId) -} - -case class UpdateUserBoundingBoxColorSkeletonAction(boundingBox: NamedBoundingBox, - actionTracingId: String, - actionTimestamp: Option[Long] = None, - actionAuthorId: Option[String] = None, - info: Option[String] = None) - extends ApplyableVolumeUpdateAction { - override def applyOn(tracing: VolumeTracing): VolumeTracing = { - def updateUserBoundingBoxes() = - tracing.userBoundingBoxes.map { currentBoundingBox => - if (boundingBox.id == currentBoundingBox.id) - currentBoundingBox.copy(color = boundingBox.toProto.color) - else + if (boundingBoxId == currentBoundingBox.id) { + currentBoundingBox.copy( + id = updatedProps.id.getOrElse(currentBoundingBox.id), + name = + if (updatedPropKeys.contains("name") && updatedProps.name.isDefined) updatedProps.name + else currentBoundingBox.name, + isVisible = + if (updatedPropKeys.contains("isVisible") && updatedProps.isVisible.isDefined) updatedProps.isVisible + else currentBoundingBox.isVisible, + color = + if (updatedPropKeys.contains("color") && updatedProps.color.isDefined) updatedProps.colorOptProto + else currentBoundingBox.color, + boundingBox = + if (updatedPropKeys.contains("boundingBox")) + updatedProps.boundingBoxProto.getOrElse(currentBoundingBox.boundingBox) + else currentBoundingBox.boundingBox + ) + } else currentBoundingBox } tracing.withUserBoundingBoxes(updateUserBoundingBoxes()) @@ -514,29 +449,17 @@ object UpdateUserBoundingBoxesVolumeAction { implicit val jsonFormat: OFormat[UpdateUserBoundingBoxesVolumeAction] = Json.format[UpdateUserBoundingBoxesVolumeAction] } -object AddUserBoundingBoxSkeletonAction { - implicit val jsonFormat: OFormat[AddUserBoundingBoxSkeletonAction] = - Json.format[AddUserBoundingBoxSkeletonAction] -} -object DeleteUserBoundingBoxSkeletonAction { - implicit val jsonFormat: OFormat[DeleteUserBoundingBoxSkeletonAction] = - Json.format[DeleteUserBoundingBoxSkeletonAction] -} -object UpdateUserBoundingBoxBoundsSkeletonAction { - implicit val jsonFormat: OFormat[UpdateUserBoundingBoxBoundsSkeletonAction] = - Json.format[UpdateUserBoundingBoxBoundsSkeletonAction] -} -object UpdateUserBoundingBoxVisibilityVolumeAction { - implicit val jsonFormat: OFormat[UpdateUserBoundingBoxVisibilityVolumeAction] = - Json.format[UpdateUserBoundingBoxVisibilityVolumeAction] +object AddUserBoundingBoxVolumeAction { + implicit val jsonFormat: OFormat[AddUserBoundingBoxVolumeAction] = + Json.format[AddUserBoundingBoxVolumeAction] } -object UpdateUserBoundingBoxNameSkeletonAction { - implicit val jsonFormat: OFormat[UpdateUserBoundingBoxNameSkeletonAction] = - Json.format[UpdateUserBoundingBoxNameSkeletonAction] +object DeleteUserBoundingBoxVolumeAction { + implicit val jsonFormat: OFormat[DeleteUserBoundingBoxVolumeAction] = + Json.format[DeleteUserBoundingBoxVolumeAction] } -object UpdateUserBoundingBoxColorSkeletonAction { - implicit val jsonFormat: OFormat[UpdateUserBoundingBoxColorSkeletonAction] = - Json.format[UpdateUserBoundingBoxColorSkeletonAction] +object UpdateUserBoundingBoxVolumeAction { + implicit val jsonFormat: OFormat[UpdateUserBoundingBoxVolumeAction] = + Json.format[UpdateUserBoundingBoxVolumeAction] } object RemoveFallbackLayerVolumeAction { implicit val jsonFormat: OFormat[RemoveFallbackLayerVolumeAction] = Json.format[RemoveFallbackLayerVolumeAction] From 55fdf45f9a1b07754ad0f99ab4a0dadf62e44660 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20B=C3=BC=C3=9Femeyer?= <39529669+MichaelBuessemeyer@users.noreply.github.com> Date: Tue, 8 Apr 2025 18:45:47 +0200 Subject: [PATCH 08/41] Don't make frontend return a single elemnt list of a user bbox for new add update actions --- .../oxalis/model/reducers/reducer_helpers.ts | 10 ++++------ .../javascripts/oxalis/model/sagas/update_actions.ts | 8 ++++---- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/frontend/javascripts/oxalis/model/reducers/reducer_helpers.ts b/frontend/javascripts/oxalis/model/reducers/reducer_helpers.ts index 7fd0a5c090f..27c05373c8e 100644 --- a/frontend/javascripts/oxalis/model/reducers/reducer_helpers.ts +++ b/frontend/javascripts/oxalis/model/reducers/reducer_helpers.ts @@ -57,12 +57,10 @@ export function convertUserBoundingBoxesFromServerToFrontend( }); } export function convertUserBoundingBoxesFromFrontendToServer( - boundingBoxes: Array, -): Array { - return boundingBoxes.map((bb) => { - const { boundingBox, ...rest } = bb; - return { ...rest, boundingBox: Utils.computeBoundingBoxObjectFromBoundingBox(boundingBox) }; - }); + boundingBox: UserBoundingBox, +): UserBoundingBoxToServer { + const { boundingBox: bb, ...rest } = boundingBox; + return { ...rest, boundingBox: Utils.computeBoundingBoxObjectFromBoundingBox(bb) }; } export function convertFrontendBoundingBoxToServer( boundingBox: BoundingBoxType, diff --git a/frontend/javascripts/oxalis/model/sagas/update_actions.ts b/frontend/javascripts/oxalis/model/sagas/update_actions.ts index bcc8f9cfd07..6982b942149 100644 --- a/frontend/javascripts/oxalis/model/sagas/update_actions.ts +++ b/frontend/javascripts/oxalis/model/sagas/update_actions.ts @@ -409,7 +409,7 @@ export function addUserBoundingBoxInSkeletonTracingAction( return { name: "addUserBoundingBoxSkeletonAction", value: { - boundingBox: convertUserBoundingBoxesFromFrontendToServer([boundingBox]), + boundingBox: convertUserBoundingBoxesFromFrontendToServer(boundingBox), actionTracingId, }, } as const; @@ -422,7 +422,7 @@ export function addUserBoundingBoxInVolumeTracingAction( return { name: "addUserBoundingBoxVolumeAction", value: { - boundingBox: convertUserBoundingBoxesFromFrontendToServer([boundingBox]), + boundingBox: convertUserBoundingBoxesFromFrontendToServer(boundingBox), actionTracingId, }, } as const; @@ -496,7 +496,7 @@ export function updateUserBoundingBoxesInSkeletonTracing( name: "updateUserBoundingBoxesInSkeletonTracing", value: { actionTracingId, - boundingBoxes: convertUserBoundingBoxesFromFrontendToServer(userBoundingBoxes), + boundingBoxes: userBoundingBoxes.map(convertUserBoundingBoxesFromFrontendToServer), }, } as const; } @@ -508,7 +508,7 @@ export function updateUserBoundingBoxesInVolumeTracing( name: "updateUserBoundingBoxesInVolumeTracing", value: { actionTracingId, - boundingBoxes: convertUserBoundingBoxesFromFrontendToServer(userBoundingBoxes), + boundingBoxes: userBoundingBoxes.map(convertUserBoundingBoxesFromFrontendToServer), }, } as const; } From d8dcfdd4c4c788f39775440b34d9c4a9bdd26ad3 Mon Sep 17 00:00:00 2001 From: Charlie Meister Date: Wed, 9 Apr 2025 14:48:45 +0200 Subject: [PATCH 09/41] [frontend] dont send empty update objects --- .../javascripts/oxalis/model/sagas/skeletontracing_saga.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/frontend/javascripts/oxalis/model/sagas/skeletontracing_saga.ts b/frontend/javascripts/oxalis/model/sagas/skeletontracing_saga.ts index 9e6c76059fe..8438c25a74d 100644 --- a/frontend/javascripts/oxalis/model/sagas/skeletontracing_saga.ts +++ b/frontend/javascripts/oxalis/model/sagas/skeletontracing_saga.ts @@ -622,7 +622,7 @@ export function* diffBoundingBoxes( const { onlyA: deletedBBoxIds, onlyB: addedBBoxIds, - both: changedBBoxIds, + both: maybeChangedBBoxIds, } = Utils.diffArrays( _.map(prevBoundingBoxes, (bbox) => bbox.id), _.map(currentBoundingBoxes, (bbox) => bbox.id), @@ -652,7 +652,7 @@ export function* diffBoundingBoxes( Toast.error(getErrorMessage(id)); } } - for (const id of changedBBoxIds) { + for (const id of maybeChangedBBoxIds) { const currentBbox = currentBoundingBoxes.find((bbox) => bbox.id === id); const prevBbox = prevBoundingBoxes.find((bbox) => bbox.id === id); if (currentBbox == null || prevBbox == null) { @@ -660,6 +660,7 @@ export function* diffBoundingBoxes( continue; } const diffBBox = Utils.diffObjects(currentBbox, prevBbox); + if (_.isEmpty(diffBBox)) continue; //TODO_C remove console.log("diffBBox", diffBBox); yield updateBBoxAction(currentBbox.id, diffBBox, tracingId); From b8d0a71e9a7f63fb0499d39d9a4f6774ba6a717b Mon Sep 17 00:00:00 2001 From: Charlie Meister Date: Wed, 9 Apr 2025 16:39:11 +0200 Subject: [PATCH 10/41] [frontend] WIP: send right bbox object to server --- .../oxalis/model/reducers/reducer_helpers.ts | 1 + .../model/sagas/skeletontracing_saga.ts | 7 +++++-- .../oxalis/model/sagas/update_actions.ts | 21 +++++++++++++++---- .../test/sagas/compact_toggle_actions.spec.ts | 1 - .../test/sagas/skeletontracing_saga.spec.ts | 1 - 5 files changed, 23 insertions(+), 8 deletions(-) diff --git a/frontend/javascripts/oxalis/model/reducers/reducer_helpers.ts b/frontend/javascripts/oxalis/model/reducers/reducer_helpers.ts index 27c05373c8e..c649282138d 100644 --- a/frontend/javascripts/oxalis/model/reducers/reducer_helpers.ts +++ b/frontend/javascripts/oxalis/model/reducers/reducer_helpers.ts @@ -56,6 +56,7 @@ export function convertUserBoundingBoxesFromServerToFrontend( }; }); } + export function convertUserBoundingBoxesFromFrontendToServer( boundingBox: UserBoundingBox, ): UserBoundingBoxToServer { diff --git a/frontend/javascripts/oxalis/model/sagas/skeletontracing_saga.ts b/frontend/javascripts/oxalis/model/sagas/skeletontracing_saga.ts index 8438c25a74d..a60372e0caa 100644 --- a/frontend/javascripts/oxalis/model/sagas/skeletontracing_saga.ts +++ b/frontend/javascripts/oxalis/model/sagas/skeletontracing_saga.ts @@ -661,8 +661,11 @@ export function* diffBoundingBoxes( } const diffBBox = Utils.diffObjects(currentBbox, prevBbox); if (_.isEmpty(diffBBox)) continue; - //TODO_C remove - console.log("diffBBox", diffBBox); + const changedKeys = Object.keys(diffBBox); + if (changedKeys.includes("boundingBox")) { + diffBBox.boundingBox.min = currentBbox.boundingBox.min; + diffBBox.boundingBox.max = currentBbox.boundingBox.max; + } yield updateBBoxAction(currentBbox.id, diffBBox, tracingId); } } diff --git a/frontend/javascripts/oxalis/model/sagas/update_actions.ts b/frontend/javascripts/oxalis/model/sagas/update_actions.ts index 6982b942149..6e3a3fb2857 100644 --- a/frontend/javascripts/oxalis/model/sagas/update_actions.ts +++ b/frontend/javascripts/oxalis/model/sagas/update_actions.ts @@ -1,4 +1,5 @@ -import type { Vector3 } from "oxalis/constants"; +import * as Utils from "libs/utils"; +import { EMPTY_OBJECT, type Vector3 } from "oxalis/constants"; import type { SendBucketInfo } from "oxalis/model/bucket_data_handling/wkstore_adapter"; import { convertUserBoundingBoxesFromFrontendToServer } from "oxalis/model/reducers/reducer_helpers"; import type { @@ -459,13 +460,19 @@ export function updateUserBoundingBoxInSkeletonTracingAction( updatedProps: Partial, actionTracingId: string, ) { + let serverBBox = EMPTY_OBJECT; + const { boundingBox, ...rest } = updatedProps; const updatedPropKeys = Object.keys(updatedProps); + if (updatedProps.boundingBox?.min != null && updatedProps.boundingBox?.max != null) { + const bb = updatedProps.boundingBox; + serverBBox = { boundingBox: Utils.computeBoundingBoxObjectFromBoundingBox(bb) }; + } return { - name: "updateUserBoundingBoxSkeletonAction", + name: "updateUserBoundingBoxVolumeAction", value: { boundingBoxId, actionTracingId, - updatedProps, + updatedProps: { ...serverBBox, ...rest }, updatedPropKeys, }, } as const; @@ -476,13 +483,19 @@ export function updateUserBoundingBoxInVolumeTracingAction( updatedProps: Partial, actionTracingId: string, ) { + let serverBBox = EMPTY_OBJECT; + const { boundingBox, ...rest } = updatedProps; const updatedPropKeys = Object.keys(updatedProps); + if (updatedProps.boundingBox?.min != null && updatedProps.boundingBox?.max != null) { + const bb = updatedProps.boundingBox; + serverBBox = { boundingBox: Utils.computeBoundingBoxObjectFromBoundingBox(bb) }; + } return { name: "updateUserBoundingBoxVolumeAction", value: { boundingBoxId, actionTracingId, - updatedProps, + updatedProps: { ...serverBBox, ...rest }, updatedPropKeys, }, } as const; diff --git a/frontend/javascripts/test/sagas/compact_toggle_actions.spec.ts b/frontend/javascripts/test/sagas/compact_toggle_actions.spec.ts index a6d9424cf2c..c2ec4b140d4 100644 --- a/frontend/javascripts/test/sagas/compact_toggle_actions.spec.ts +++ b/frontend/javascripts/test/sagas/compact_toggle_actions.spec.ts @@ -107,7 +107,6 @@ function testDiffing(prevState: OxalisState, nextState: OxalisState) { enforceSkeletonTracing(nextState.annotation), flycamMock, flycamMock, - nextState.activeUser?.id || null, ), ), ), diff --git a/frontend/javascripts/test/sagas/skeletontracing_saga.spec.ts b/frontend/javascripts/test/sagas/skeletontracing_saga.spec.ts index 08ff771dc3b..35fb080c355 100644 --- a/frontend/javascripts/test/sagas/skeletontracing_saga.spec.ts +++ b/frontend/javascripts/test/sagas/skeletontracing_saga.spec.ts @@ -63,7 +63,6 @@ function testDiffing( enforceSkeletonTracing(nextAnnotation), prevFlycam, flycam, - null, ), ), ); From 21e13e2779b28340784c7f52b972ede91f53cccb Mon Sep 17 00:00:00 2001 From: Charlie Meister Date: Wed, 9 Apr 2025 18:19:01 +0200 Subject: [PATCH 11/41] [frontend] clean up code --- .../oxalis/model/sagas/update_actions.ts | 45 ++++++++++--------- 1 file changed, 24 insertions(+), 21 deletions(-) diff --git a/frontend/javascripts/oxalis/model/sagas/update_actions.ts b/frontend/javascripts/oxalis/model/sagas/update_actions.ts index 6e3a3fb2857..e6a8b8c0b17 100644 --- a/frontend/javascripts/oxalis/model/sagas/update_actions.ts +++ b/frontend/javascripts/oxalis/model/sagas/update_actions.ts @@ -455,7 +455,8 @@ export function deleteUserBoundingBoxInVolumeTracingAction( } as const; } -export function updateUserBoundingBoxInSkeletonTracingAction( +function getUpdateUserBoundingBoxAction( + actionName: "updateUserBoundingBoxVolumeAction" | "updateUserBoundingBoxSkeletonAction", boundingBoxId: number, updatedProps: Partial, actionTracingId: string, @@ -463,12 +464,11 @@ export function updateUserBoundingBoxInSkeletonTracingAction( let serverBBox = EMPTY_OBJECT; const { boundingBox, ...rest } = updatedProps; const updatedPropKeys = Object.keys(updatedProps); - if (updatedProps.boundingBox?.min != null && updatedProps.boundingBox?.max != null) { - const bb = updatedProps.boundingBox; - serverBBox = { boundingBox: Utils.computeBoundingBoxObjectFromBoundingBox(bb) }; + if (boundingBox != null) { + serverBBox = { boundingBox: Utils.computeBoundingBoxObjectFromBoundingBox(boundingBox) }; } return { - name: "updateUserBoundingBoxVolumeAction", + name: actionName, value: { boundingBoxId, actionTracingId, @@ -483,22 +483,25 @@ export function updateUserBoundingBoxInVolumeTracingAction( updatedProps: Partial, actionTracingId: string, ) { - let serverBBox = EMPTY_OBJECT; - const { boundingBox, ...rest } = updatedProps; - const updatedPropKeys = Object.keys(updatedProps); - if (updatedProps.boundingBox?.min != null && updatedProps.boundingBox?.max != null) { - const bb = updatedProps.boundingBox; - serverBBox = { boundingBox: Utils.computeBoundingBoxObjectFromBoundingBox(bb) }; - } - return { - name: "updateUserBoundingBoxVolumeAction", - value: { - boundingBoxId, - actionTracingId, - updatedProps: { ...serverBBox, ...rest }, - updatedPropKeys, - }, - } as const; + return getUpdateUserBoundingBoxAction( + "updateUserBoundingBoxVolumeAction", + boundingBoxId, + updatedProps, + actionTracingId, + ); +} + +export function updateUserBoundingBoxInSkeletonTracingAction( + boundingBoxId: number, + updatedProps: Partial, + actionTracingId: string, +) { + return getUpdateUserBoundingBoxAction( + "updateUserBoundingBoxSkeletonAction", + boundingBoxId, + updatedProps, + actionTracingId, + ); } export function updateUserBoundingBoxesInSkeletonTracing( From 374fe71fb8a4011c56c8b3b5c063a395061b75b5 Mon Sep 17 00:00:00 2001 From: Charlie Meister Date: Thu, 10 Apr 2025 11:55:49 +0200 Subject: [PATCH 12/41] [frontend] remove unneccesary update actions sent to backend --- .../helpers/compaction/compact_save_queue.ts | 43 +++++++++++++++++-- 1 file changed, 39 insertions(+), 4 deletions(-) diff --git a/frontend/javascripts/oxalis/model/helpers/compaction/compact_save_queue.ts b/frontend/javascripts/oxalis/model/helpers/compaction/compact_save_queue.ts index 7504a03eded..493c33f53db 100644 --- a/frontend/javascripts/oxalis/model/helpers/compaction/compact_save_queue.ts +++ b/frontend/javascripts/oxalis/model/helpers/compaction/compact_save_queue.ts @@ -70,6 +70,39 @@ function removeSubsequentUpdateNodeActions(updateActionsBatches: Array) { + const obsoleteUpdateActions = []; + + // Actions are obsolete, if they are for the same bounding box and for the same prop. + // The given action is always compared to the next next one, as usually an + // updateUserBoundingBoxSkeletonAction and updateUserBoundingBoxVolumeAction + // is sent at the same time. + for (let i = 0; i < updateActionsBatches.length - 2; i++) { + const actions1 = updateActionsBatches[i].actions; + const actions2 = updateActionsBatches[i + 2].actions; + + if ( + actions1.length === 1 && + actions2.length === 1 && + (actions1[0].name === "updateUserBoundingBoxSkeletonAction" || + actions1[0].name === "updateUserBoundingBoxVolumeAction") && + actions1[0].name === actions2[0].name && + actions1[0].value.boundingBoxId === actions2[0].value.boundingBoxId && + _.isEqual(actions1[0].value.updatedPropKeys, actions2[0].value.updatedPropKeys) + ) { + obsoleteUpdateActions.push(updateActionsBatches[i]); + console.log( + actions1[0].value.boundingBoxId, + actions1[0].value.updatedProps, + actions2[0].value.boundingBoxId, + actions2[0].value.updatedProps, + ); // TODO_c remove + } + } + + return _.without(updateActionsBatches, ...obsoleteUpdateActions); +} + function removeSubsequentUpdateSegmentActions(updateActionsBatches: Array) { const obsoleteUpdateActions = []; @@ -100,10 +133,12 @@ export default function compactSaveQueue( (updateActionsBatch) => updateActionsBatch.actions.length > 0, ); - return removeSubsequentUpdateSegmentActions( - removeSubsequentUpdateTreeActions( - removeSubsequentUpdateNodeActions( - removeAllButLastUpdateTdCameraAction(removeAllButLastUpdateTracingAction(result)), + return removeSubsequentUpdateBBoxActions( + removeSubsequentUpdateSegmentActions( + removeSubsequentUpdateTreeActions( + removeSubsequentUpdateNodeActions( + removeAllButLastUpdateTdCameraAction(removeAllButLastUpdateTracingAction(result)), + ), ), ), ); From c6a9b0fae80b17d276aa5a59f8871cdecc0eb887 Mon Sep 17 00:00:00 2001 From: Charlie Meister Date: Thu, 10 Apr 2025 13:21:27 +0200 Subject: [PATCH 13/41] [frontend] remove old actions --- .../oxalis/model/sagas/update_actions.ts | 32 ------------------- .../javascripts/oxalis/view/version_entry.tsx | 9 ------ 2 files changed, 41 deletions(-) diff --git a/frontend/javascripts/oxalis/model/sagas/update_actions.ts b/frontend/javascripts/oxalis/model/sagas/update_actions.ts index e6a8b8c0b17..5443c83207c 100644 --- a/frontend/javascripts/oxalis/model/sagas/update_actions.ts +++ b/frontend/javascripts/oxalis/model/sagas/update_actions.ts @@ -57,12 +57,6 @@ export type UpdateUserBoundingBoxInSkeletonTracingAction = ReturnType< export type UpdateUserBoundingBoxInVolumeTracingAction = ReturnType< typeof updateUserBoundingBoxInVolumeTracingAction >; -type UpdateUserBoundingBoxesInSkeletonTracingUpdateAction = ReturnType< - typeof updateUserBoundingBoxesInSkeletonTracing ->; -type UpdateUserBoundingBoxesInVolumeTracingUpdateAction = ReturnType< - typeof updateUserBoundingBoxesInVolumeTracing ->; export type UpdateBucketUpdateAction = ReturnType; export type UpdateSegmentGroupsUpdateAction = ReturnType; @@ -108,8 +102,6 @@ export type UpdateActionWithoutIsolationRequirement = | DeleteUserBoundingBoxInVolumeTracingAction | UpdateUserBoundingBoxInSkeletonTracingAction | UpdateUserBoundingBoxInVolumeTracingAction - | UpdateUserBoundingBoxesInSkeletonTracingUpdateAction - | UpdateUserBoundingBoxesInVolumeTracingUpdateAction | CreateSegmentUpdateAction | UpdateSegmentUpdateAction | DeleteSegmentUpdateAction @@ -504,30 +496,6 @@ export function updateUserBoundingBoxInSkeletonTracingAction( ); } -export function updateUserBoundingBoxesInSkeletonTracing( - userBoundingBoxes: Array, - actionTracingId: string, -) { - return { - name: "updateUserBoundingBoxesInSkeletonTracing", - value: { - actionTracingId, - boundingBoxes: userBoundingBoxes.map(convertUserBoundingBoxesFromFrontendToServer), - }, - } as const; -} -export function updateUserBoundingBoxesInVolumeTracing( - userBoundingBoxes: Array, - actionTracingId: string, -) { - return { - name: "updateUserBoundingBoxesInVolumeTracing", - value: { - actionTracingId, - boundingBoxes: userBoundingBoxes.map(convertUserBoundingBoxesFromFrontendToServer), - }, - } as const; -} export function createSegmentVolumeAction( id: number, anchorPosition: Vector3 | null | undefined, diff --git a/frontend/javascripts/oxalis/view/version_entry.tsx b/frontend/javascripts/oxalis/view/version_entry.tsx index 072f9e52f13..4d0f2ac8521 100644 --- a/frontend/javascripts/oxalis/view/version_entry.tsx +++ b/frontend/javascripts/oxalis/view/version_entry.tsx @@ -2,7 +2,6 @@ import { ArrowsAltOutlined, BackwardOutlined, CodeSandboxOutlined, - CodepenOutlined, DeleteOutlined, EditOutlined, EyeOutlined, @@ -76,14 +75,6 @@ const descriptionFns: Record< description: "Created the annotation.", icon: , }), - updateUserBoundingBoxesInSkeletonTracing: (): Description => ({ - description: "Updated a bounding box.", - icon: , - }), - updateUserBoundingBoxesInVolumeTracing: (): Description => ({ - description: "Updated a bounding box.", - icon: , - }), addUserBoundingBoxSkeletonAction: (): Description => ({ description: "Added a bounding box.", icon: , From 0ae286c5095d87bac7a3dc1411da409961e3ba80 Mon Sep 17 00:00:00 2001 From: Charlie Meister Date: Wed, 23 Apr 2025 14:19:07 +0200 Subject: [PATCH 14/41] [frontend] use updateUserBoundingBoxVisibilityActions --- .../model/sagas/skeletontracing_saga.ts | 10 +++++ .../oxalis/model/sagas/update_actions.ts | 43 ++++++++++++++++++- .../javascripts/oxalis/view/version_entry.tsx | 8 ++++ 3 files changed, 59 insertions(+), 2 deletions(-) diff --git a/frontend/javascripts/oxalis/model/sagas/skeletontracing_saga.ts b/frontend/javascripts/oxalis/model/sagas/skeletontracing_saga.ts index a60372e0caa..2e96e23d342 100644 --- a/frontend/javascripts/oxalis/model/sagas/skeletontracing_saga.ts +++ b/frontend/javascripts/oxalis/model/sagas/skeletontracing_saga.ts @@ -66,6 +66,8 @@ import { updateTreeVisibility, updateUserBoundingBoxInSkeletonTracingAction, updateUserBoundingBoxInVolumeTracingAction, + updateUserBoundingBoxVisibilityInSkeletonTracingAction, + updateUserBoundingBoxVisibilityInVolumeTracingAction, } from "oxalis/model/sagas/update_actions"; import { api } from "oxalis/singletons"; import type { @@ -639,6 +641,10 @@ export function* diffBoundingBoxes( tracingType === AnnotationLayerEnum.Skeleton ? updateUserBoundingBoxInSkeletonTracingAction : updateUserBoundingBoxInVolumeTracingAction; + const updateBBoxVisibilityAction = + tracingType === AnnotationLayerEnum.Skeleton + ? updateUserBoundingBoxVisibilityInSkeletonTracingAction + : updateUserBoundingBoxVisibilityInVolumeTracingAction; const getErrorMessage = (id: number) => `User bounding box with id ${id} not found in ${tracingType} tracing.`; for (const id of deletedBBoxIds) { @@ -662,6 +668,10 @@ export function* diffBoundingBoxes( const diffBBox = Utils.diffObjects(currentBbox, prevBbox); if (_.isEmpty(diffBBox)) continue; const changedKeys = Object.keys(diffBBox); + if (changedKeys.includes("isVisible")) { + yield updateBBoxVisibilityAction(currentBbox.id, currentBbox.isVisible, tracingId); + continue; + } if (changedKeys.includes("boundingBox")) { diffBBox.boundingBox.min = currentBbox.boundingBox.min; diffBBox.boundingBox.max = currentBbox.boundingBox.max; diff --git a/frontend/javascripts/oxalis/model/sagas/update_actions.ts b/frontend/javascripts/oxalis/model/sagas/update_actions.ts index 5443c83207c..35eaccfaf2c 100644 --- a/frontend/javascripts/oxalis/model/sagas/update_actions.ts +++ b/frontend/javascripts/oxalis/model/sagas/update_actions.ts @@ -21,6 +21,8 @@ export type NodeWithTreeId = { treeId: number; } & Node; +type PartialBoundingBoxWithoutVisibility = Partial>; + export type UpdateTreeUpdateAction = ReturnType | ReturnType; export type DeleteTreeUpdateAction = ReturnType; export type MoveTreeComponentUpdateAction = ReturnType; @@ -57,6 +59,12 @@ export type UpdateUserBoundingBoxInSkeletonTracingAction = ReturnType< export type UpdateUserBoundingBoxInVolumeTracingAction = ReturnType< typeof updateUserBoundingBoxInVolumeTracingAction >; +export type UpdateUserBoundingBoxVisibilityInSkeletonTracingAction = ReturnType< + typeof updateUserBoundingBoxVisibilityInSkeletonTracingAction +>; +export type UpdateUserBoundingBoxVisibilityInVolumeTracingAction = ReturnType< + typeof updateUserBoundingBoxVisibilityInVolumeTracingAction +>; export type UpdateBucketUpdateAction = ReturnType; export type UpdateSegmentGroupsUpdateAction = ReturnType; @@ -102,6 +110,8 @@ export type UpdateActionWithoutIsolationRequirement = | DeleteUserBoundingBoxInVolumeTracingAction | UpdateUserBoundingBoxInSkeletonTracingAction | UpdateUserBoundingBoxInVolumeTracingAction + | UpdateUserBoundingBoxVisibilityInSkeletonTracingAction + | UpdateUserBoundingBoxVisibilityInVolumeTracingAction | CreateSegmentUpdateAction | UpdateSegmentUpdateAction | DeleteSegmentUpdateAction @@ -450,7 +460,7 @@ export function deleteUserBoundingBoxInVolumeTracingAction( function getUpdateUserBoundingBoxAction( actionName: "updateUserBoundingBoxVolumeAction" | "updateUserBoundingBoxSkeletonAction", boundingBoxId: number, - updatedProps: Partial, + updatedProps: PartialBoundingBoxWithoutVisibility, actionTracingId: string, ) { let serverBBox = EMPTY_OBJECT; @@ -472,7 +482,7 @@ function getUpdateUserBoundingBoxAction( export function updateUserBoundingBoxInVolumeTracingAction( boundingBoxId: number, - updatedProps: Partial, + updatedProps: PartialBoundingBoxWithoutVisibility, actionTracingId: string, ) { return getUpdateUserBoundingBoxAction( @@ -496,6 +506,35 @@ export function updateUserBoundingBoxInSkeletonTracingAction( ); } +export function updateUserBoundingBoxVisibilityInSkeletonTracingAction( + boundingBoxId: number, + isVisible: boolean, + actionTracingId: string, +) { + return { + name: "updateUserBoundingBoxVisibilitySkeletonAction", + value: { + boundingBoxId, + actionTracingId, + isVisible, + }, + } as const; +} +export function updateUserBoundingBoxVisibilityInVolumeTracingAction( + boundingBoxId: number, + isVisible: boolean, + actionTracingId: string, +) { + return { + name: "updateUserBoundingBoxVisibilityVolumeAction", + value: { + boundingBoxId, + actionTracingId, + isVisible, + }, + } as const; +} + export function createSegmentVolumeAction( id: number, anchorPosition: Vector3 | null | undefined, diff --git a/frontend/javascripts/oxalis/view/version_entry.tsx b/frontend/javascripts/oxalis/view/version_entry.tsx index 4d0f2ac8521..8ee34bdf72e 100644 --- a/frontend/javascripts/oxalis/view/version_entry.tsx +++ b/frontend/javascripts/oxalis/view/version_entry.tsx @@ -99,6 +99,14 @@ const descriptionFns: Record< description: "Updated a bounding box.", icon: , }), + updateUserBoundingBoxVisibilitySkeletonAction: (): Description => ({ + description: "Toggled the visibility of a bounding box.", + icon: , + }), + updateUserBoundingBoxVisibilityVolumeAction: (): Description => ({ + description: "Toggled the visibility of a bounding box.", + icon: , + }), removeFallbackLayer: (): Description => ({ description: "Removed the segmentation fallback layer.", icon: , From 675ae3571f7c7570c24f4bb3322c1670690f1909 Mon Sep 17 00:00:00 2001 From: Charlie Meister Date: Wed, 23 Apr 2025 14:28:39 +0200 Subject: [PATCH 15/41] [frontend] adjust new update actions name fields --- .../helpers/compaction/compact_save_queue.ts | 4 ++-- .../oxalis/model/sagas/update_actions.ts | 18 ++++++++-------- .../javascripts/oxalis/view/version_entry.tsx | 21 +++++++++---------- 3 files changed, 21 insertions(+), 22 deletions(-) diff --git a/frontend/javascripts/oxalis/model/helpers/compaction/compact_save_queue.ts b/frontend/javascripts/oxalis/model/helpers/compaction/compact_save_queue.ts index 493c33f53db..17552bf6f7f 100644 --- a/frontend/javascripts/oxalis/model/helpers/compaction/compact_save_queue.ts +++ b/frontend/javascripts/oxalis/model/helpers/compaction/compact_save_queue.ts @@ -84,8 +84,8 @@ function removeSubsequentUpdateBBoxActions(updateActionsBatches: Array, }), - addUserBoundingBoxSkeletonAction: (): Description => ({ + addUserBoundingBoxSkeleton: (): Description => ({ description: "Added a bounding box.", icon: , }), - addUserBoundingBoxVolumeAction: (): Description => ({ + addUserBoundingBoxVolume: (): Description => ({ description: "Added a bounding box.", icon: , }), - deleteUserBoundingBoxSkeletonAction: (): Description => ({ + deleteUserBoundingBoxSkeleton: (): Description => ({ description: "Deleted a bounding box.", icon: , }), - deleteUserBoundingBoxVolumeAction: (): Description => ({ + deleteUserBoundingBoxVolume: (): Description => ({ description: "Deleted a bounding box.", icon: , }), - updateUserBoundingBoxSkeletonAction: (): Description => ({ + updateUserBoundingBoxSkeleton: (): Description => ({ description: "Updated a bounding box.", icon: , }), - updateUserBoundingBoxVolumeAction: (): Description => ({ + updateUserBoundingBoxVolume: (): Description => ({ description: "Updated a bounding box.", icon: , }), - updateUserBoundingBoxVisibilitySkeletonAction: (): Description => ({ + updateUserBoundingBoxVisibilitySkeleton: (): Description => ({ description: "Toggled the visibility of a bounding box.", icon: , }), - updateUserBoundingBoxVisibilityVolumeAction: (): Description => ({ + updateUserBoundingBoxVisibilityVolume: (): Description => ({ description: "Toggled the visibility of a bounding box.", icon: , }), @@ -215,9 +215,8 @@ const descriptionFns: Record< icon: Hide Tree Edges Icon, }), updateTreeGroupVisibility: (action: UpdateTreeGroupVisibilityUpdateAction): Description => ({ - description: `Updated the visibility of the group with id ${ - action.value.treeGroupId != null ? action.value.treeGroupId : MISSING_GROUP_ID - }.`, + description: `Updated the visibility of the group with id ${action.value.treeGroupId != null ? action.value.treeGroupId : MISSING_GROUP_ID + }.`, icon: , }), createEdge: (action: CreateEdgeUpdateAction): Description => ({ From 4d33b5c609fa65a76adef14e05ce2fe5bb82ac61 Mon Sep 17 00:00:00 2001 From: Charlie Meister Date: Fri, 25 Apr 2025 18:07:45 +0200 Subject: [PATCH 16/41] [frontend] rename actions to ...In[Volume|Skeleton]TracingAction --- .../helpers/compaction/compact_save_queue.ts | 6 ++--- .../oxalis/model/sagas/update_actions.ts | 22 +++++++++---------- .../javascripts/oxalis/view/version_entry.tsx | 21 +++++++++--------- 3 files changed, 25 insertions(+), 24 deletions(-) diff --git a/frontend/javascripts/oxalis/model/helpers/compaction/compact_save_queue.ts b/frontend/javascripts/oxalis/model/helpers/compaction/compact_save_queue.ts index 17552bf6f7f..dd7b2d30a1b 100644 --- a/frontend/javascripts/oxalis/model/helpers/compaction/compact_save_queue.ts +++ b/frontend/javascripts/oxalis/model/helpers/compaction/compact_save_queue.ts @@ -75,7 +75,7 @@ function removeSubsequentUpdateBBoxActions(updateActionsBatches: Array; export type DeleteSegmentUpdateAction = ReturnType; export type DeleteSegmentDataUpdateAction = ReturnType; -export type AddUserBoundingBoxSkeletonAction = ReturnType< +export type AddUserBoundingBoxInSkeletonTracingAction = ReturnType< typeof addUserBoundingBoxInSkeletonTracingAction >; export type AddUserBoundingBoxInVolumeTracingAction = ReturnType< @@ -104,7 +104,7 @@ export type UpdateActionWithoutIsolationRequirement = | DeleteEdgeUpdateAction | UpdateSkeletonTracingUpdateAction | UpdateVolumeTracingUpdateAction - | AddUserBoundingBoxSkeletonAction + | AddUserBoundingBoxInSkeletonTracingAction | AddUserBoundingBoxInVolumeTracingAction | DeleteUserBoundingBoxInSkeletonTracingAction | DeleteUserBoundingBoxInVolumeTracingAction @@ -410,7 +410,7 @@ export function addUserBoundingBoxInSkeletonTracingAction( actionTracingId: string, ) { return { - name: "addUserBoundingBoxSkeleton", + name: "addUserBoundingBoxInSkeletonTracing", value: { boundingBox: convertUserBoundingBoxesFromFrontendToServer(boundingBox), actionTracingId, @@ -423,7 +423,7 @@ export function addUserBoundingBoxInVolumeTracingAction( actionTracingId: string, ) { return { - name: "addUserBoundingBoxVolume", + name: "addUserBoundingBoxInVolumeTracing", value: { boundingBox: convertUserBoundingBoxesFromFrontendToServer(boundingBox), actionTracingId, @@ -436,7 +436,7 @@ export function deleteUserBoundingBoxInSkeletonTracingAction( actionTracingId: string, ) { return { - name: "deleteUserBoundingBoxSkeleton", + name: "deleteUserBoundingBoxInSkeletonTracing", value: { boundingBoxId, actionTracingId, @@ -449,7 +449,7 @@ export function deleteUserBoundingBoxInVolumeTracingAction( actionTracingId: string, ) { return { - name: "deleteUserBoundingBoxVolume", + name: "deleteUserBoundingBoxInVolumeTracing", value: { boundingBoxId, actionTracingId, @@ -458,7 +458,7 @@ export function deleteUserBoundingBoxInVolumeTracingAction( } function getUpdateUserBoundingBoxAction( - actionName: "updateUserBoundingBoxVolume" | "updateUserBoundingBoxSkeleton", + actionName: "updateUserBoundingBoxInVolumeTracing" | "updateUserBoundingBoxInSkeletonTracing", boundingBoxId: number, updatedProps: PartialBoundingBoxWithoutVisibility, actionTracingId: string, @@ -486,7 +486,7 @@ export function updateUserBoundingBoxInVolumeTracingAction( actionTracingId: string, ) { return getUpdateUserBoundingBoxAction( - "updateUserBoundingBoxVolume", + "updateUserBoundingBoxInVolumeTracing", boundingBoxId, updatedProps, actionTracingId, @@ -499,7 +499,7 @@ export function updateUserBoundingBoxInSkeletonTracingAction( actionTracingId: string, ) { return getUpdateUserBoundingBoxAction( - "updateUserBoundingBoxSkeleton", + "updateUserBoundingBoxInSkeletonTracing", boundingBoxId, updatedProps, actionTracingId, @@ -512,7 +512,7 @@ export function updateUserBoundingBoxVisibilityInSkeletonTracingAction( actionTracingId: string, ) { return { - name: "updateUserBoundingBoxVisibilitySkeleton", + name: "updateUserBoundingBoxVisibilityInSkeletonTracing", value: { boundingBoxId, actionTracingId, @@ -526,7 +526,7 @@ export function updateUserBoundingBoxVisibilityInVolumeTracingAction( actionTracingId: string, ) { return { - name: "updateUserBoundingBoxVisibilityVolume", + name: "updateUserBoundingBoxVisibilityInVolumeTracing", value: { boundingBoxId, actionTracingId, diff --git a/frontend/javascripts/oxalis/view/version_entry.tsx b/frontend/javascripts/oxalis/view/version_entry.tsx index 5b163b7ee2c..3a2f167a2c4 100644 --- a/frontend/javascripts/oxalis/view/version_entry.tsx +++ b/frontend/javascripts/oxalis/view/version_entry.tsx @@ -75,35 +75,35 @@ const descriptionFns: Record< description: "Created the annotation.", icon: , }), - addUserBoundingBoxSkeleton: (): Description => ({ + addUserBoundingBoxInSkeletonTracing: (): Description => ({ description: "Added a bounding box.", icon: , }), - addUserBoundingBoxVolume: (): Description => ({ + addUserBoundingBoxInVolumeTracing: (): Description => ({ description: "Added a bounding box.", icon: , }), - deleteUserBoundingBoxSkeleton: (): Description => ({ + deleteUserBoundingBoxInSkeletonTracing: (): Description => ({ description: "Deleted a bounding box.", icon: , }), - deleteUserBoundingBoxVolume: (): Description => ({ + deleteUserBoundingBoxInVolumeTracing: (): Description => ({ description: "Deleted a bounding box.", icon: , }), - updateUserBoundingBoxSkeleton: (): Description => ({ + updateUserBoundingBoxInSkeletonTracing: (): Description => ({ description: "Updated a bounding box.", icon: , }), - updateUserBoundingBoxVolume: (): Description => ({ + updateUserBoundingBoxInVolumeTracing: (): Description => ({ description: "Updated a bounding box.", icon: , }), - updateUserBoundingBoxVisibilitySkeleton: (): Description => ({ + updateUserBoundingBoxVisibilityInSkeletonTracing: (): Description => ({ description: "Toggled the visibility of a bounding box.", icon: , }), - updateUserBoundingBoxVisibilityVolume: (): Description => ({ + updateUserBoundingBoxVisibilityInVolumeTracing: (): Description => ({ description: "Toggled the visibility of a bounding box.", icon: , }), @@ -215,8 +215,9 @@ const descriptionFns: Record< icon: Hide Tree Edges Icon, }), updateTreeGroupVisibility: (action: UpdateTreeGroupVisibilityUpdateAction): Description => ({ - description: `Updated the visibility of the group with id ${action.value.treeGroupId != null ? action.value.treeGroupId : MISSING_GROUP_ID - }.`, + description: `Updated the visibility of the group with id ${ + action.value.treeGroupId != null ? action.value.treeGroupId : MISSING_GROUP_ID + }.`, icon: , }), createEdge: (action: CreateEdgeUpdateAction): Description => ({ From 3ed226fe50b1a67d62d9d39b50b97b0938bdbf5c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20B=C3=BC=C3=9Femeyer?= <39529669+MichaelBuessemeyer@users.noreply.github.com> Date: Wed, 7 May 2025 15:40:51 +0200 Subject: [PATCH 17/41] fix merge --- frontend/javascripts/oxalis/model/sagas/volumetracing_saga.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frontend/javascripts/oxalis/model/sagas/volumetracing_saga.tsx b/frontend/javascripts/oxalis/model/sagas/volumetracing_saga.tsx index 8823b29953f..a97f08f752a 100644 --- a/frontend/javascripts/oxalis/model/sagas/volumetracing_saga.tsx +++ b/frontend/javascripts/oxalis/model/sagas/volumetracing_saga.tsx @@ -77,7 +77,7 @@ import { Model, api } from "oxalis/singletons"; import type { Flycam, SegmentMap, VolumeTracing } from "oxalis/store"; import type { ActionPattern } from "redux-saga/effects"; import { actionChannel, call, fork, put, takeEvery, takeLatest } from "typed-redux-saga"; -import { AnnotationLayerEnum } from "types/api_flow_types"; +import { AnnotationLayerEnum } from "types/api_types"; import { pushSaveQueueTransaction } from "../actions/save_actions"; import { ensureWkReady } from "./ready_sagas"; import { diffBoundingBoxes } from "./skeletontracing_saga"; From ae851c39bfd74487fbf3351e2e45af6d101f801d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20B=C3=BC=C3=9Femeyer?= <39529669+MichaelBuessemeyer@users.noreply.github.com> Date: Wed, 7 May 2025 15:45:13 +0200 Subject: [PATCH 18/41] re-add old UpdateUserBoundingBoxVisibility action & fix naming of action names --- .../annotation/UpdateActions.scala | 18 +++++---- .../updating/SkeletonUpdateActions.scala | 37 +++++++++++++++++-- .../tracings/volume/VolumeUpdateActions.scala | 36 ++++++++++++++++-- 3 files changed, 78 insertions(+), 13 deletions(-) diff --git a/webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/annotation/UpdateActions.scala b/webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/annotation/UpdateActions.scala index a790185c066..99b0cf651d2 100644 --- a/webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/annotation/UpdateActions.scala +++ b/webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/annotation/UpdateActions.scala @@ -54,20 +54,24 @@ object UpdateAction { case "updateTreeEdgesVisibility" => deserialize[UpdateTreeEdgesVisibilitySkeletonAction](jsonValue) case "updateUserBoundingBoxesInSkeletonTracing" => deserialize[UpdateUserBoundingBoxesSkeletonAction](jsonValue) - case "addUserBoundingBoxSkeletonAction" => deserialize[AddUserBoundingBoxSkeletonAction](jsonValue) - case "deleteUserBoundingBoxSkeletonAction" => deserialize[DeleteUserBoundingBoxSkeletonAction](jsonValue) - case "updateUserBoundingBoxSkeletonAction" => + case "addUserBoundingBoxInSkeletonTracing" => deserialize[AddUserBoundingBoxSkeletonAction](jsonValue) + case "deleteUserBoundingBoxInSkeletonTracing" => deserialize[DeleteUserBoundingBoxSkeletonAction](jsonValue) + case "updateUserBoundingBoxInSkeletonTracing" => deserialize[UpdateUserBoundingBoxSkeletonAction](jsonValue) + case "updateUserBoundingBoxVisibilityInSkeletonTracing" => + deserialize[UpdateUserBoundingBoxVisibilitySkeletonAction](jsonValue) // Volume case "updateBucket" => deserialize[UpdateBucketVolumeAction](jsonValue) case "updateVolumeTracing" => deserialize[UpdateTracingVolumeAction](jsonValue) - case "updateUserBoundingBoxVolumeAction" => - deserialize[UpdateUserBoundingBoxVolumeAction](jsonValue) - case "addUserBoundingBoxVolumeAction" => deserialize[AddUserBoundingBoxVolumeAction](jsonValue) - case "deleteUserBoundingBoxVolumeAction" => deserialize[DeleteUserBoundingBoxVolumeAction](jsonValue) case "updateUserBoundingBoxesInVolumeTracing" => deserialize[UpdateUserBoundingBoxesVolumeAction](jsonValue) + case "addUserBoundingBoxInVolumeTracing" => deserialize[AddUserBoundingBoxVolumeAction](jsonValue) + case "deleteUserBoundingBoxInVolumeTracing" => deserialize[DeleteUserBoundingBoxVolumeAction](jsonValue) + case "updateUserBoundingBoxInVolumeTracing" => + deserialize[UpdateUserBoundingBoxVolumeAction](jsonValue) + case "updateUserBoundingBoxVisibilityInVolumeTracing" => + deserialize[UpdateUserBoundingBoxVisibilityVolumeAction](jsonValue) case "removeFallbackLayer" => deserialize[RemoveFallbackLayerVolumeAction](jsonValue) case "importVolumeTracing" => deserialize[ImportVolumeDataVolumeAction](jsonValue) case "createSegment" => deserialize[CreateSegmentVolumeAction](jsonValue) diff --git a/webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/tracings/skeleton/updating/SkeletonUpdateActions.scala b/webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/tracings/skeleton/updating/SkeletonUpdateActions.scala index 39b2313ae85..339859c2d4a 100644 --- a/webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/tracings/skeleton/updating/SkeletonUpdateActions.scala +++ b/webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/tracings/skeleton/updating/SkeletonUpdateActions.scala @@ -592,9 +592,6 @@ case class UpdateUserBoundingBoxSkeletonAction(boundingBoxId: Int, name = if (updatedPropKeys.contains("name") && updatedProps.name.isDefined) updatedProps.name else currentBoundingBox.name, - isVisible = - if (updatedPropKeys.contains("isVisible") && updatedProps.isVisible.isDefined) updatedProps.isVisible - else currentBoundingBox.isVisible, color = if (updatedPropKeys.contains("color") && updatedProps.color.isDefined) updatedProps.colorOptProto else currentBoundingBox.color, @@ -618,6 +615,36 @@ case class UpdateUserBoundingBoxSkeletonAction(boundingBoxId: Int, this.copy(actionTracingId = newTracingId) } +case class UpdateUserBoundingBoxVisibilitySkeletonAction(boundingBoxId: Option[Int], + isVisible: Boolean, + actionTracingId: String, + actionTimestamp: Option[Long] = None, + actionAuthorId: Option[String] = None, + info: Option[String] = None) + extends SkeletonUpdateAction { + override def applyOn(tracing: SkeletonTracing): SkeletonTracing = { + def updateUserBoundingBoxes() = + tracing.userBoundingBoxes.map { boundingBox => + if (boundingBoxId.forall(_ == boundingBox.id)) + boundingBox.copy(isVisible = Some(isVisible)) + else + boundingBox + } + + tracing.withUserBoundingBoxes(updateUserBoundingBoxes()) + } + + override def addTimestamp(timestamp: Long): UpdateAction = + this.copy(actionTimestamp = Some(timestamp)) + override def addInfo(info: Option[String]): UpdateAction = this.copy(info = info) + override def addAuthorId(authorId: Option[String]): UpdateAction = + this.copy(actionAuthorId = authorId) + override def withActionTracingId(newTracingId: String): LayerUpdateAction = + this.copy(actionTracingId = newTracingId) + + override def isViewOnlyChange: Boolean = true +} + object CreateTreeSkeletonAction { implicit val jsonFormat: OFormat[CreateTreeSkeletonAction] = Json.format[CreateTreeSkeletonAction] } @@ -681,3 +708,7 @@ object UpdateUserBoundingBoxSkeletonAction { implicit val jsonFormat: OFormat[UpdateUserBoundingBoxSkeletonAction] = Json.format[UpdateUserBoundingBoxSkeletonAction] } +object UpdateUserBoundingBoxVisibilitySkeletonAction { + implicit val jsonFormat: OFormat[UpdateUserBoundingBoxVisibilitySkeletonAction] = + Json.format[UpdateUserBoundingBoxVisibilitySkeletonAction] +} diff --git a/webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/tracings/volume/VolumeUpdateActions.scala b/webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/tracings/volume/VolumeUpdateActions.scala index fe51021407f..33d3c3b6f5a 100644 --- a/webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/tracings/volume/VolumeUpdateActions.scala +++ b/webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/tracings/volume/VolumeUpdateActions.scala @@ -170,9 +170,6 @@ case class UpdateUserBoundingBoxVolumeAction(boundingBoxId: Int, name = if (updatedPropKeys.contains("name") && updatedProps.name.isDefined) updatedProps.name else currentBoundingBox.name, - isVisible = - if (updatedPropKeys.contains("isVisible") && updatedProps.isVisible.isDefined) updatedProps.isVisible - else currentBoundingBox.isVisible, color = if (updatedPropKeys.contains("color") && updatedProps.color.isDefined) updatedProps.colorOptProto else currentBoundingBox.color, @@ -196,6 +193,36 @@ case class UpdateUserBoundingBoxVolumeAction(boundingBoxId: Int, this.copy(actionTracingId = newTracingId) } +case class UpdateUserBoundingBoxVisibilityVolumeAction(boundingBoxId: Option[Int], + isVisible: Boolean, + actionTracingId: String, + actionTimestamp: Option[Long] = None, + actionAuthorId: Option[String] = None, + info: Option[String] = None) + extends ApplyableVolumeUpdateAction { + override def addTimestamp(timestamp: Long): VolumeUpdateAction = this.copy(actionTimestamp = Some(timestamp)) + override def addAuthorId(authorId: Option[String]): VolumeUpdateAction = + this.copy(actionAuthorId = authorId) + override def addInfo(info: Option[String]): UpdateAction = this.copy(info = info) + override def withActionTracingId(newTracingId: String): LayerUpdateAction = + this.copy(actionTracingId = newTracingId) + + override def applyOn(tracing: VolumeTracing): VolumeTracing = { + + def updateUserBoundingBoxes(): Seq[NamedBoundingBoxProto] = + tracing.userBoundingBoxes.map { boundingBox => + if (boundingBoxId.forall(_ == boundingBox.id)) + boundingBox.copy(isVisible = Some(isVisible)) + else + boundingBox + } + + tracing.withUserBoundingBoxes(updateUserBoundingBoxes()) + } + + override def isViewOnlyChange: Boolean = true +} + case class RemoveFallbackLayerVolumeAction(actionTracingId: String, actionTimestamp: Option[Long] = None, actionAuthorId: Option[String] = None, @@ -461,6 +488,9 @@ object UpdateUserBoundingBoxVolumeAction { implicit val jsonFormat: OFormat[UpdateUserBoundingBoxVolumeAction] = Json.format[UpdateUserBoundingBoxVolumeAction] } +object UpdateUserBoundingBoxVisibilityVolumeAction { + implicit val jsonFormat: OFormat[UpdateUserBoundingBoxVisibilityVolumeAction] = Json.format[UpdateUserBoundingBoxVisibilityVolumeAction] +} object RemoveFallbackLayerVolumeAction { implicit val jsonFormat: OFormat[RemoveFallbackLayerVolumeAction] = Json.format[RemoveFallbackLayerVolumeAction] } From 003bcb73562a57d5082bedd7dcda5b99e4585cc0 Mon Sep 17 00:00:00 2001 From: Philipp Otto Date: Wed, 14 May 2025 11:17:33 +0200 Subject: [PATCH 19/41] format --- frontend/javascripts/viewer/model/sagas/skeletontracing_saga.ts | 2 +- frontend/javascripts/viewer/model/sagas/update_actions.ts | 2 +- frontend/javascripts/viewer/model/sagas/volumetracing_saga.tsx | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/frontend/javascripts/viewer/model/sagas/skeletontracing_saga.ts b/frontend/javascripts/viewer/model/sagas/skeletontracing_saga.ts index bc9a6b01bf1..69f83391b40 100644 --- a/frontend/javascripts/viewer/model/sagas/skeletontracing_saga.ts +++ b/frontend/javascripts/viewer/model/sagas/skeletontracing_saga.ts @@ -21,6 +21,7 @@ import { takeEvery, throttle, } from "typed-redux-saga"; +import { AnnotationLayerEnum, type ServerSkeletonTracing } from "types/api_types"; import { TreeTypeEnum } from "viewer/constants"; import { getLayerByName } from "viewer/model/accessors/dataset_accessor"; import { getPosition, getRotation } from "viewer/model/accessors/flycam_accessor"; @@ -92,7 +93,6 @@ import type { WebknossosState, } from "viewer/store"; import Store from "viewer/store"; -import { AnnotationLayerEnum, type ServerSkeletonTracing } from "types/api_types"; import { ensureWkReady } from "./ready_sagas"; import { takeWithBatchActionSupport } from "./saga_helpers"; diff --git a/frontend/javascripts/viewer/model/sagas/update_actions.ts b/frontend/javascripts/viewer/model/sagas/update_actions.ts index 09349d8b1c0..3001e8cdfce 100644 --- a/frontend/javascripts/viewer/model/sagas/update_actions.ts +++ b/frontend/javascripts/viewer/model/sagas/update_actions.ts @@ -1,5 +1,5 @@ import * as Utils from "libs/utils"; -import type { AdditionalCoordinate, APIMagRestrictions, MetadataEntryProto } from "types/api_types"; +import type { APIMagRestrictions, AdditionalCoordinate, MetadataEntryProto } from "types/api_types"; import { EMPTY_OBJECT, type Vector3 } from "viewer/constants"; import type { SendBucketInfo } from "viewer/model/bucket_data_handling/wkstore_adapter"; import { convertUserBoundingBoxesFromFrontendToServer } from "viewer/model/reducers/reducer_helpers"; diff --git a/frontend/javascripts/viewer/model/sagas/volumetracing_saga.tsx b/frontend/javascripts/viewer/model/sagas/volumetracing_saga.tsx index 0dc221c26b1..087c2dae0c3 100644 --- a/frontend/javascripts/viewer/model/sagas/volumetracing_saga.tsx +++ b/frontend/javascripts/viewer/model/sagas/volumetracing_saga.tsx @@ -11,6 +11,7 @@ import { AnnotationTool } from "viewer/model/accessors/tool_accessor"; import messages from "messages"; import type { ActionPattern } from "redux-saga/effects"; import { actionChannel, call, fork, put, takeEvery, takeLatest } from "typed-redux-saga"; +import { AnnotationLayerEnum } from "types/api_types"; import { getSupportedValueRangeOfLayer, isInSupportedValueRangeForLayer, @@ -77,7 +78,6 @@ import { import type VolumeLayer from "viewer/model/volumetracing/volumelayer"; import { Model, api } from "viewer/singletons"; import type { Flycam, SegmentMap, VolumeTracing } from "viewer/store"; -import { AnnotationLayerEnum } from "types/api_types"; import { pushSaveQueueTransaction } from "../actions/save_actions"; import { ensureWkReady } from "./ready_sagas"; import { diffBoundingBoxes } from "./skeletontracing_saga"; From 6043d959054b56c0e053e883665533bd85a49d6a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20B=C3=BC=C3=9Femeyer?= <39529669+MichaelBuessemeyer@users.noreply.github.com> Date: Thu, 15 May 2025 14:25:41 +0200 Subject: [PATCH 20/41] add missing serialization of bbox visibility updates & fix bbox action names in serialization --- .../annotation/UpdateActions.scala | 26 ++++++++++++------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/annotation/UpdateActions.scala b/webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/annotation/UpdateActions.scala index 99b0cf651d2..12b7416d96f 100644 --- a/webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/annotation/UpdateActions.scala +++ b/webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/annotation/UpdateActions.scala @@ -148,15 +148,18 @@ object UpdateAction { case s: UpdateUserBoundingBoxesSkeletonAction => Json.obj("name" -> "updateUserBoundingBoxesInSkeletonTracing", "value" -> Json.toJson(s)(UpdateUserBoundingBoxesSkeletonAction.jsonFormat)) - case s: UpdateUserBoundingBoxSkeletonAction => - Json.obj("name" -> "updateUserBoundingBoxSkeletonAction", - "value" -> Json.toJson(s)(UpdateUserBoundingBoxSkeletonAction.jsonFormat)) case s: AddUserBoundingBoxSkeletonAction => - Json.obj("name" -> "addUserBoundingBoxSkeletonAction", + Json.obj("name" -> "addUserBoundingBoxInSkeletonTracing", "value" -> Json.toJson(s)(AddUserBoundingBoxSkeletonAction.jsonFormat)) case s: DeleteUserBoundingBoxSkeletonAction => - Json.obj("name" -> "deleteUserBoundingBoxSkeletonAction", + Json.obj("name" -> "deleteUserBoundingBoxInSkeletonTracing", "value" -> Json.toJson(s)(DeleteUserBoundingBoxSkeletonAction.jsonFormat)) + case s: UpdateUserBoundingBoxSkeletonAction => + Json.obj("name" -> "updateUserBoundingBoxInSkeletonTracing", + "value" -> Json.toJson(s)(UpdateUserBoundingBoxSkeletonAction.jsonFormat)) + case s: UpdateUserBoundingBoxVisibilitySkeletonAction => + Json.obj("name" -> "updateUserBoundingBoxVisibilityInSkeletonTracing", + "value" -> Json.toJson(s)(UpdateUserBoundingBoxVisibilitySkeletonAction.jsonFormat)) // Volume case s: UpdateBucketVolumeAction => @@ -166,15 +169,18 @@ object UpdateAction { case s: UpdateUserBoundingBoxesVolumeAction => Json.obj("name" -> "updateUserBoundingBoxesInVolumeTracing", "value" -> Json.toJson(s)(UpdateUserBoundingBoxesVolumeAction.jsonFormat)) - case s: UpdateUserBoundingBoxVolumeAction => - Json.obj("name" -> "updateUserBoundingBoxVolumeAction", - "value" -> Json.toJson(s)(UpdateUserBoundingBoxVolumeAction.jsonFormat)) case s: AddUserBoundingBoxVolumeAction => - Json.obj("name" -> "addUserBoundingBoxVolumeAction", + Json.obj("name" -> "addUserBoundingBoxInVolumeTracing", "value" -> Json.toJson(s)(AddUserBoundingBoxVolumeAction.jsonFormat)) case s: DeleteUserBoundingBoxVolumeAction => - Json.obj("name" -> "deleteUserBoundingBoxVolumeAction", + Json.obj("name" -> "deleteUserBoundingBoxInVolumeTracing", "value" -> Json.toJson(s)(DeleteUserBoundingBoxVolumeAction.jsonFormat)) + case s: UpdateUserBoundingBoxVolumeAction => + Json.obj("name" -> "updateUserBoundingBoxInVolumeTracing", + "value" -> Json.toJson(s)(UpdateUserBoundingBoxVolumeAction.jsonFormat)) + case s: UpdateUserBoundingBoxVisibilityVolumeAction => + Json.obj("name" -> "updateUserBoundingBoxVisibilityInVolumeTracing", + "value" -> Json.toJson(s)(UpdateUserBoundingBoxVisibilityVolumeAction.jsonFormat)) case s: RemoveFallbackLayerVolumeAction => Json.obj("name" -> "removeFallbackLayer", "value" -> Json.toJson(s)(RemoveFallbackLayerVolumeAction.jsonFormat)) case s: ImportVolumeDataVolumeAction => From 589554cb8b1fd4711aa40442c71233281954735a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20B=C3=BC=C3=9Femeyer?= <39529669+MichaelBuessemeyer@users.noreply.github.com> Date: Thu, 15 May 2025 14:33:34 +0200 Subject: [PATCH 21/41] format backend --- .../tracingstore/tracings/volume/VolumeUpdateActions.scala | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/tracings/volume/VolumeUpdateActions.scala b/webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/tracings/volume/VolumeUpdateActions.scala index 33d3c3b6f5a..7265469ad3c 100644 --- a/webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/tracings/volume/VolumeUpdateActions.scala +++ b/webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/tracings/volume/VolumeUpdateActions.scala @@ -489,7 +489,8 @@ object UpdateUserBoundingBoxVolumeAction { Json.format[UpdateUserBoundingBoxVolumeAction] } object UpdateUserBoundingBoxVisibilityVolumeAction { - implicit val jsonFormat: OFormat[UpdateUserBoundingBoxVisibilityVolumeAction] = Json.format[UpdateUserBoundingBoxVisibilityVolumeAction] + implicit val jsonFormat: OFormat[UpdateUserBoundingBoxVisibilityVolumeAction] = + Json.format[UpdateUserBoundingBoxVisibilityVolumeAction] } object RemoveFallbackLayerVolumeAction { implicit val jsonFormat: OFormat[RemoveFallbackLayerVolumeAction] = Json.format[RemoveFallbackLayerVolumeAction] From e4e690887b414d437b8bebd8caf5eb8f6f747997 Mon Sep 17 00:00:00 2001 From: Charlie Meister Date: Thu, 15 May 2025 15:10:30 +0200 Subject: [PATCH 22/41] remove console.log --- .../viewer/model/helpers/compaction/compact_save_queue.ts | 6 ------ 1 file changed, 6 deletions(-) diff --git a/frontend/javascripts/viewer/model/helpers/compaction/compact_save_queue.ts b/frontend/javascripts/viewer/model/helpers/compaction/compact_save_queue.ts index 99422722369..f73c7a85261 100644 --- a/frontend/javascripts/viewer/model/helpers/compaction/compact_save_queue.ts +++ b/frontend/javascripts/viewer/model/helpers/compaction/compact_save_queue.ts @@ -91,12 +91,6 @@ function removeSubsequentUpdateBBoxActions(updateActionsBatches: Array Date: Wed, 21 May 2025 13:44:11 +0200 Subject: [PATCH 23/41] WIP frontend: address review 1 --- .../helpers/compaction/compact_save_queue.ts | 20 ++++--- .../viewer/model/reducers/reducer_helpers.ts | 2 +- .../model/sagas/skeletontracing_saga.ts | 43 ++++++++------- .../viewer/model/sagas/update_actions.ts | 45 +++++++-------- .../javascripts/viewer/view/version_entry.tsx | 55 +++++++++++++------ 5 files changed, 96 insertions(+), 69 deletions(-) diff --git a/frontend/javascripts/viewer/model/helpers/compaction/compact_save_queue.ts b/frontend/javascripts/viewer/model/helpers/compaction/compact_save_queue.ts index f73c7a85261..6f17f6e007f 100644 --- a/frontend/javascripts/viewer/model/helpers/compaction/compact_save_queue.ts +++ b/frontend/javascripts/viewer/model/helpers/compaction/compact_save_queue.ts @@ -74,6 +74,7 @@ function removeSubsequentUpdateBBoxActions(updateActionsBatches: Array, ): Array { @@ -127,13 +137,5 @@ export default function compactSaveQueue( (updateActionsBatch) => updateActionsBatch.actions.length > 0, ); - return removeSubsequentUpdateBBoxActions( - removeSubsequentUpdateSegmentActions( - removeSubsequentUpdateTreeActions( - removeSubsequentUpdateNodeActions( - removeAllButLastUpdateTdCameraAction(removeAllButLastUpdateTracingAction(result)), - ), - ), - ), - ); + return compactAll(result); } diff --git a/frontend/javascripts/viewer/model/reducers/reducer_helpers.ts b/frontend/javascripts/viewer/model/reducers/reducer_helpers.ts index b5d98c0b4de..e560bcab77e 100644 --- a/frontend/javascripts/viewer/model/reducers/reducer_helpers.ts +++ b/frontend/javascripts/viewer/model/reducers/reducer_helpers.ts @@ -56,7 +56,7 @@ export function convertUserBoundingBoxesFromServerToFrontend( }); } -export function convertUserBoundingBoxesFromFrontendToServer( +export function convertUserBoundingBoxFromFrontendToServer( boundingBox: UserBoundingBox, ): UserBoundingBoxToServer { const { boundingBox: bb, ...rest } = boundingBox; diff --git a/frontend/javascripts/viewer/model/sagas/skeletontracing_saga.ts b/frontend/javascripts/viewer/model/sagas/skeletontracing_saga.ts index 69f83391b40..d842b95e4de 100644 --- a/frontend/javascripts/viewer/model/sagas/skeletontracing_saga.ts +++ b/frontend/javascripts/viewer/model/sagas/skeletontracing_saga.ts @@ -60,26 +60,26 @@ import type { Saga } from "viewer/model/sagas/effect-generators"; import { select } from "viewer/model/sagas/effect-generators"; import type { UpdateActionWithoutIsolationRequirement } from "viewer/model/sagas/update_actions"; import { - addUserBoundingBoxInSkeletonTracingAction, - addUserBoundingBoxInVolumeTracingAction, + addUserBoundingBoxInSkeletonTracing, + addUserBoundingBoxInVolumeTracing, createEdge, createNode, createTree, deleteEdge, deleteNode, deleteTree, - deleteUserBoundingBoxInSkeletonTracingAction, - deleteUserBoundingBoxInVolumeTracingAction, + deleteUserBoundingBoxInSkeletonTracing, + deleteUserBoundingBoxInVolumeTracing, updateNode, updateSkeletonTracing, updateTree, updateTreeEdgesVisibility, updateTreeGroups, updateTreeVisibility, - updateUserBoundingBoxInSkeletonTracingAction, - updateUserBoundingBoxInVolumeTracingAction, - updateUserBoundingBoxVisibilityInSkeletonTracingAction, - updateUserBoundingBoxVisibilityInVolumeTracingAction, + updateUserBoundingBoxInSkeletonTracing, + updateUserBoundingBoxInVolumeTracing, + updateUserBoundingBoxVisibilityInSkeletonTracing, + updateUserBoundingBoxVisibilityInVolumeTracing, } from "viewer/model/sagas/update_actions"; import { api } from "viewer/singletons"; import type { @@ -621,30 +621,31 @@ export function* diffBoundingBoxes( tracingId: string, tracingType: AnnotationLayerEnum, ) { + if (prevBoundingBoxes === currentBoundingBoxes) return; const { onlyA: deletedBBoxIds, onlyB: addedBBoxIds, both: maybeChangedBBoxIds, } = Utils.diffArrays( - _.map(prevBoundingBoxes, (bbox) => bbox.id), - _.map(currentBoundingBoxes, (bbox) => bbox.id), + prevBoundingBoxes.map((bbox) => bbox.id), + currentBoundingBoxes.map((bbox) => bbox.id), ); const addBBoxAction = tracingType === AnnotationLayerEnum.Skeleton - ? addUserBoundingBoxInSkeletonTracingAction - : addUserBoundingBoxInVolumeTracingAction; + ? addUserBoundingBoxInSkeletonTracing + : addUserBoundingBoxInVolumeTracing; const deleteBBoxAction = tracingType === AnnotationLayerEnum.Skeleton - ? deleteUserBoundingBoxInSkeletonTracingAction - : deleteUserBoundingBoxInVolumeTracingAction; + ? deleteUserBoundingBoxInSkeletonTracing + : deleteUserBoundingBoxInVolumeTracing; const updateBBoxAction = tracingType === AnnotationLayerEnum.Skeleton - ? updateUserBoundingBoxInSkeletonTracingAction - : updateUserBoundingBoxInVolumeTracingAction; + ? updateUserBoundingBoxInSkeletonTracing + : updateUserBoundingBoxInVolumeTracing; const updateBBoxVisibilityAction = tracingType === AnnotationLayerEnum.Skeleton - ? updateUserBoundingBoxVisibilityInSkeletonTracingAction - : updateUserBoundingBoxVisibilityInVolumeTracingAction; + ? updateUserBoundingBoxVisibilityInSkeletonTracing + : updateUserBoundingBoxVisibilityInVolumeTracing; const getErrorMessage = (id: number) => `User bounding box with id ${id} not found in ${tracingType} tracing.`; for (const id of deletedBBoxIds) { @@ -655,16 +656,16 @@ export function* diffBoundingBoxes( if (bbox) { yield addBBoxAction(bbox, tracingId); } else { - Toast.error(getErrorMessage(id)); + throw new Error(getErrorMessage(id)); } } for (const id of maybeChangedBBoxIds) { const currentBbox = currentBoundingBoxes.find((bbox) => bbox.id === id); const prevBbox = prevBoundingBoxes.find((bbox) => bbox.id === id); if (currentBbox == null || prevBbox == null) { - Toast.error(getErrorMessage(id)); - continue; + throw new Error(getErrorMessage(id)); } + if (currentBbox === prevBbox) continue; const diffBBox = Utils.diffObjects(currentBbox, prevBbox); if (_.isEmpty(diffBBox)) continue; const changedKeys = Object.keys(diffBBox); diff --git a/frontend/javascripts/viewer/model/sagas/update_actions.ts b/frontend/javascripts/viewer/model/sagas/update_actions.ts index 3001e8cdfce..2951d966ac5 100644 --- a/frontend/javascripts/viewer/model/sagas/update_actions.ts +++ b/frontend/javascripts/viewer/model/sagas/update_actions.ts @@ -2,7 +2,7 @@ import * as Utils from "libs/utils"; import type { APIMagRestrictions, AdditionalCoordinate, MetadataEntryProto } from "types/api_types"; import { EMPTY_OBJECT, type Vector3 } from "viewer/constants"; import type { SendBucketInfo } from "viewer/model/bucket_data_handling/wkstore_adapter"; -import { convertUserBoundingBoxesFromFrontendToServer } from "viewer/model/reducers/reducer_helpers"; +import { convertUserBoundingBoxFromFrontendToServer } from "viewer/model/reducers/reducer_helpers"; import type { Node, NumberLike, @@ -17,6 +17,7 @@ export type NodeWithTreeId = { treeId: number; } & Node; +// This type is meant to contain only the properties that have changed type PartialBoundingBoxWithoutVisibility = Partial>; export type UpdateTreeUpdateAction = ReturnType | ReturnType; @@ -38,28 +39,28 @@ export type UpdateSegmentUpdateAction = ReturnType; export type DeleteSegmentDataUpdateAction = ReturnType; export type AddUserBoundingBoxInSkeletonTracingAction = ReturnType< - typeof addUserBoundingBoxInSkeletonTracingAction + typeof addUserBoundingBoxInSkeletonTracing >; export type AddUserBoundingBoxInVolumeTracingAction = ReturnType< - typeof addUserBoundingBoxInVolumeTracingAction + typeof addUserBoundingBoxInVolumeTracing >; export type DeleteUserBoundingBoxInSkeletonTracingAction = ReturnType< - typeof deleteUserBoundingBoxInSkeletonTracingAction + typeof deleteUserBoundingBoxInSkeletonTracing >; export type DeleteUserBoundingBoxInVolumeTracingAction = ReturnType< - typeof deleteUserBoundingBoxInVolumeTracingAction + typeof deleteUserBoundingBoxInVolumeTracing >; export type UpdateUserBoundingBoxInSkeletonTracingAction = ReturnType< - typeof updateUserBoundingBoxInSkeletonTracingAction + typeof updateUserBoundingBoxInSkeletonTracing >; export type UpdateUserBoundingBoxInVolumeTracingAction = ReturnType< - typeof updateUserBoundingBoxInVolumeTracingAction + typeof updateUserBoundingBoxInVolumeTracing >; export type UpdateUserBoundingBoxVisibilityInSkeletonTracingAction = ReturnType< - typeof updateUserBoundingBoxVisibilityInSkeletonTracingAction + typeof updateUserBoundingBoxVisibilityInSkeletonTracing >; export type UpdateUserBoundingBoxVisibilityInVolumeTracingAction = ReturnType< - typeof updateUserBoundingBoxVisibilityInVolumeTracingAction + typeof updateUserBoundingBoxVisibilityInVolumeTracing >; export type UpdateBucketUpdateAction = ReturnType; export type UpdateSegmentGroupsUpdateAction = ReturnType; @@ -401,33 +402,33 @@ export function updateVolumeTracing( } as const; } -export function addUserBoundingBoxInSkeletonTracingAction( +export function addUserBoundingBoxInSkeletonTracing( boundingBox: UserBoundingBox, actionTracingId: string, ) { return { name: "addUserBoundingBoxInSkeletonTracing", value: { - boundingBox: convertUserBoundingBoxesFromFrontendToServer(boundingBox), + boundingBox: convertUserBoundingBoxFromFrontendToServer(boundingBox), actionTracingId, }, } as const; } -export function addUserBoundingBoxInVolumeTracingAction( +export function addUserBoundingBoxInVolumeTracing( boundingBox: UserBoundingBox, actionTracingId: string, ) { return { name: "addUserBoundingBoxInVolumeTracing", value: { - boundingBox: convertUserBoundingBoxesFromFrontendToServer(boundingBox), + boundingBox: convertUserBoundingBoxFromFrontendToServer(boundingBox), actionTracingId, }, } as const; } -export function deleteUserBoundingBoxInSkeletonTracingAction( +export function deleteUserBoundingBoxInSkeletonTracing( boundingBoxId: number, actionTracingId: string, ) { @@ -440,7 +441,7 @@ export function deleteUserBoundingBoxInSkeletonTracingAction( } as const; } -export function deleteUserBoundingBoxInVolumeTracingAction( +export function deleteUserBoundingBoxInVolumeTracing( boundingBoxId: number, actionTracingId: string, ) { @@ -453,7 +454,7 @@ export function deleteUserBoundingBoxInVolumeTracingAction( } as const; } -function getUpdateUserBoundingBoxAction( +function getUpdateUserBoundingBox( actionName: "updateUserBoundingBoxInVolumeTracing" | "updateUserBoundingBoxInSkeletonTracing", boundingBoxId: number, updatedProps: PartialBoundingBoxWithoutVisibility, @@ -476,12 +477,12 @@ function getUpdateUserBoundingBoxAction( } as const; } -export function updateUserBoundingBoxInVolumeTracingAction( +export function updateUserBoundingBoxInVolumeTracing( boundingBoxId: number, updatedProps: PartialBoundingBoxWithoutVisibility, actionTracingId: string, ) { - return getUpdateUserBoundingBoxAction( + return getUpdateUserBoundingBox( "updateUserBoundingBoxInVolumeTracing", boundingBoxId, updatedProps, @@ -489,12 +490,12 @@ export function updateUserBoundingBoxInVolumeTracingAction( ); } -export function updateUserBoundingBoxInSkeletonTracingAction( +export function updateUserBoundingBoxInSkeletonTracing( boundingBoxId: number, updatedProps: Partial, actionTracingId: string, ) { - return getUpdateUserBoundingBoxAction( + return getUpdateUserBoundingBox( "updateUserBoundingBoxInSkeletonTracing", boundingBoxId, updatedProps, @@ -502,7 +503,7 @@ export function updateUserBoundingBoxInSkeletonTracingAction( ); } -export function updateUserBoundingBoxVisibilityInSkeletonTracingAction( +export function updateUserBoundingBoxVisibilityInSkeletonTracing( boundingBoxId: number, isVisible: boolean, actionTracingId: string, @@ -516,7 +517,7 @@ export function updateUserBoundingBoxVisibilityInSkeletonTracingAction( }, } as const; } -export function updateUserBoundingBoxVisibilityInVolumeTracingAction( +export function updateUserBoundingBoxVisibilityInVolumeTracing( boundingBoxId: number, isVisible: boolean, actionTracingId: string, diff --git a/frontend/javascripts/viewer/view/version_entry.tsx b/frontend/javascripts/viewer/view/version_entry.tsx index 59bdd1e45db..1fb570dbaf5 100644 --- a/frontend/javascripts/viewer/view/version_entry.tsx +++ b/frontend/javascripts/viewer/view/version_entry.tsx @@ -22,6 +22,8 @@ import { getReadableNameByVolumeTracingId } from "viewer/model/accessors/volumet import type { AddLayerToAnnotationUpdateAction, AddSegmentIndexUpdateAction, + AddUserBoundingBoxInSkeletonTracingAction, + AddUserBoundingBoxInVolumeTracingAction, CreateEdgeUpdateAction, CreateNodeUpdateAction, CreateSegmentUpdateAction, @@ -31,6 +33,8 @@ import type { DeleteSegmentDataUpdateAction, DeleteSegmentUpdateAction, DeleteTreeUpdateAction, + DeleteUserBoundingBoxInSkeletonTracingAction, + DeleteUserBoundingBoxInVolumeTracingAction, MergeAgglomerateUpdateAction, MergeTreeUpdateAction, MoveTreeComponentUpdateAction, @@ -48,6 +52,9 @@ import type { UpdateTreeGroupVisibilityUpdateAction, UpdateTreeUpdateAction, UpdateTreeVisibilityUpdateAction, + UpdateUserBoundingBoxInSkeletonTracingAction, + UpdateUserBoundingBoxInVolumeTracingAction, + UpdateUserBoundingBoxVisibilityInSkeletonTracingAction, } from "viewer/model/sagas/update_actions"; import type { StoreAnnotation } from "viewer/store"; import { MISSING_GROUP_ID } from "viewer/view/right-border-tabs/trees_tab/tree_hierarchy_view_helpers"; @@ -75,36 +82,52 @@ const descriptionFns: Record< description: "Created the annotation.", icon: , }), - addUserBoundingBoxInSkeletonTracing: (): Description => ({ - description: "Added a bounding box.", + addUserBoundingBoxInSkeletonTracing: ( + firstAction: AddUserBoundingBoxInSkeletonTracingAction, + ): Description => ({ + description: `Created bounding box ${firstAction.value.boundingBox.id}.`, icon: , }), - addUserBoundingBoxInVolumeTracing: (): Description => ({ - description: "Added a bounding box.", + addUserBoundingBoxInVolumeTracing: ( + firstAction: AddUserBoundingBoxInVolumeTracingAction, + ): Description => ({ + description: `Created bounding box ${firstAction.value.boundingBox.id}.`, icon: , }), - deleteUserBoundingBoxInSkeletonTracing: (): Description => ({ - description: "Deleted a bounding box.", + deleteUserBoundingBoxInSkeletonTracing: ( + firstAction: DeleteUserBoundingBoxInSkeletonTracingAction, + ): Description => ({ + description: `Deleted bounding box ${firstAction.value.boundingBoxId}.`, icon: , }), - deleteUserBoundingBoxInVolumeTracing: (): Description => ({ - description: "Deleted a bounding box.", + deleteUserBoundingBoxInVolumeTracing: ( + firstAction: DeleteUserBoundingBoxInVolumeTracingAction, + ): Description => ({ + description: `Deleted bounding box ${firstAction.value.boundingBoxId}.`, icon: , }), - updateUserBoundingBoxInSkeletonTracing: (): Description => ({ - description: "Updated a bounding box.", + updateUserBoundingBoxInSkeletonTracing: ( + firstAction: UpdateUserBoundingBoxInSkeletonTracingAction, + ): Description => ({ + description: `Updated bounding box ${firstAction.value.boundingBoxId}.`, icon: , }), - updateUserBoundingBoxInVolumeTracing: (): Description => ({ - description: "Updated a bounding box.", + updateUserBoundingBoxInVolumeTracing: ( + firstAction: UpdateUserBoundingBoxInVolumeTracingAction, + ): Description => ({ + description: `Updated bounding box ${firstAction.value.boundingBoxId}.`, icon: , }), - updateUserBoundingBoxVisibilityInSkeletonTracing: (): Description => ({ - description: "Toggled the visibility of a bounding box.", + updateUserBoundingBoxVisibilityInSkeletonTracing: ( + firstAction: UpdateUserBoundingBoxVisibilityInSkeletonTracingAction, + ): Description => ({ + description: `Toggled the visibility of bounding box ${firstAction.value.boundingBoxId}.`, icon: , }), - updateUserBoundingBoxVisibilityInVolumeTracing: (): Description => ({ - description: "Toggled the visibility of a bounding box.", + updateUserBoundingBoxVisibilityInVolumeTracing: ( + firstAction: UpdateUserBoundingBoxInVolumeTracingAction, + ): Description => ({ + description: `Toggled the visibility of bounding box ${firstAction.value.boundingBoxId}.`, icon: , }), removeFallbackLayer: (): Description => ({ From 93a19556c20fdf755f8b882dadcfa222dfdc9eef Mon Sep 17 00:00:00 2001 From: Charlie Meister Date: Thu, 22 May 2025 20:30:09 +0200 Subject: [PATCH 24/41] frontend: add legacy actions back in --- .../viewer/model/sagas/update_actions.ts | 39 ++++++++++++++++++- .../javascripts/viewer/view/version_entry.tsx | 18 ++++++++- 2 files changed, 54 insertions(+), 3 deletions(-) diff --git a/frontend/javascripts/viewer/model/sagas/update_actions.ts b/frontend/javascripts/viewer/model/sagas/update_actions.ts index 2951d966ac5..0a4b946f11a 100644 --- a/frontend/javascripts/viewer/model/sagas/update_actions.ts +++ b/frontend/javascripts/viewer/model/sagas/update_actions.ts @@ -38,6 +38,12 @@ export type CreateSegmentUpdateAction = ReturnType; export type DeleteSegmentUpdateAction = ReturnType; export type DeleteSegmentDataUpdateAction = ReturnType; +export type LEGACY_UpdateUserBoundingBoxesInSkeletonTracingUpdateAction = ReturnType< + typeof LEGACY_updateUserBoundingBoxesInSkeletonTracing +>; +export type LEGACY_UpdateUserBoundingBoxesInVolumeTracingUpdateAction = ReturnType< + typeof LEGACY_updateUserBoundingBoxesInVolumeTracing +>; export type AddUserBoundingBoxInSkeletonTracingAction = ReturnType< typeof addUserBoundingBoxInSkeletonTracing >; @@ -101,6 +107,8 @@ export type UpdateActionWithoutIsolationRequirement = | DeleteEdgeUpdateAction | UpdateSkeletonTracingUpdateAction | UpdateVolumeTracingUpdateAction + | LEGACY_UpdateUserBoundingBoxesInSkeletonTracingUpdateAction + | LEGACY_UpdateUserBoundingBoxesInVolumeTracingUpdateAction | AddUserBoundingBoxInSkeletonTracingAction | AddUserBoundingBoxInVolumeTracingAction | DeleteUserBoundingBoxInSkeletonTracingAction @@ -401,7 +409,34 @@ export function updateVolumeTracing( }, } as const; } - +export function LEGACY_updateUserBoundingBoxesInSkeletonTracing( + userBoundingBoxes: Array, + actionTracingId: string, +) { + return { + name: "updateUserBoundingBoxesInSkeletonTracing", + value: { + actionTracingId, + boundingBoxes: userBoundingBoxes.map((bbox) => + convertUserBoundingBoxFromFrontendToServer(bbox), + ), + }, + } as const; +} +export function LEGACY_updateUserBoundingBoxesInVolumeTracing( + userBoundingBoxes: Array, + actionTracingId: string, +) { + return { + name: "updateUserBoundingBoxesInVolumeTracing", + value: { + actionTracingId, + boundingBoxes: userBoundingBoxes.map((bbox) => + convertUserBoundingBoxFromFrontendToServer(bbox), + ), + }, + } as const; +} export function addUserBoundingBoxInSkeletonTracing( boundingBox: UserBoundingBox, actionTracingId: string, @@ -492,7 +527,7 @@ export function updateUserBoundingBoxInVolumeTracing( export function updateUserBoundingBoxInSkeletonTracing( boundingBoxId: number, - updatedProps: Partial, + updatedProps: PartialBoundingBoxWithoutVisibility, actionTracingId: string, ) { return getUpdateUserBoundingBox( diff --git a/frontend/javascripts/viewer/view/version_entry.tsx b/frontend/javascripts/viewer/view/version_entry.tsx index 1fb570dbaf5..ce8c106c077 100644 --- a/frontend/javascripts/viewer/view/version_entry.tsx +++ b/frontend/javascripts/viewer/view/version_entry.tsx @@ -2,6 +2,7 @@ import { ArrowsAltOutlined, BackwardOutlined, CodeSandboxOutlined, + CodepenOutlined, DeleteOutlined, EditOutlined, EyeOutlined, @@ -35,6 +36,8 @@ import type { DeleteTreeUpdateAction, DeleteUserBoundingBoxInSkeletonTracingAction, DeleteUserBoundingBoxInVolumeTracingAction, + LEGACY_UpdateUserBoundingBoxesInSkeletonTracingUpdateAction, + LEGACY_UpdateUserBoundingBoxesInVolumeTracingUpdateAction, MergeAgglomerateUpdateAction, MergeTreeUpdateAction, MoveTreeComponentUpdateAction, @@ -55,6 +58,7 @@ import type { UpdateUserBoundingBoxInSkeletonTracingAction, UpdateUserBoundingBoxInVolumeTracingAction, UpdateUserBoundingBoxVisibilityInSkeletonTracingAction, + UpdateUserBoundingBoxVisibilityInVolumeTracingAction, } from "viewer/model/sagas/update_actions"; import type { StoreAnnotation } from "viewer/store"; import { MISSING_GROUP_ID } from "viewer/view/right-border-tabs/trees_tab/tree_hierarchy_view_helpers"; @@ -82,6 +86,18 @@ const descriptionFns: Record< description: "Created the annotation.", icon: , }), + updateUserBoundingBoxesInSkeletonTracing: ( + firstAction: LEGACY_UpdateUserBoundingBoxesInSkeletonTracingUpdateAction, + ): Description => ({ + description: `Updated bounding boxes ${firstAction.value.boundingBoxes.map((bbox) => bbox.id).join()}.`, + icon: , + }), + updateUserBoundingBoxesInVolumeTracing: ( + firstAction: LEGACY_UpdateUserBoundingBoxesInVolumeTracingUpdateAction, + ): Description => ({ + description: `Updated bounding boxes ${firstAction.value.boundingBoxes.map((bbox) => bbox.id).join()}.`, + icon: , + }), addUserBoundingBoxInSkeletonTracing: ( firstAction: AddUserBoundingBoxInSkeletonTracingAction, ): Description => ({ @@ -125,7 +141,7 @@ const descriptionFns: Record< icon: , }), updateUserBoundingBoxVisibilityInVolumeTracing: ( - firstAction: UpdateUserBoundingBoxInVolumeTracingAction, + firstAction: UpdateUserBoundingBoxVisibilityInVolumeTracingAction, ): Description => ({ description: `Toggled the visibility of bounding box ${firstAction.value.boundingBoxId}.`, icon: , From df56334dd80bd0c71ad48fa51458143afe407175 Mon Sep 17 00:00:00 2001 From: Charlie Meister Date: Thu, 22 May 2025 23:48:04 +0200 Subject: [PATCH 25/41] send boolean list to backend --- .../viewer/model/sagas/update_actions.ts | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/frontend/javascripts/viewer/model/sagas/update_actions.ts b/frontend/javascripts/viewer/model/sagas/update_actions.ts index 0a4b946f11a..c85634e1b9e 100644 --- a/frontend/javascripts/viewer/model/sagas/update_actions.ts +++ b/frontend/javascripts/viewer/model/sagas/update_actions.ts @@ -1,6 +1,6 @@ import * as Utils from "libs/utils"; import type { APIMagRestrictions, AdditionalCoordinate, MetadataEntryProto } from "types/api_types"; -import { EMPTY_OBJECT, type Vector3 } from "viewer/constants"; +import type { Vector3 } from "viewer/constants"; import type { SendBucketInfo } from "viewer/model/bucket_data_handling/wkstore_adapter"; import { convertUserBoundingBoxFromFrontendToServer } from "viewer/model/reducers/reducer_helpers"; import type { @@ -495,19 +495,21 @@ function getUpdateUserBoundingBox( updatedProps: PartialBoundingBoxWithoutVisibility, actionTracingId: string, ) { - let serverBBox = EMPTY_OBJECT; const { boundingBox, ...rest } = updatedProps; - const updatedPropKeys = Object.keys(updatedProps); - if (boundingBox != null) { - serverBBox = { boundingBox: Utils.computeBoundingBoxObjectFromBoundingBox(boundingBox) }; - } + const updatedPropsForServer = + boundingBox != null + ? { ...rest, boundingBox: Utils.computeBoundingBoxObjectFromBoundingBox(boundingBox) } + : updatedProps; + const updatedPropsKeys = Object.keys(updatedPropsForServer); return { name: actionName, value: { boundingBoxId, actionTracingId, - updatedProps: { ...serverBBox, ...rest }, - updatedPropKeys, + updatedProps: updatedPropsForServer, + hasUpdatedBoundingBox: updatedPropsKeys.includes("boundingBox"), + hasUpdatedName: updatedPropsKeys.includes("name"), + haUpdatedColor: updatedPropsKeys.includes("color"), }, } as const; } From 1b9dbebf3e8f9221170181f4a374b72887c2ba2b Mon Sep 17 00:00:00 2001 From: Charlie Meister Date: Fri, 23 May 2025 12:24:14 +0200 Subject: [PATCH 26/41] [ci skip] WIP: add test and rewrite compact save function --- .../test/model/compact_save_queue.spec.ts | 142 ++++++++++++++++++ .../helpers/compaction/compact_save_queue.ts | 55 ++++--- .../viewer/model/sagas/update_actions.ts | 2 +- 3 files changed, 178 insertions(+), 21 deletions(-) create mode 100644 frontend/javascripts/test/model/compact_save_queue.spec.ts diff --git a/frontend/javascripts/test/model/compact_save_queue.spec.ts b/frontend/javascripts/test/model/compact_save_queue.spec.ts new file mode 100644 index 00000000000..6f4c33a9876 --- /dev/null +++ b/frontend/javascripts/test/model/compact_save_queue.spec.ts @@ -0,0 +1,142 @@ +import { removeSubsequentUpdateBBoxActions } from "viewer/model/helpers/compaction/compact_save_queue"; +import { describe, expect, it } from "vitest"; + +describe("Compact Save Queue", () => { + it("UpdateUserBoundingBoxActions should be compacted", () => { + const actions = [ + { + version: -1, + transactionId: "eq3scfgvoa", + transactionGroupCount: 1, + transactionGroupIndex: 0, + timestamp: 1747994347590, + authorId: "123", + actions: [ + { + name: "updateUserBoundingBoxInSkeletonTracing", + value: { + boundingBoxId: 3, + actionTracingId: "f33b5d13-3b4a-4e98-abfc-64bf7ca82002", + updatedProps: { + boundingBox: { + topLeft: [3224, 3955, 944], + width: 1, + height: 157, + depth: 12, + }, + }, + hasUpdatedBoundingBox: true, + hasUpdatedName: false, + hasUpdatedColor: false, + }, + }, + ], + stats: { + "4bb38dd3-fa3a-4034-9608-beedb2ecfc10": { + segmentCount: 4, + }, + "f33b5d13-3b4a-4e98-abfc-64bf7ca82002": { + treeCount: 6, + nodeCount: 28, + edgeCount: 23, + branchPointCount: 0, + }, + }, + info: '["INITIALIZE_CONNECTOME_TRACING","SET_TD_CAMERA_WITHOUT_TIME_TRACKING * 5","SET_INPUT_CATCHER_RECTS","SET_TD_CAMERA_WITHOUT_TIME_TRACKING","UPDATE_CONNECTOME_FILE_LIST","SET_MAXIMUM_ZOOM_FOR_ALL_MAGS_FOR_LAYER * 4","SET_TOOL","CHANGE_USER_BOUNDING_BOX * 2","SET_TD_CAMERA_WITHOUT_TIME_TRACKING","CHANGE_USER_BOUNDING_BOX * 3"]', + }, + { + version: -1, + transactionId: "msk6wwebqf", + transactionGroupCount: 1, + transactionGroupIndex: 0, + timestamp: 1747994347633, + authorId: "123", + actions: [ + { + name: "updateUserBoundingBoxInSkeletonTracing", + value: { + boundingBoxId: 3, + actionTracingId: "f33b5d13-3b4a-4e98-abfc-64bf7ca82002", + updatedProps: { + boundingBox: { + topLeft: [3224, 3955, 944], + width: 1, + height: 100, + depth: 12, + }, + }, + hasUpdatedBoundingBox: true, + hasUpdatedName: false, + hasUpdatedColor: false, + }, + }, + ], + stats: { + "4bb38dd3-fa3a-4034-9608-beedb2ecfc10": { + segmentCount: 4, + }, + "f33b5d13-3b4a-4e98-abfc-64bf7ca82002": { + treeCount: 6, + nodeCount: 28, + edgeCount: 23, + branchPointCount: 0, + }, + }, + info: '["INITIALIZE_CONNECTOME_TRACING","SET_TD_CAMERA_WITHOUT_TIME_TRACKING * 5","SET_INPUT_CATCHER_RECTS","SET_TD_CAMERA_WITHOUT_TIME_TRACKING","UPDATE_CONNECTOME_FILE_LIST","SET_MAXIMUM_ZOOM_FOR_ALL_MAGS_FOR_LAYER * 4","SET_TOOL","CHANGE_USER_BOUNDING_BOX * 2","SET_TD_CAMERA_WITHOUT_TIME_TRACKING","CHANGE_USER_BOUNDING_BOX * 3"]', + }, + { + version: -1, + transactionId: "1clexor0ve", + transactionGroupCount: 1, + transactionGroupIndex: 0, + timestamp: 1747994347637, + authorId: "123", + actions: [ + { + name: "updateUserBoundingBoxInVolumeTracing", + value: { + boundingBoxId: 3, + actionTracingId: "4bb38dd3-fa3a-4034-9608-beedb2ecfc10", + updatedProps: { + boundingBox: { + topLeft: [3224, 3955, 944], + width: 1, + height: 92, + depth: 12, + }, + }, + hasUpdatedBoundingBox: true, + hasUpdatedName: false, + hasUpdatedColor: false, + }, + }, + ], + stats: { + "4bb38dd3-fa3a-4034-9608-beedb2ecfc10": { + segmentCount: 4, + }, + "f33b5d13-3b4a-4e98-abfc-64bf7ca82002": { + treeCount: 6, + nodeCount: 28, + edgeCount: 23, + branchPointCount: 0, + }, + }, + info: '["INITIALIZE_CONNECTOME_TRACING","SET_TD_CAMERA_WITHOUT_TIME_TRACKING * 5","SET_INPUT_CATCHER_RECTS","SET_TD_CAMERA_WITHOUT_TIME_TRACKING","UPDATE_CONNECTOME_FILE_LIST","SET_MAXIMUM_ZOOM_FOR_ALL_MAGS_FOR_LAYER * 4","SET_TOOL","CHANGE_USER_BOUNDING_BOX * 2","SET_TD_CAMERA_WITHOUT_TIME_TRACKING","CHANGE_USER_BOUNDING_BOX * 3"]', + }, + ]; + + const compactedActions = removeSubsequentUpdateBBoxActions(actions); + expect(compactedActions).toHaveLength(2); + const latestSkeletonTracingAction = compactedActions.find( + (action) => action.actions[0].name === "updateUserBoundingBoxInSkeletonTracing", + ); + expect(latestSkeletonTracingAction).toBeDefined(); + expect(latestSkeletonTracingAction?.actions[0].value.updatedProps.boundingBox).toEqual({ + topLeft: [3224, 3955, 944], + width: 1, + height: 100, + depth: 12, + }); + }); +}); diff --git a/frontend/javascripts/viewer/model/helpers/compaction/compact_save_queue.ts b/frontend/javascripts/viewer/model/helpers/compaction/compact_save_queue.ts index 6f17f6e007f..52ee8f88f5d 100644 --- a/frontend/javascripts/viewer/model/helpers/compaction/compact_save_queue.ts +++ b/frontend/javascripts/viewer/model/helpers/compaction/compact_save_queue.ts @@ -1,4 +1,8 @@ import _ from "lodash"; +import type { + UpdateUserBoundingBoxInSkeletonTracingAction, + UpdateUserBoundingBoxInVolumeTracingAction, +} from "viewer/model/sagas/update_actions"; import type { SaveQueueEntry } from "viewer/store"; function removeAllButLastUpdateTracingAction(updateActionsBatches: Array) { @@ -70,32 +74,43 @@ function removeSubsequentUpdateNodeActions(updateActionsBatches: Array) { - const obsoleteUpdateActions = []; - +export function removeSubsequentUpdateBBoxActions(updateActionsBatches: Array) { // Actions are obsolete, if they are for the same bounding box and for the same prop. // E.g. when rezising a bounding box, multiple updateActions are sent during the resize, while only the last one is needed. - // The given action is always compared to the next next one, as usually an - // updateUserBoundingBoxInSkeletonTracingAction and updateUserBoundingBoxInVolumeTracingAction - // is sent at the same time. - for (let i = 0; i < updateActionsBatches.length - 2; i++) { - const actions1 = updateActionsBatches[i].actions; - const actions2 = updateActionsBatches[i + 2].actions; - + const previousActionsById: Record< + string, + UpdateUserBoundingBoxInSkeletonTracingAction | UpdateUserBoundingBoxInVolumeTracingAction + > = {}; + const relevantActions = []; + for (let i = updateActionsBatches.length - 1; i >= 0; i--) { + const currentActions = updateActionsBatches[i].actions; if ( - actions1.length === 1 && - actions2.length === 1 && - (actions1[0].name === "updateUserBoundingBoxInSkeletonTracing" || - actions1[0].name === "updateUserBoundingBoxInVolumeTracing") && - actions1[0].name === actions2[0].name && - actions1[0].value.boundingBoxId === actions2[0].value.boundingBoxId && - _.isEqual(actions1[0].value.updatedPropKeys, actions2[0].value.updatedPropKeys) + currentActions.length === 1 && + ["updateUserBoundingBoxInSkeletonTracing", "updateUserBoundingBoxInVolumeTracing"].includes( + currentActions[0].name, + ) ) { - obsoleteUpdateActions.push(updateActionsBatches[i]); + const currentAction = currentActions[0] as + | UpdateUserBoundingBoxInSkeletonTracingAction + | UpdateUserBoundingBoxInVolumeTracingAction; + const previousActionForTracing = previousActionsById[currentAction.value.actionTracingId]; + if ( + previousActionForTracing != null && + previousActionForTracing.name === currentAction.name && + previousActionForTracing.value.boundingBoxId === currentAction.value.boundingBoxId && + previousActionForTracing.value.hasUpdatedColor === currentAction.value.hasUpdatedColor && + previousActionForTracing.value.hasUpdatedName === currentAction.value.hasUpdatedName && + previousActionForTracing.value.hasUpdatedBoundingBox === + currentAction.value.hasUpdatedBoundingBox + ) { + relevantActions.unshift(updateActionsBatches[i]); + } + previousActionsById[currentAction.value.actionTracingId] = currentAction; + } else { + relevantActions.unshift(updateActionsBatches[i]); } } - - return _.without(updateActionsBatches, ...obsoleteUpdateActions); + return relevantActions; } function removeSubsequentUpdateSegmentActions(updateActionsBatches: Array) { diff --git a/frontend/javascripts/viewer/model/sagas/update_actions.ts b/frontend/javascripts/viewer/model/sagas/update_actions.ts index c85634e1b9e..7437e9d824a 100644 --- a/frontend/javascripts/viewer/model/sagas/update_actions.ts +++ b/frontend/javascripts/viewer/model/sagas/update_actions.ts @@ -509,7 +509,7 @@ function getUpdateUserBoundingBox( updatedProps: updatedPropsForServer, hasUpdatedBoundingBox: updatedPropsKeys.includes("boundingBox"), hasUpdatedName: updatedPropsKeys.includes("name"), - haUpdatedColor: updatedPropsKeys.includes("color"), + hasUpdatedColor: updatedPropsKeys.includes("color"), }, } as const; } From 3a21f41e5af716d1cf002086dc431ec8757c8fc9 Mon Sep 17 00:00:00 2001 From: Charlie Meister Date: Fri, 23 May 2025 16:29:56 +0200 Subject: [PATCH 27/41] fix test --- .../test/model/compact_save_queue.spec.ts | 290 +++++++++++++++++- .../helpers/compaction/compact_save_queue.ts | 12 +- 2 files changed, 282 insertions(+), 20 deletions(-) diff --git a/frontend/javascripts/test/model/compact_save_queue.spec.ts b/frontend/javascripts/test/model/compact_save_queue.spec.ts index 6f4c33a9876..ebbd52a0adc 100644 --- a/frontend/javascripts/test/model/compact_save_queue.spec.ts +++ b/frontend/javascripts/test/model/compact_save_queue.spec.ts @@ -2,7 +2,7 @@ import { removeSubsequentUpdateBBoxActions } from "viewer/model/helpers/compacti import { describe, expect, it } from "vitest"; describe("Compact Save Queue", () => { - it("UpdateUserBoundingBoxActions should be compacted", () => { + it("UpdateUserBoundingBoxActions of the same tracing should be compacted", () => { const actions = [ { version: -1, @@ -13,10 +13,10 @@ describe("Compact Save Queue", () => { authorId: "123", actions: [ { - name: "updateUserBoundingBoxInSkeletonTracing", + name: "updateUserBoundingBoxInVolumeTracing", value: { boundingBoxId: 3, - actionTracingId: "f33b5d13-3b4a-4e98-abfc-64bf7ca82002", + actionTracingId: "volumeTracing1", updatedProps: { boundingBox: { topLeft: [3224, 3955, 944], @@ -32,10 +32,10 @@ describe("Compact Save Queue", () => { }, ], stats: { - "4bb38dd3-fa3a-4034-9608-beedb2ecfc10": { + volumeTracing1: { segmentCount: 4, }, - "f33b5d13-3b4a-4e98-abfc-64bf7ca82002": { + skeletonTracing1: { treeCount: 6, nodeCount: 28, edgeCount: 23, @@ -53,10 +53,10 @@ describe("Compact Save Queue", () => { authorId: "123", actions: [ { - name: "updateUserBoundingBoxInSkeletonTracing", + name: "updateUserBoundingBoxInVolumeTracing", value: { boundingBoxId: 3, - actionTracingId: "f33b5d13-3b4a-4e98-abfc-64bf7ca82002", + actionTracingId: "volumeTracing1", updatedProps: { boundingBox: { topLeft: [3224, 3955, 944], @@ -72,10 +72,10 @@ describe("Compact Save Queue", () => { }, ], stats: { - "4bb38dd3-fa3a-4034-9608-beedb2ecfc10": { + volumeTracing1: { segmentCount: 4, }, - "f33b5d13-3b4a-4e98-abfc-64bf7ca82002": { + skeletonTracing1: { treeCount: 6, nodeCount: 28, edgeCount: 23, @@ -96,7 +96,7 @@ describe("Compact Save Queue", () => { name: "updateUserBoundingBoxInVolumeTracing", value: { boundingBoxId: 3, - actionTracingId: "4bb38dd3-fa3a-4034-9608-beedb2ecfc10", + actionTracingId: "volumeTracing1", updatedProps: { boundingBox: { topLeft: [3224, 3955, 944], @@ -112,10 +112,130 @@ describe("Compact Save Queue", () => { }, ], stats: { - "4bb38dd3-fa3a-4034-9608-beedb2ecfc10": { + volumeTracing1: { segmentCount: 4, }, - "f33b5d13-3b4a-4e98-abfc-64bf7ca82002": { + skeletonTracing1: { + treeCount: 6, + nodeCount: 28, + edgeCount: 23, + branchPointCount: 0, + }, + }, + info: '["INITIALIZE_CONNECTOME_TRACING","SET_TD_CAMERA_WITHOUT_TIME_TRACKING * 5","SET_INPUT_CATCHER_RECTS","SET_TD_CAMERA_WITHOUT_TIME_TRACKING","UPDATE_CONNECTOME_FILE_LIST","SET_MAXIMUM_ZOOM_FOR_ALL_MAGS_FOR_LAYER * 4","SET_TOOL","CHANGE_USER_BOUNDING_BOX * 2","SET_TD_CAMERA_WITHOUT_TIME_TRACKING","CHANGE_USER_BOUNDING_BOX * 3"]', + }, + { + version: -1, + transactionId: "eq3scfgvoa", + transactionGroupCount: 1, + transactionGroupIndex: 0, + timestamp: 1747994347590, + authorId: "123", + actions: [ + { + name: "updateUserBoundingBoxInVolumeTracing", + value: { + boundingBoxId: 3, + actionTracingId: "skeletonTracing1", + updatedProps: { + boundingBox: { + topLeft: [322, 3955, 944], + width: 1, + height: 157, + depth: 12, + }, + }, + hasUpdatedBoundingBox: true, + hasUpdatedName: false, + hasUpdatedColor: false, + }, + }, + ], + stats: { + volumeTracing1: { + segmentCount: 4, + }, + skeletonTracing1: { + treeCount: 6, + nodeCount: 28, + edgeCount: 23, + branchPointCount: 0, + }, + }, + info: '["INITIALIZE_CONNECTOME_TRACING","SET_TD_CAMERA_WITHOUT_TIME_TRACKING * 5","SET_INPUT_CATCHER_RECTS","SET_TD_CAMERA_WITHOUT_TIME_TRACKING","UPDATE_CONNECTOME_FILE_LIST","SET_MAXIMUM_ZOOM_FOR_ALL_MAGS_FOR_LAYER * 4","SET_TOOL","CHANGE_USER_BOUNDING_BOX * 2","SET_TD_CAMERA_WITHOUT_TIME_TRACKING","CHANGE_USER_BOUNDING_BOX * 3"]', + }, + { + version: -1, + transactionId: "msk6wwebqf", + transactionGroupCount: 1, + transactionGroupIndex: 0, + timestamp: 1747994347633, + authorId: "123", + actions: [ + { + name: "updateUserBoundingBoxInVolumeTracing", + value: { + boundingBoxId: 3, + actionTracingId: "skeletonTracing1", + updatedProps: { + boundingBox: { + topLeft: [3224, 395, 944], + width: 1, + height: 100, + depth: 12, + }, + }, + hasUpdatedBoundingBox: true, + hasUpdatedName: false, + hasUpdatedColor: false, + }, + }, + ], + stats: { + volumeTracing1: { + segmentCount: 4, + }, + skeletonTracing1: { + treeCount: 6, + nodeCount: 28, + edgeCount: 23, + branchPointCount: 0, + }, + }, + info: '["INITIALIZE_CONNECTOME_TRACING","SET_TD_CAMERA_WITHOUT_TIME_TRACKING * 5","SET_INPUT_CATCHER_RECTS","SET_TD_CAMERA_WITHOUT_TIME_TRACKING","UPDATE_CONNECTOME_FILE_LIST","SET_MAXIMUM_ZOOM_FOR_ALL_MAGS_FOR_LAYER * 4","SET_TOOL","CHANGE_USER_BOUNDING_BOX * 2","SET_TD_CAMERA_WITHOUT_TIME_TRACKING","CHANGE_USER_BOUNDING_BOX * 3"]', + }, + { + version: -1, + transactionId: "1clexor0ve", + transactionGroupCount: 1, + transactionGroupIndex: 0, + timestamp: 1747994347637, + authorId: "123", + actions: [ + { + name: "updateUserBoundingBoxInVolumeTracing", + value: { + boundingBoxId: 3, + actionTracingId: "skeletonTracing1", + updatedProps: { + boundingBox: { + topLeft: [3224, 3955, 900], + width: 1, + height: 92, + depth: 12, + }, + }, + hasUpdatedBoundingBox: true, + hasUpdatedName: false, + hasUpdatedColor: false, + }, + }, + ], + stats: { + volumeTracing1: { + segmentCount: 4, + }, + skeletonTracing1: { treeCount: 6, nodeCount: 28, edgeCount: 23, @@ -125,18 +245,160 @@ describe("Compact Save Queue", () => { info: '["INITIALIZE_CONNECTOME_TRACING","SET_TD_CAMERA_WITHOUT_TIME_TRACKING * 5","SET_INPUT_CATCHER_RECTS","SET_TD_CAMERA_WITHOUT_TIME_TRACKING","UPDATE_CONNECTOME_FILE_LIST","SET_MAXIMUM_ZOOM_FOR_ALL_MAGS_FOR_LAYER * 4","SET_TOOL","CHANGE_USER_BOUNDING_BOX * 2","SET_TD_CAMERA_WITHOUT_TIME_TRACKING","CHANGE_USER_BOUNDING_BOX * 3"]', }, ]; - + // @ts-ignore const compactedActions = removeSubsequentUpdateBBoxActions(actions); expect(compactedActions).toHaveLength(2); const latestSkeletonTracingAction = compactedActions.find( (action) => action.actions[0].name === "updateUserBoundingBoxInSkeletonTracing", ); expect(latestSkeletonTracingAction).toBeDefined(); - expect(latestSkeletonTracingAction?.actions[0].value.updatedProps.boundingBox).toEqual({ + const volumeActionValue = latestSkeletonTracingAction?.actions[0].value; + expect(volumeActionValue).not.toBeNull(); + volumeActionValue; + expect(volumeActionValue).toHaveProperty("updatedProps"); + // @ts-ignore + expect(volumeActionValue.updatedProps.boundingBox).toEqual({ topLeft: [3224, 3955, 944], width: 1, height: 100, depth: 12, }); + const latestVolumeTracingAction = compactedActions.find( + (action) => action.actions[0].name === "updateUserBoundingBoxInSkeletonTracing", + ); + expect(latestVolumeTracingAction).toBeDefined(); + const skeletonActionValue = latestVolumeTracingAction?.actions[0].value; + expect(skeletonActionValue).not.toBeNull(); + expect(skeletonActionValue).toHaveProperty("updatedProps"); + // @ts-ignore + expect(skeletonActionValue.updatedProps.boundingBox).toEqual({ + topLeft: [3224, 3955, 900], + width: 1, + height: 92, + depth: 12, + }); + }); + + it("UpdateUserBoundingBoxActions should be not compacted for different props", () => { + const actions = [ + { + version: -1, + transactionId: "eq3scfgvoa", + transactionGroupCount: 1, + transactionGroupIndex: 0, + timestamp: 1747994347590, + authorId: "123", + actions: [ + { + name: "updateUserBoundingBoxInVolumeTracing", + value: { + boundingBoxId: 3, + actionTracingId: "volumeTracing1", + updatedProps: { + name: "test1", + }, + hasUpdatedBoundingBox: false, + hasUpdatedName: true, + hasUpdatedColor: false, + }, + }, + ], + stats: { + volumeTracing1: { + segmentCount: 4, + }, + skeletonTracing1: { + treeCount: 6, + nodeCount: 28, + edgeCount: 23, + branchPointCount: 0, + }, + }, + info: '["INITIALIZE_CONNECTOME_TRACING","SET_TD_CAMERA_WITHOUT_TIME_TRACKING * 5","SET_INPUT_CATCHER_RECTS","SET_TD_CAMERA_WITHOUT_TIME_TRACKING","UPDATE_CONNECTOME_FILE_LIST","SET_MAXIMUM_ZOOM_FOR_ALL_MAGS_FOR_LAYER * 4","SET_TOOL","CHANGE_USER_BOUNDING_BOX * 2","SET_TD_CAMERA_WITHOUT_TIME_TRACKING","CHANGE_USER_BOUNDING_BOX * 3"]', + }, + { + version: -1, + transactionId: "msk6wwebqf", + transactionGroupCount: 1, + transactionGroupIndex: 0, + timestamp: 1747994347633, + authorId: "123", + actions: [ + { + name: "updateUserBoundingBoxInVolumeTracing", + value: { + boundingBoxId: 3, + actionTracingId: "volumeTracing1", + updatedProps: { + boundingBox: { + topLeft: [3224, 3955, 944], + width: 1, + height: 100, + depth: 12, + }, + }, + hasUpdatedBoundingBox: true, + hasUpdatedName: false, + hasUpdatedColor: false, + }, + }, + ], + stats: { + volumeTracing1: { + segmentCount: 4, + }, + skeletonTracing1: { + treeCount: 6, + nodeCount: 28, + edgeCount: 23, + branchPointCount: 0, + }, + }, + info: '["INITIALIZE_CONNECTOME_TRACING","SET_TD_CAMERA_WITHOUT_TIME_TRACKING * 5","SET_INPUT_CATCHER_RECTS","SET_TD_CAMERA_WITHOUT_TIME_TRACKING","UPDATE_CONNECTOME_FILE_LIST","SET_MAXIMUM_ZOOM_FOR_ALL_MAGS_FOR_LAYER * 4","SET_TOOL","CHANGE_USER_BOUNDING_BOX * 2","SET_TD_CAMERA_WITHOUT_TIME_TRACKING","CHANGE_USER_BOUNDING_BOX * 3"]', + }, + { + version: -1, + transactionId: "1clexor0ve", + transactionGroupCount: 1, + transactionGroupIndex: 0, + timestamp: 1747994347637, + authorId: "123", + actions: [ + { + name: "updateUserBoundingBoxInVolumeTracing", + value: { + boundingBoxId: 3, + actionTracingId: "volumeTracing1", + updatedProps: { + boundingBox: { + topLeft: [3224, 3955, 944], + width: 1, + height: 92, + depth: 12, + }, + }, + hasUpdatedBoundingBox: true, + hasUpdatedName: false, + hasUpdatedColor: false, + }, + }, + ], + stats: { + volumeTracing1: { + segmentCount: 4, + }, + skeletonTracing1: { + treeCount: 6, + nodeCount: 28, + edgeCount: 23, + branchPointCount: 0, + }, + }, + info: '["INITIALIZE_CONNECTOME_TRACING","SET_TD_CAMERA_WITHOUT_TIME_TRACKING * 5","SET_INPUT_CATCHER_RECTS","SET_TD_CAMERA_WITHOUT_TIME_TRACKING","UPDATE_CONNECTOME_FILE_LIST","SET_MAXIMUM_ZOOM_FOR_ALL_MAGS_FOR_LAYER * 4","SET_TOOL","CHANGE_USER_BOUNDING_BOX * 2","SET_TD_CAMERA_WITHOUT_TIME_TRACKING","CHANGE_USER_BOUNDING_BOX * 3"]', + }, + ]; + + const compactedActions = removeSubsequentUpdateBBoxActions(actions); + expect(compactedActions).toHaveLength(2); }); }); diff --git a/frontend/javascripts/viewer/model/helpers/compaction/compact_save_queue.ts b/frontend/javascripts/viewer/model/helpers/compaction/compact_save_queue.ts index 52ee8f88f5d..457323b624d 100644 --- a/frontend/javascripts/viewer/model/helpers/compaction/compact_save_queue.ts +++ b/frontend/javascripts/viewer/model/helpers/compaction/compact_save_queue.ts @@ -95,12 +95,12 @@ export function removeSubsequentUpdateBBoxActions(updateActionsBatches: Array Date: Fri, 23 May 2025 17:20:03 +0200 Subject: [PATCH 28/41] frontend: make types in test prettier --- .../javascripts/test/model/compact_save_queue.spec.ts | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/frontend/javascripts/test/model/compact_save_queue.spec.ts b/frontend/javascripts/test/model/compact_save_queue.spec.ts index ebbd52a0adc..cb0f6fb5f7b 100644 --- a/frontend/javascripts/test/model/compact_save_queue.spec.ts +++ b/frontend/javascripts/test/model/compact_save_queue.spec.ts @@ -1,9 +1,10 @@ import { removeSubsequentUpdateBBoxActions } from "viewer/model/helpers/compaction/compact_save_queue"; +import type { SaveQueueEntry } from "viewer/store"; import { describe, expect, it } from "vitest"; describe("Compact Save Queue", () => { it("UpdateUserBoundingBoxActions of the same tracing should be compacted", () => { - const actions = [ + const actions: SaveQueueEntry[] = [ { version: -1, transactionId: "eq3scfgvoa", @@ -245,7 +246,6 @@ describe("Compact Save Queue", () => { info: '["INITIALIZE_CONNECTOME_TRACING","SET_TD_CAMERA_WITHOUT_TIME_TRACKING * 5","SET_INPUT_CATCHER_RECTS","SET_TD_CAMERA_WITHOUT_TIME_TRACKING","UPDATE_CONNECTOME_FILE_LIST","SET_MAXIMUM_ZOOM_FOR_ALL_MAGS_FOR_LAYER * 4","SET_TOOL","CHANGE_USER_BOUNDING_BOX * 2","SET_TD_CAMERA_WITHOUT_TIME_TRACKING","CHANGE_USER_BOUNDING_BOX * 3"]', }, ]; - // @ts-ignore const compactedActions = removeSubsequentUpdateBBoxActions(actions); expect(compactedActions).toHaveLength(2); const latestSkeletonTracingAction = compactedActions.find( @@ -254,9 +254,8 @@ describe("Compact Save Queue", () => { expect(latestSkeletonTracingAction).toBeDefined(); const volumeActionValue = latestSkeletonTracingAction?.actions[0].value; expect(volumeActionValue).not.toBeNull(); - volumeActionValue; expect(volumeActionValue).toHaveProperty("updatedProps"); - // @ts-ignore + // @ts-ignore we checked before that voluemActionValue is not null and that is has the property updatedProps expect(volumeActionValue.updatedProps.boundingBox).toEqual({ topLeft: [3224, 3955, 944], width: 1, @@ -270,7 +269,7 @@ describe("Compact Save Queue", () => { const skeletonActionValue = latestVolumeTracingAction?.actions[0].value; expect(skeletonActionValue).not.toBeNull(); expect(skeletonActionValue).toHaveProperty("updatedProps"); - // @ts-ignore + // @ts-ignore checked before expect(skeletonActionValue.updatedProps.boundingBox).toEqual({ topLeft: [3224, 3955, 900], width: 1, @@ -280,7 +279,7 @@ describe("Compact Save Queue", () => { }); it("UpdateUserBoundingBoxActions should be not compacted for different props", () => { - const actions = [ + const actions: SaveQueueEntry[] = [ { version: -1, transactionId: "eq3scfgvoa", From 6f105971cebd09351d131b446518f87a761117b1 Mon Sep 17 00:00:00 2001 From: Florian M Date: Mon, 26 May 2025 10:28:56 +0200 Subject: [PATCH 29/41] add unit tests for TristateJsonTestSuite --- test/backend/TristateJsonTestSuite.scala | 61 +++++++++++++++++++ .../util/tools/TristateOptionJsonHelper.scala | 2 + 2 files changed, 63 insertions(+) create mode 100644 test/backend/TristateJsonTestSuite.scala diff --git a/test/backend/TristateJsonTestSuite.scala b/test/backend/TristateJsonTestSuite.scala new file mode 100644 index 00000000000..a5d009f9153 --- /dev/null +++ b/test/backend/TristateJsonTestSuite.scala @@ -0,0 +1,61 @@ +package backend + +import com.scalableminds.util.tools.{JsonHelper, TristateOptionJsonHelper} +import org.scalatestplus.play.PlaySpec +import play.api.libs.json.{Json, OFormat} + +class TristateJsonTestSuite extends PlaySpec { + + case class ExampleClass( + requiredKey: String, + optionalKey: Option[String], + tristateOptionalKey: Option[Option[String]] = Some(None) + ) + + object ExampleClass extends TristateOptionJsonHelper { + implicit val jsonFormat: OFormat[ExampleClass] = + Json.configured(tristateOptionParsing).format[ExampleClass] + } + + "TristateJsonFormat" should { + "parse the keys correctly if all are present" in { + val jsonString = """{"requiredKey": "a", "optionalKey": "b", "tristateOptionalKey": "c"}""" + val validatedBox = JsonHelper.parseAs[ExampleClass](jsonString) + assert(validatedBox.isDefined) + validatedBox.foreach { validated => + assert(validated.optionalKey.isDefined) + assert(validated.tristateOptionalKey.isDefined) + assert(validated.tristateOptionalKey.contains(Some("c"))) + } + } + "parse the keys correctly if optional and tristateOptional are absent" in { + val jsonString = """{"requiredKey": "a"}""" + val validatedBox = JsonHelper.parseAs[ExampleClass](jsonString) + assert(validatedBox.isDefined) + validatedBox.foreach { validated => + assert(validated.optionalKey.isEmpty) + assert(validated.tristateOptionalKey.isEmpty) + } + } + "parse the keys correctly if optional and tristateOptional are null" in { + val jsonString = """{"requiredKey": "a", "optionalKey": null, "tristateOptionalKey": null}""" + val validatedBox = JsonHelper.parseAs[ExampleClass](jsonString) + assert(validatedBox.isDefined) + validatedBox.foreach { validated => + assert(validated.optionalKey.isEmpty) + assert(validated.tristateOptionalKey.isDefined) + assert(validated.tristateOptionalKey.contains(None)) + } + } + "in writing, write null for Some(None)" in { + val value = ExampleClass("a", None, Some(None)) + val jsonString = Json.stringify(Json.toJson(value)) + assert(jsonString == """{"requiredKey":"a","tristateOptionalKey":null}""") + } + "in writing, skip key for None" in { + val value = ExampleClass("a", None, None) + val jsonString = Json.stringify(Json.toJson(value)) + assert(jsonString == """{"requiredKey":"a"}""") + } + } +} diff --git a/util/src/main/scala/com/scalableminds/util/tools/TristateOptionJsonHelper.scala b/util/src/main/scala/com/scalableminds/util/tools/TristateOptionJsonHelper.scala index 04dc8b432c3..57c211ba7b4 100644 --- a/util/src/main/scala/com/scalableminds/util/tools/TristateOptionJsonHelper.scala +++ b/util/src/main/scala/com/scalableminds/util/tools/TristateOptionJsonHelper.scala @@ -4,6 +4,8 @@ import play.api.libs.json.Json.WithDefaultValues import play.api.libs.json.JsonConfiguration.Aux import play.api.libs.json._ +// Allows a case class json format that distinguishes between absent keys and keys with the value null +// See TristateJsonTestSuite for a usage example. The Some(None) defualt must by set for the case class trait TristateOptionJsonHelper { implicit protected def optionFormat[T: Format]: Format[Option[T]] = new Format[Option[T]] { From 04b7d7f864db07ea64d974508e243c889960fb3c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20B=C3=BC=C3=9Femeyer?= <39529669+MichaelBuessemeyer@users.noreply.github.com> Date: Mon, 26 May 2025 11:48:15 +0200 Subject: [PATCH 30/41] change backend bbox update action to use new optional field parsing --- .../updating/SkeletonUpdateActions.scala | 29 +++++++++---------- .../tracings/volume/VolumeUpdateActions.scala | 29 +++++++++---------- 2 files changed, 28 insertions(+), 30 deletions(-) diff --git a/webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/tracings/skeleton/updating/SkeletonUpdateActions.scala b/webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/tracings/skeleton/updating/SkeletonUpdateActions.scala index 339859c2d4a..5ed78e3c7a6 100644 --- a/webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/tracings/skeleton/updating/SkeletonUpdateActions.scala +++ b/webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/tracings/skeleton/updating/SkeletonUpdateActions.scala @@ -1,7 +1,9 @@ package com.scalableminds.webknossos.tracingstore.tracings.skeleton.updating import com.scalableminds.webknossos.tracingstore.tracings._ -import com.scalableminds.util.geometry.{Vec3Double, Vec3Int} +import com.scalableminds.util.geometry.{BoundingBox, Vec3Double, Vec3Int} +import com.scalableminds.util.image.Color +import com.scalableminds.util.tools.TristateOptionJsonHelper import com.scalableminds.webknossos.datastore.SkeletonTracing.{Edge, Node, SkeletonTracing, Tree, TreeGroup} import com.scalableminds.webknossos.datastore.helpers.{NodeDefaults, ProtoGeometryImplicits} import com.scalableminds.webknossos.datastore.models.AdditionalCoordinate @@ -576,28 +578,25 @@ case class DeleteUserBoundingBoxSkeletonAction(boundingBoxId: Int, } case class UpdateUserBoundingBoxSkeletonAction(boundingBoxId: Int, - updatedPropKeys: List[String], - updatedProps: NamedBoundingBoxUpdate, + name: Option[Option[String]], + color: Option[Option[Color]], + boundingBox: Option[Option[BoundingBox]], actionTracingId: String, actionTimestamp: Option[Long] = None, actionAuthorId: Option[String] = None, info: Option[String] = None) - extends SkeletonUpdateAction { + extends SkeletonUpdateAction + with ProtoGeometryImplicits { override def applyOn(tracing: SkeletonTracing): SkeletonTracing = { def updateUserBoundingBoxes() = tracing.userBoundingBoxes.map { currentBoundingBox => if (boundingBoxId == currentBoundingBox.id) { currentBoundingBox.copy( - id = updatedProps.id.getOrElse(currentBoundingBox.id), - name = - if (updatedPropKeys.contains("name") && updatedProps.name.isDefined) updatedProps.name - else currentBoundingBox.name, - color = - if (updatedPropKeys.contains("color") && updatedProps.color.isDefined) updatedProps.colorOptProto - else currentBoundingBox.color, + name = name.getOrElse(currentBoundingBox.name), + color = if (color.isDefined) color.flatMap(colorOptToProto) else currentBoundingBox.color, boundingBox = - if (updatedPropKeys.contains("boundingBox")) - updatedProps.boundingBoxProto.getOrElse(currentBoundingBox.boundingBox) + if (boundingBox.isDefined) + boundingBox.flatMap(boundingBoxOptToProto).getOrElse(currentBoundingBox.boundingBox) else currentBoundingBox.boundingBox ) } else @@ -704,9 +703,9 @@ object DeleteUserBoundingBoxSkeletonAction { implicit val jsonFormat: OFormat[DeleteUserBoundingBoxSkeletonAction] = Json.format[DeleteUserBoundingBoxSkeletonAction] } -object UpdateUserBoundingBoxSkeletonAction { +object UpdateUserBoundingBoxSkeletonAction extends TristateOptionJsonHelper { implicit val jsonFormat: OFormat[UpdateUserBoundingBoxSkeletonAction] = - Json.format[UpdateUserBoundingBoxSkeletonAction] + Json.configured(tristateOptionParsing).format[UpdateUserBoundingBoxSkeletonAction] } object UpdateUserBoundingBoxVisibilitySkeletonAction { implicit val jsonFormat: OFormat[UpdateUserBoundingBoxVisibilitySkeletonAction] = diff --git a/webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/tracings/volume/VolumeUpdateActions.scala b/webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/tracings/volume/VolumeUpdateActions.scala index 7265469ad3c..587a9bc3096 100644 --- a/webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/tracings/volume/VolumeUpdateActions.scala +++ b/webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/tracings/volume/VolumeUpdateActions.scala @@ -1,6 +1,8 @@ package com.scalableminds.webknossos.tracingstore.tracings.volume -import com.scalableminds.util.geometry.{Vec3Double, Vec3Int} +import com.scalableminds.util.geometry.{BoundingBox, Vec3Double, Vec3Int} +import com.scalableminds.util.image.Color +import com.scalableminds.util.tools.TristateOptionJsonHelper import com.scalableminds.webknossos.datastore.VolumeTracing.{Segment, SegmentGroup, VolumeTracing} import com.scalableminds.webknossos.datastore.geometry.NamedBoundingBoxProto import com.scalableminds.webknossos.datastore.helpers.ProtoGeometryImplicits @@ -154,28 +156,25 @@ case class DeleteUserBoundingBoxVolumeAction(boundingBoxId: Int, } case class UpdateUserBoundingBoxVolumeAction(boundingBoxId: Int, - updatedPropKeys: List[String], - updatedProps: NamedBoundingBoxUpdate, + name: Option[Option[String]], + color: Option[Option[Color]], + boundingBox: Option[Option[BoundingBox]], actionTracingId: String, actionTimestamp: Option[Long] = None, actionAuthorId: Option[String] = None, info: Option[String] = None) - extends ApplyableVolumeUpdateAction { + extends ApplyableVolumeUpdateAction + with ProtoGeometryImplicits { override def applyOn(tracing: VolumeTracing): VolumeTracing = { def updateUserBoundingBoxes() = tracing.userBoundingBoxes.map { currentBoundingBox => if (boundingBoxId == currentBoundingBox.id) { currentBoundingBox.copy( - id = updatedProps.id.getOrElse(currentBoundingBox.id), - name = - if (updatedPropKeys.contains("name") && updatedProps.name.isDefined) updatedProps.name - else currentBoundingBox.name, - color = - if (updatedPropKeys.contains("color") && updatedProps.color.isDefined) updatedProps.colorOptProto - else currentBoundingBox.color, + name = name.getOrElse(currentBoundingBox.name), + color = if (color.isDefined) color.flatMap(colorOptToProto) else currentBoundingBox.color, boundingBox = - if (updatedPropKeys.contains("boundingBox")) - updatedProps.boundingBoxProto.getOrElse(currentBoundingBox.boundingBox) + if (boundingBox.isDefined) + boundingBox.flatMap(boundingBoxOptToProto).getOrElse(currentBoundingBox.boundingBox) else currentBoundingBox.boundingBox ) } else @@ -484,9 +483,9 @@ object DeleteUserBoundingBoxVolumeAction { implicit val jsonFormat: OFormat[DeleteUserBoundingBoxVolumeAction] = Json.format[DeleteUserBoundingBoxVolumeAction] } -object UpdateUserBoundingBoxVolumeAction { +object UpdateUserBoundingBoxVolumeAction extends TristateOptionJsonHelper { implicit val jsonFormat: OFormat[UpdateUserBoundingBoxVolumeAction] = - Json.format[UpdateUserBoundingBoxVolumeAction] + Json.configured(tristateOptionParsing).format[UpdateUserBoundingBoxVolumeAction] } object UpdateUserBoundingBoxVisibilityVolumeAction { implicit val jsonFormat: OFormat[UpdateUserBoundingBoxVisibilityVolumeAction] = From dd551784a0470a5c451b05a7cb44d042c997ae9f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20B=C3=BC=C3=9Femeyer?= <39529669+MichaelBuessemeyer@users.noreply.github.com> Date: Mon, 26 May 2025 12:56:01 +0200 Subject: [PATCH 31/41] remove unused code --- .../tracings/NamedBoundingBox.scala | 19 +------------------ .../tracings/volume/VolumeUpdateActions.scala | 2 +- 2 files changed, 2 insertions(+), 19 deletions(-) diff --git a/webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/tracings/NamedBoundingBox.scala b/webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/tracings/NamedBoundingBox.scala index 79a114d6ffc..526af171b67 100644 --- a/webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/tracings/NamedBoundingBox.scala +++ b/webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/tracings/NamedBoundingBox.scala @@ -3,11 +3,7 @@ package com.scalableminds.webknossos.tracingstore.tracings import play.api.libs.json.{Json, OFormat} import com.scalableminds.util.geometry.BoundingBox import com.scalableminds.util.image.Color -import com.scalableminds.webknossos.datastore.geometry.{ - BoundingBoxProto, - ColorProto, - NamedBoundingBoxProto => ProtoBoundingBox -} +import com.scalableminds.webknossos.datastore.geometry.{NamedBoundingBoxProto => ProtoBoundingBox} import com.scalableminds.webknossos.datastore.helpers.ProtoGeometryImplicits import com.scalableminds.webknossos.tracingstore.tracings.skeleton.updating.SkeletonUpdateActionHelper @@ -22,16 +18,3 @@ case class NamedBoundingBox(id: Int, } object NamedBoundingBox { implicit val jsonFormat: OFormat[NamedBoundingBox] = Json.format[NamedBoundingBox] } - -case class NamedBoundingBoxUpdate(id: Option[Int], - name: Option[String], - isVisible: Option[Boolean], - color: Option[Color], - boundingBox: Option[BoundingBox], -) extends ProtoGeometryImplicits { - def colorOptProto: Option[ColorProto] = colorOptToProto(color) - def boundingBoxProto: Option[BoundingBoxProto] = boundingBoxOptToProto(boundingBox) -} -object NamedBoundingBoxUpdate { - implicit val jsonFormat: OFormat[NamedBoundingBoxUpdate] = Json.format[NamedBoundingBoxUpdate] -} diff --git a/webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/tracings/volume/VolumeUpdateActions.scala b/webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/tracings/volume/VolumeUpdateActions.scala index 587a9bc3096..60eaa09aa71 100644 --- a/webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/tracings/volume/VolumeUpdateActions.scala +++ b/webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/tracings/volume/VolumeUpdateActions.scala @@ -8,7 +8,7 @@ import com.scalableminds.webknossos.datastore.geometry.NamedBoundingBoxProto import com.scalableminds.webknossos.datastore.helpers.ProtoGeometryImplicits import com.scalableminds.webknossos.datastore.models.{AdditionalCoordinate, BucketPosition} import com.scalableminds.webknossos.tracingstore.annotation.{LayerUpdateAction, UpdateAction} -import com.scalableminds.webknossos.tracingstore.tracings.{MetadataEntry, NamedBoundingBox, NamedBoundingBoxUpdate} +import com.scalableminds.webknossos.tracingstore.tracings.{MetadataEntry, NamedBoundingBox} import play.api.libs.json._ trait VolumeUpdateActionHelper { From c5ddd56abb0048e408ab321e01df5017db736513 Mon Sep 17 00:00:00 2001 From: Philipp Otto Date: Mon, 26 May 2025 13:40:39 +0200 Subject: [PATCH 32/41] fix tests --- .../test/model/compact_save_queue.spec.ts | 386 ++++++------------ .../helpers/compaction/compact_save_queue.ts | 16 +- .../viewer/model/sagas/update_actions.ts | 12 +- 3 files changed, 133 insertions(+), 281 deletions(-) diff --git a/frontend/javascripts/test/model/compact_save_queue.spec.ts b/frontend/javascripts/test/model/compact_save_queue.spec.ts index cb0f6fb5f7b..70f730b0a58 100644 --- a/frontend/javascripts/test/model/compact_save_queue.spec.ts +++ b/frontend/javascripts/test/model/compact_save_queue.spec.ts @@ -1,4 +1,10 @@ import { removeSubsequentUpdateBBoxActions } from "viewer/model/helpers/compaction/compact_save_queue"; +import { + updateUserBoundingBoxInSkeletonTracing, + type UpdateUserBoundingBoxInSkeletonTracingAction, + updateUserBoundingBoxInVolumeTracing, + type UpdateUserBoundingBoxInVolumeTracingAction, +} from "viewer/model/sagas/update_actions"; import type { SaveQueueEntry } from "viewer/store"; import { describe, expect, it } from "vitest"; @@ -13,37 +19,19 @@ describe("Compact Save Queue", () => { timestamp: 1747994347590, authorId: "123", actions: [ - { - name: "updateUserBoundingBoxInVolumeTracing", - value: { - boundingBoxId: 3, - actionTracingId: "volumeTracing1", - updatedProps: { - boundingBox: { - topLeft: [3224, 3955, 944], - width: 1, - height: 157, - depth: 12, - }, + updateUserBoundingBoxInVolumeTracing( + 3, + { + boundingBox: { + min: [3224, 3955, 944], + max: [3225, 4112, 956], }, - hasUpdatedBoundingBox: true, - hasUpdatedName: false, - hasUpdatedColor: false, }, - }, + "volumeTracing1", + ), ], - stats: { - volumeTracing1: { - segmentCount: 4, - }, - skeletonTracing1: { - treeCount: 6, - nodeCount: 28, - edgeCount: 23, - branchPointCount: 0, - }, - }, - info: '["INITIALIZE_CONNECTOME_TRACING","SET_TD_CAMERA_WITHOUT_TIME_TRACKING * 5","SET_INPUT_CATCHER_RECTS","SET_TD_CAMERA_WITHOUT_TIME_TRACKING","UPDATE_CONNECTOME_FILE_LIST","SET_MAXIMUM_ZOOM_FOR_ALL_MAGS_FOR_LAYER * 4","SET_TOOL","CHANGE_USER_BOUNDING_BOX * 2","SET_TD_CAMERA_WITHOUT_TIME_TRACKING","CHANGE_USER_BOUNDING_BOX * 3"]', + stats: undefined, + info: "", }, { version: -1, @@ -53,37 +41,19 @@ describe("Compact Save Queue", () => { timestamp: 1747994347633, authorId: "123", actions: [ - { - name: "updateUserBoundingBoxInVolumeTracing", - value: { - boundingBoxId: 3, - actionTracingId: "volumeTracing1", - updatedProps: { - boundingBox: { - topLeft: [3224, 3955, 944], - width: 1, - height: 100, - depth: 12, - }, + updateUserBoundingBoxInVolumeTracing( + 3, + { + boundingBox: { + min: [3224, 3955, 944], + max: [3225, 4055, 956], }, - hasUpdatedBoundingBox: true, - hasUpdatedName: false, - hasUpdatedColor: false, }, - }, + "volumeTracing1", + ), ], - stats: { - volumeTracing1: { - segmentCount: 4, - }, - skeletonTracing1: { - treeCount: 6, - nodeCount: 28, - edgeCount: 23, - branchPointCount: 0, - }, - }, - info: '["INITIALIZE_CONNECTOME_TRACING","SET_TD_CAMERA_WITHOUT_TIME_TRACKING * 5","SET_INPUT_CATCHER_RECTS","SET_TD_CAMERA_WITHOUT_TIME_TRACKING","UPDATE_CONNECTOME_FILE_LIST","SET_MAXIMUM_ZOOM_FOR_ALL_MAGS_FOR_LAYER * 4","SET_TOOL","CHANGE_USER_BOUNDING_BOX * 2","SET_TD_CAMERA_WITHOUT_TIME_TRACKING","CHANGE_USER_BOUNDING_BOX * 3"]', + stats: undefined, + info: "", }, { version: -1, @@ -93,37 +63,19 @@ describe("Compact Save Queue", () => { timestamp: 1747994347637, authorId: "123", actions: [ - { - name: "updateUserBoundingBoxInVolumeTracing", - value: { - boundingBoxId: 3, - actionTracingId: "volumeTracing1", - updatedProps: { - boundingBox: { - topLeft: [3224, 3955, 944], - width: 1, - height: 92, - depth: 12, - }, + updateUserBoundingBoxInVolumeTracing( + 3, + { + boundingBox: { + min: [3224, 3955, 944], + max: [3225, 4047, 956], }, - hasUpdatedBoundingBox: true, - hasUpdatedName: false, - hasUpdatedColor: false, }, - }, + "volumeTracing1", + ), ], - stats: { - volumeTracing1: { - segmentCount: 4, - }, - skeletonTracing1: { - treeCount: 6, - nodeCount: 28, - edgeCount: 23, - branchPointCount: 0, - }, - }, - info: '["INITIALIZE_CONNECTOME_TRACING","SET_TD_CAMERA_WITHOUT_TIME_TRACKING * 5","SET_INPUT_CATCHER_RECTS","SET_TD_CAMERA_WITHOUT_TIME_TRACKING","UPDATE_CONNECTOME_FILE_LIST","SET_MAXIMUM_ZOOM_FOR_ALL_MAGS_FOR_LAYER * 4","SET_TOOL","CHANGE_USER_BOUNDING_BOX * 2","SET_TD_CAMERA_WITHOUT_TIME_TRACKING","CHANGE_USER_BOUNDING_BOX * 3"]', + stats: undefined, + info: "", }, { version: -1, @@ -133,37 +85,19 @@ describe("Compact Save Queue", () => { timestamp: 1747994347590, authorId: "123", actions: [ - { - name: "updateUserBoundingBoxInVolumeTracing", - value: { - boundingBoxId: 3, - actionTracingId: "skeletonTracing1", - updatedProps: { - boundingBox: { - topLeft: [322, 3955, 944], - width: 1, - height: 157, - depth: 12, - }, + updateUserBoundingBoxInSkeletonTracing( + 3, + { + boundingBox: { + min: [322, 3955, 944], + max: [323, 4112, 956], }, - hasUpdatedBoundingBox: true, - hasUpdatedName: false, - hasUpdatedColor: false, }, - }, + "skeletonTracing1", + ), ], - stats: { - volumeTracing1: { - segmentCount: 4, - }, - skeletonTracing1: { - treeCount: 6, - nodeCount: 28, - edgeCount: 23, - branchPointCount: 0, - }, - }, - info: '["INITIALIZE_CONNECTOME_TRACING","SET_TD_CAMERA_WITHOUT_TIME_TRACKING * 5","SET_INPUT_CATCHER_RECTS","SET_TD_CAMERA_WITHOUT_TIME_TRACKING","UPDATE_CONNECTOME_FILE_LIST","SET_MAXIMUM_ZOOM_FOR_ALL_MAGS_FOR_LAYER * 4","SET_TOOL","CHANGE_USER_BOUNDING_BOX * 2","SET_TD_CAMERA_WITHOUT_TIME_TRACKING","CHANGE_USER_BOUNDING_BOX * 3"]', + stats: undefined, + info: "", }, { version: -1, @@ -173,37 +107,19 @@ describe("Compact Save Queue", () => { timestamp: 1747994347633, authorId: "123", actions: [ - { - name: "updateUserBoundingBoxInVolumeTracing", - value: { - boundingBoxId: 3, - actionTracingId: "skeletonTracing1", - updatedProps: { - boundingBox: { - topLeft: [3224, 395, 944], - width: 1, - height: 100, - depth: 12, - }, + updateUserBoundingBoxInSkeletonTracing( + 3, + { + boundingBox: { + min: [3224, 395, 944], + max: [3225, 495, 956], }, - hasUpdatedBoundingBox: true, - hasUpdatedName: false, - hasUpdatedColor: false, }, - }, + "skeletonTracing1", + ), ], - stats: { - volumeTracing1: { - segmentCount: 4, - }, - skeletonTracing1: { - treeCount: 6, - nodeCount: 28, - edgeCount: 23, - branchPointCount: 0, - }, - }, - info: '["INITIALIZE_CONNECTOME_TRACING","SET_TD_CAMERA_WITHOUT_TIME_TRACKING * 5","SET_INPUT_CATCHER_RECTS","SET_TD_CAMERA_WITHOUT_TIME_TRACKING","UPDATE_CONNECTOME_FILE_LIST","SET_MAXIMUM_ZOOM_FOR_ALL_MAGS_FOR_LAYER * 4","SET_TOOL","CHANGE_USER_BOUNDING_BOX * 2","SET_TD_CAMERA_WITHOUT_TIME_TRACKING","CHANGE_USER_BOUNDING_BOX * 3"]', + stats: undefined, + info: "", }, { version: -1, @@ -213,69 +129,61 @@ describe("Compact Save Queue", () => { timestamp: 1747994347637, authorId: "123", actions: [ - { - name: "updateUserBoundingBoxInVolumeTracing", - value: { - boundingBoxId: 3, - actionTracingId: "skeletonTracing1", - updatedProps: { - boundingBox: { - topLeft: [3224, 3955, 900], - width: 1, - height: 92, - depth: 12, - }, + updateUserBoundingBoxInSkeletonTracing( + 3, + { + boundingBox: { + min: [3224, 3955, 900], + max: [3225, 4047, 912], }, - hasUpdatedBoundingBox: true, - hasUpdatedName: false, - hasUpdatedColor: false, }, - }, + "skeletonTracing1", + ), ], - stats: { - volumeTracing1: { - segmentCount: 4, - }, - skeletonTracing1: { - treeCount: 6, - nodeCount: 28, - edgeCount: 23, - branchPointCount: 0, - }, - }, - info: '["INITIALIZE_CONNECTOME_TRACING","SET_TD_CAMERA_WITHOUT_TIME_TRACKING * 5","SET_INPUT_CATCHER_RECTS","SET_TD_CAMERA_WITHOUT_TIME_TRACKING","UPDATE_CONNECTOME_FILE_LIST","SET_MAXIMUM_ZOOM_FOR_ALL_MAGS_FOR_LAYER * 4","SET_TOOL","CHANGE_USER_BOUNDING_BOX * 2","SET_TD_CAMERA_WITHOUT_TIME_TRACKING","CHANGE_USER_BOUNDING_BOX * 3"]', + stats: undefined, + info: "", }, ]; const compactedActions = removeSubsequentUpdateBBoxActions(actions); expect(compactedActions).toHaveLength(2); - const latestSkeletonTracingAction = compactedActions.find( - (action) => action.actions[0].name === "updateUserBoundingBoxInSkeletonTracing", - ); - expect(latestSkeletonTracingAction).toBeDefined(); - const volumeActionValue = latestSkeletonTracingAction?.actions[0].value; + const volumeActionBatch = compactedActions[0]; + const skeletonActionBatch = compactedActions[1]; + expect(skeletonActionBatch.actions[0].name).toBe("updateUserBoundingBoxInSkeletonTracing"); + expect(volumeActionBatch.actions[0].name).toBe("updateUserBoundingBoxInVolumeTracing"); + + const volumeActionValue = ( + volumeActionBatch?.actions[0] as UpdateUserBoundingBoxInVolumeTracingAction + ).value; expect(volumeActionValue).not.toBeNull(); - expect(volumeActionValue).toHaveProperty("updatedProps"); - // @ts-ignore we checked before that voluemActionValue is not null and that is has the property updatedProps - expect(volumeActionValue.updatedProps.boundingBox).toEqual({ + if (volumeActionValue == null) { + throw new Error("volumeActionValue must not be null"); + } + + expect(volumeActionValue.boundingBox).toEqual({ topLeft: [3224, 3955, 944], width: 1, - height: 100, + height: 92, depth: 12, }); - const latestVolumeTracingAction = compactedActions.find( - (action) => action.actions[0].name === "updateUserBoundingBoxInSkeletonTracing", - ); - expect(latestVolumeTracingAction).toBeDefined(); - const skeletonActionValue = latestVolumeTracingAction?.actions[0].value; + + const skeletonActionValue = ( + skeletonActionBatch?.actions[0] as UpdateUserBoundingBoxInSkeletonTracingAction + ).value; expect(skeletonActionValue).not.toBeNull(); - expect(skeletonActionValue).toHaveProperty("updatedProps"); - // @ts-ignore checked before - expect(skeletonActionValue.updatedProps.boundingBox).toEqual({ + if (skeletonActionValue == null) { + throw new Error("skeletonActionValue must not be null"); + } + + expect(skeletonActionValue.boundingBox).toEqual({ topLeft: [3224, 3955, 900], width: 1, height: 92, depth: 12, }); + + if (skeletonActionValue == null) { + throw new Error("skeletonActionValue must not be null"); + } }); it("UpdateUserBoundingBoxActions should be not compacted for different props", () => { @@ -288,32 +196,16 @@ describe("Compact Save Queue", () => { timestamp: 1747994347590, authorId: "123", actions: [ - { - name: "updateUserBoundingBoxInVolumeTracing", - value: { - boundingBoxId: 3, - actionTracingId: "volumeTracing1", - updatedProps: { - name: "test1", - }, - hasUpdatedBoundingBox: false, - hasUpdatedName: true, - hasUpdatedColor: false, + updateUserBoundingBoxInVolumeTracing( + 3, + { + name: "test1", }, - }, + "volumeTracing1", + ), ], - stats: { - volumeTracing1: { - segmentCount: 4, - }, - skeletonTracing1: { - treeCount: 6, - nodeCount: 28, - edgeCount: 23, - branchPointCount: 0, - }, - }, - info: '["INITIALIZE_CONNECTOME_TRACING","SET_TD_CAMERA_WITHOUT_TIME_TRACKING * 5","SET_INPUT_CATCHER_RECTS","SET_TD_CAMERA_WITHOUT_TIME_TRACKING","UPDATE_CONNECTOME_FILE_LIST","SET_MAXIMUM_ZOOM_FOR_ALL_MAGS_FOR_LAYER * 4","SET_TOOL","CHANGE_USER_BOUNDING_BOX * 2","SET_TD_CAMERA_WITHOUT_TIME_TRACKING","CHANGE_USER_BOUNDING_BOX * 3"]', + stats: undefined, + info: "", }, { version: -1, @@ -323,37 +215,19 @@ describe("Compact Save Queue", () => { timestamp: 1747994347633, authorId: "123", actions: [ - { - name: "updateUserBoundingBoxInVolumeTracing", - value: { - boundingBoxId: 3, - actionTracingId: "volumeTracing1", - updatedProps: { - boundingBox: { - topLeft: [3224, 3955, 944], - width: 1, - height: 100, - depth: 12, - }, + updateUserBoundingBoxInVolumeTracing( + 3, + { + boundingBox: { + min: [3224, 3955, 944], + max: [3225, 4055, 956], }, - hasUpdatedBoundingBox: true, - hasUpdatedName: false, - hasUpdatedColor: false, }, - }, + "volumeTracing1", + ), ], - stats: { - volumeTracing1: { - segmentCount: 4, - }, - skeletonTracing1: { - treeCount: 6, - nodeCount: 28, - edgeCount: 23, - branchPointCount: 0, - }, - }, - info: '["INITIALIZE_CONNECTOME_TRACING","SET_TD_CAMERA_WITHOUT_TIME_TRACKING * 5","SET_INPUT_CATCHER_RECTS","SET_TD_CAMERA_WITHOUT_TIME_TRACKING","UPDATE_CONNECTOME_FILE_LIST","SET_MAXIMUM_ZOOM_FOR_ALL_MAGS_FOR_LAYER * 4","SET_TOOL","CHANGE_USER_BOUNDING_BOX * 2","SET_TD_CAMERA_WITHOUT_TIME_TRACKING","CHANGE_USER_BOUNDING_BOX * 3"]', + stats: undefined, + info: "", }, { version: -1, @@ -363,37 +237,19 @@ describe("Compact Save Queue", () => { timestamp: 1747994347637, authorId: "123", actions: [ - { - name: "updateUserBoundingBoxInVolumeTracing", - value: { - boundingBoxId: 3, - actionTracingId: "volumeTracing1", - updatedProps: { - boundingBox: { - topLeft: [3224, 3955, 944], - width: 1, - height: 92, - depth: 12, - }, + updateUserBoundingBoxInVolumeTracing( + 3, + { + boundingBox: { + min: [3224, 3955, 944], + max: [3225, 4047, 956], }, - hasUpdatedBoundingBox: true, - hasUpdatedName: false, - hasUpdatedColor: false, }, - }, + "volumeTracing1", + ), ], - stats: { - volumeTracing1: { - segmentCount: 4, - }, - skeletonTracing1: { - treeCount: 6, - nodeCount: 28, - edgeCount: 23, - branchPointCount: 0, - }, - }, - info: '["INITIALIZE_CONNECTOME_TRACING","SET_TD_CAMERA_WITHOUT_TIME_TRACKING * 5","SET_INPUT_CATCHER_RECTS","SET_TD_CAMERA_WITHOUT_TIME_TRACKING","UPDATE_CONNECTOME_FILE_LIST","SET_MAXIMUM_ZOOM_FOR_ALL_MAGS_FOR_LAYER * 4","SET_TOOL","CHANGE_USER_BOUNDING_BOX * 2","SET_TD_CAMERA_WITHOUT_TIME_TRACKING","CHANGE_USER_BOUNDING_BOX * 3"]', + stats: undefined, + info: "", }, ]; diff --git a/frontend/javascripts/viewer/model/helpers/compaction/compact_save_queue.ts b/frontend/javascripts/viewer/model/helpers/compaction/compact_save_queue.ts index 457323b624d..fd3fcab965d 100644 --- a/frontend/javascripts/viewer/model/helpers/compaction/compact_save_queue.ts +++ b/frontend/javascripts/viewer/model/helpers/compaction/compact_save_queue.ts @@ -93,15 +93,15 @@ export function removeSubsequentUpdateBBoxActions(updateActionsBatches: Array Date: Mon, 26 May 2025 14:22:54 +0200 Subject: [PATCH 33/41] reduce amount of checks in diffBoundingBoxes --- .../model/sagas/skeletontracing_saga.ts | 28 +++++++++---------- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/frontend/javascripts/viewer/model/sagas/skeletontracing_saga.ts b/frontend/javascripts/viewer/model/sagas/skeletontracing_saga.ts index 574963cd2f6..aa97895d68b 100644 --- a/frontend/javascripts/viewer/model/sagas/skeletontracing_saga.ts +++ b/frontend/javascripts/viewer/model/sagas/skeletontracing_saga.ts @@ -620,22 +620,20 @@ export function* diffBoundingBoxes( prevBoundingBoxes.map((bbox) => bbox.id), currentBoundingBoxes.map((bbox) => bbox.id), ); - const addBBoxAction = + const [addBBoxAction, deleteBBoxAction, updateBBoxAction, updateBBoxVisibilityAction] = tracingType === AnnotationLayerEnum.Skeleton - ? addUserBoundingBoxInSkeletonTracing - : addUserBoundingBoxInVolumeTracing; - const deleteBBoxAction = - tracingType === AnnotationLayerEnum.Skeleton - ? deleteUserBoundingBoxInSkeletonTracing - : deleteUserBoundingBoxInVolumeTracing; - const updateBBoxAction = - tracingType === AnnotationLayerEnum.Skeleton - ? updateUserBoundingBoxInSkeletonTracing - : updateUserBoundingBoxInVolumeTracing; - const updateBBoxVisibilityAction = - tracingType === AnnotationLayerEnum.Skeleton - ? updateUserBoundingBoxVisibilityInSkeletonTracing - : updateUserBoundingBoxVisibilityInVolumeTracing; + ? [ + addUserBoundingBoxInSkeletonTracing, + deleteUserBoundingBoxInSkeletonTracing, + updateUserBoundingBoxInSkeletonTracing, + updateUserBoundingBoxVisibilityInSkeletonTracing, + ] + : [ + addUserBoundingBoxInVolumeTracing, + deleteUserBoundingBoxInVolumeTracing, + updateUserBoundingBoxInVolumeTracing, + updateUserBoundingBoxVisibilityInVolumeTracing, + ]; const getErrorMessage = (id: number) => `User bounding box with id ${id} not found in ${tracingType} tracing.`; for (const id of deletedBBoxIds) { From 2d0c5438932098577763973ce282df0136ef0f6b Mon Sep 17 00:00:00 2001 From: Philipp Otto Date: Mon, 26 May 2025 14:26:05 +0200 Subject: [PATCH 34/41] update changelog --- CHANGELOG.unreleased.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.unreleased.md b/CHANGELOG.unreleased.md index b86dd1e17a8..11c69a3b78b 100644 --- a/CHANGELOG.unreleased.md +++ b/CHANGELOG.unreleased.md @@ -46,6 +46,7 @@ For upgrade instructions, please check the [migration guide](MIGRATIONS.released - Fixed a bug where merging annotations with large tree IDs could lead to an error. [#8643](https://github.com/scalableminds/webknossos/pull/8643) - Fixed that the segment stats were sometimes not displayed in the context menu. [#8645](https://github.com/scalableminds/webknossos/pull/8645) - Fixed a bug in zarr streaming where directly after the datastore startup, chunk responses would have status 404 (leading zarr clients to fill with fill_value). Now it will yield status 503, so that clients can retry or escalate this as an error. [#8644](https://github.com/scalableminds/webknossos/pull/8644) +- Improved efficiency of saving bounding box related changes. [#8492](https://github.com/scalableminds/webknossos/pull/8492) ### Removed - The old "Selective Segment Visibility" feature that allowed to only see the active and the hovered segment was removed. From now on the visibility of segments can be controlled with checkboxes in the segment list and with the "Hide unlisted segments" toggle in the layer settings. [#8546](https://github.com/scalableminds/webknossos/pull/8546) From 3ed790f11edc06f65144b3b92580c7ba3087d34b Mon Sep 17 00:00:00 2001 From: Philipp Otto Date: Mon, 26 May 2025 14:28:05 +0200 Subject: [PATCH 35/41] remove redundant check in spec --- frontend/javascripts/test/model/compact_save_queue.spec.ts | 4 ---- 1 file changed, 4 deletions(-) diff --git a/frontend/javascripts/test/model/compact_save_queue.spec.ts b/frontend/javascripts/test/model/compact_save_queue.spec.ts index 70f730b0a58..4bc2d541352 100644 --- a/frontend/javascripts/test/model/compact_save_queue.spec.ts +++ b/frontend/javascripts/test/model/compact_save_queue.spec.ts @@ -180,10 +180,6 @@ describe("Compact Save Queue", () => { height: 92, depth: 12, }); - - if (skeletonActionValue == null) { - throw new Error("skeletonActionValue must not be null"); - } }); it("UpdateUserBoundingBoxActions should be not compacted for different props", () => { From ac859f307ce0aa67c59a431174418c227446cf43 Mon Sep 17 00:00:00 2001 From: Philipp Otto Date: Mon, 26 May 2025 16:10:42 +0200 Subject: [PATCH 36/41] Apply suggestions from code review --- frontend/javascripts/viewer/view/version_entry.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frontend/javascripts/viewer/view/version_entry.tsx b/frontend/javascripts/viewer/view/version_entry.tsx index f5bbd97e351..1df695d9505 100644 --- a/frontend/javascripts/viewer/view/version_entry.tsx +++ b/frontend/javascripts/viewer/view/version_entry.tsx @@ -91,13 +91,13 @@ const descriptionFns: Record< updateUserBoundingBoxesInSkeletonTracing: ( firstAction: LEGACY_UpdateUserBoundingBoxesInSkeletonTracingUpdateAction, ): Description => ({ - description: `Updated bounding boxes ${firstAction.value.boundingBoxes.map((bbox) => bbox.id).join()}.`, + description: `Updated bounding boxes ${firstAction.value.boundingBoxes.map((bbox) => bbox.id).join(", ")}.`, icon: , }), updateUserBoundingBoxesInVolumeTracing: ( firstAction: LEGACY_UpdateUserBoundingBoxesInVolumeTracingUpdateAction, ): Description => ({ - description: `Updated bounding boxes ${firstAction.value.boundingBoxes.map((bbox) => bbox.id).join()}.`, + description: `Updated bounding boxes ${firstAction.value.boundingBoxes.map((bbox) => bbox.id).join(", ")}.`, icon: , }), addUserBoundingBoxInSkeletonTracing: ( From ff5e196e782823a50d2937a0d7621bd1892e2a27 Mon Sep 17 00:00:00 2001 From: Philipp Otto Date: Mon, 26 May 2025 16:16:16 +0200 Subject: [PATCH 37/41] change edit to eye icon --- frontend/javascripts/viewer/view/version_entry.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frontend/javascripts/viewer/view/version_entry.tsx b/frontend/javascripts/viewer/view/version_entry.tsx index 1df695d9505..822cc1ecb11 100644 --- a/frontend/javascripts/viewer/view/version_entry.tsx +++ b/frontend/javascripts/viewer/view/version_entry.tsx @@ -140,13 +140,13 @@ const descriptionFns: Record< firstAction: UpdateUserBoundingBoxVisibilityInSkeletonTracingAction, ): Description => ({ description: `Toggled the visibility of bounding box ${firstAction.value.boundingBoxId}.`, - icon: , + icon: , }), updateUserBoundingBoxVisibilityInVolumeTracing: ( firstAction: UpdateUserBoundingBoxVisibilityInVolumeTracingAction, ): Description => ({ description: `Toggled the visibility of bounding box ${firstAction.value.boundingBoxId}.`, - icon: , + icon: , }), removeFallbackLayer: (): Description => ({ description: "Removed the segmentation fallback layer.", From 841dfd4361467a3f20e95b8f7c6ffbc9571b7f6a Mon Sep 17 00:00:00 2001 From: Philipp Otto Date: Tue, 27 May 2025 11:46:03 +0200 Subject: [PATCH 38/41] fix that bbox color update would not contain all three rgb values if only some of them changed; replace dangerous implementation of diffObjects --- .../dataset/dataset_settings_view.tsx | 4 +- frontend/javascripts/libs/utils.ts | 47 +++++++++---------- .../viewer/model/sagas/save_saga.ts | 3 +- .../model/sagas/skeletontracing_saga.ts | 12 ++--- 4 files changed, 29 insertions(+), 37 deletions(-) diff --git a/frontend/javascripts/dashboard/dataset/dataset_settings_view.tsx b/frontend/javascripts/dashboard/dataset/dataset_settings_view.tsx index 8697c204ce8..062ffc6037f 100644 --- a/frontend/javascripts/dashboard/dataset/dataset_settings_view.tsx +++ b/frontend/javascripts/dashboard/dataset/dataset_settings_view.tsx @@ -20,7 +20,7 @@ import type { } from "history"; import { handleGenericError } from "libs/error_handling"; import Toast from "libs/toast"; -import { diffObjects, jsonStringify } from "libs/utils"; +import { jsonStringify } from "libs/utils"; import _ from "lodash"; import messages from "messages"; import * as React from "react"; @@ -313,7 +313,7 @@ class DatasetSettingsView extends React.PureComponent) { - return _.size(diffObjects(dataSource, this.state.savedDataSourceOnServer || {})) > 0; + return _.isEqual(dataSource, this.state.savedDataSourceOnServer || {}); } didDatasourceIdChange(dataSource: Record) { diff --git a/frontend/javascripts/libs/utils.ts b/frontend/javascripts/libs/utils.ts index 8de352dfd43..62feda217db 100644 --- a/frontend/javascripts/libs/utils.ts +++ b/frontend/javascripts/libs/utils.ts @@ -1084,32 +1084,27 @@ export function getWindowBounds(): [number, number] { return [width, height]; } -/** - * Deep diff between two object, using lodash - * @param {Object} object Object compared - * @param {Object} base Object to compare with - * @return {Object} Return a new object who represent the diff - * - * Source: https://gist.github.com/Yimiprod/7ee176597fef230d1451#gistcomment-2699388 - */ -export function diffObjects( - object: Record, - base: Record, -): Record { - function changes(_object: Record, _base: Record) { - let arrayIndexCounter = 0; - return _.transform(_object, (result, value, key) => { - if (!_.isEqual(value, _base[key])) { - const resultKey = _.isArray(_base) ? arrayIndexCounter++ : key; - // @ts-expect-error ts-migrate(2571) FIXME: Object is of type 'unknown'. - result[resultKey] = - _.isObject(value) && _.isObject(_base[key]) ? changes(value, _base[key]) : value; - } - }); - } - - // @ts-expect-error ts-migrate(2322) FIXME: Type 'unknown' is not assignable to type 'Record>( + a: Dict, + b: Dict, +): Partial { + /** + * Returns the difference between two objects as a partial object. + * + * Compares two objects `a` and `b` and returns a new object containing the key-value + * pairs that are present in `b` but not in `a`, using deep equality comparison. + * Note that keys that are not present in b but in a are *not* returned. + * + * @param {Dict} a - The "original" object to compare to as a base. + * @param {Dict} b - The "newer" object that will be used for the returned values. + * @returns {Partial} - A partial object containing the key-value pairs that differ. + * + * @example + * const a = { x: 1, y: 2, z: 3 }; + * const b = { x: 1, y: 3, q: 4 }; // y is different, z is missing, q was added + * diffObjects(a, b); // returns { y: 3, q: 4 } + */ + return _.fromPairs(_.differenceWith(_.toPairs(b), _.toPairs(a), _.isEqual)) as Partial; } export function fastDiffSetAndMap(setA: Set, mapB: Map) { diff --git a/frontend/javascripts/viewer/model/sagas/save_saga.ts b/frontend/javascripts/viewer/model/sagas/save_saga.ts index 07f89294755..2d3c37eebbb 100644 --- a/frontend/javascripts/viewer/model/sagas/save_saga.ts +++ b/frontend/javascripts/viewer/model/sagas/save_saga.ts @@ -160,8 +160,7 @@ let didShowFailedSimultaneousTracingError = false; export function* sendSaveRequestToServer(): Saga { /* - * Saves a reasonably-sized part of the save queue (that corresponds to the - * tracingId) to the server (plus retry-mechanism). + * Saves a reasonably-sized part of the save queue to the server (plus retry-mechanism). * The saga returns the number of save queue items that were saved. */ diff --git a/frontend/javascripts/viewer/model/sagas/skeletontracing_saga.ts b/frontend/javascripts/viewer/model/sagas/skeletontracing_saga.ts index aa97895d68b..4f7fd7ab980 100644 --- a/frontend/javascripts/viewer/model/sagas/skeletontracing_saga.ts +++ b/frontend/javascripts/viewer/model/sagas/skeletontracing_saga.ts @@ -654,18 +654,16 @@ export function* diffBoundingBoxes( throw new Error(getErrorMessage(id)); } if (currentBbox === prevBbox) continue; + const diffBBox = Utils.diffObjects(currentBbox, prevBbox); if (_.isEmpty(diffBBox)) continue; - const changedKeys = Object.keys(diffBBox); - if (changedKeys.includes("isVisible")) { + + const { isVisible: maybeIsVisible, ...changedKeys } = diffBBox; + if (maybeIsVisible != null) { yield updateBBoxVisibilityAction(currentBbox.id, currentBbox.isVisible, tracingId); continue; } - if (changedKeys.includes("boundingBox")) { - diffBBox.boundingBox.min = currentBbox.boundingBox.min; - diffBBox.boundingBox.max = currentBbox.boundingBox.max; - } - yield updateBBoxAction(currentBbox.id, diffBBox, tracingId); + yield updateBBoxAction(currentBbox.id, changedKeys, tracingId); } } From 81ed241c45adff6d0ba574360fe635469ea89d2c Mon Sep 17 00:00:00 2001 From: Philipp Otto Date: Tue, 27 May 2025 12:13:20 +0200 Subject: [PATCH 39/41] add test for bbox diffing and fix more --- .../test/model/compact_save_queue.spec.ts | 102 +++++++++++++++++- .../model/sagas/skeletontracing_saga.ts | 10 +- 2 files changed, 106 insertions(+), 6 deletions(-) diff --git a/frontend/javascripts/test/model/compact_save_queue.spec.ts b/frontend/javascripts/test/model/compact_save_queue.spec.ts index 4bc2d541352..df89be52e42 100644 --- a/frontend/javascripts/test/model/compact_save_queue.spec.ts +++ b/frontend/javascripts/test/model/compact_save_queue.spec.ts @@ -1,14 +1,114 @@ +import { AnnotationLayerEnum } from "types/api_types"; import { removeSubsequentUpdateBBoxActions } from "viewer/model/helpers/compaction/compact_save_queue"; +import { diffBoundingBoxes } from "viewer/model/sagas/skeletontracing_saga"; import { updateUserBoundingBoxInSkeletonTracing, type UpdateUserBoundingBoxInSkeletonTracingAction, updateUserBoundingBoxInVolumeTracing, type UpdateUserBoundingBoxInVolumeTracingAction, } from "viewer/model/sagas/update_actions"; -import type { SaveQueueEntry } from "viewer/store"; +import type { SaveQueueEntry, UserBoundingBox } from "viewer/store"; import { describe, expect, it } from "vitest"; describe("Compact Save Queue", () => { + it("It should create correct update actions when diffing user bounding boxes", () => { + const oldBboxes: UserBoundingBox[] = [ + { + id: 1, + boundingBox: { + min: [1, 2, 3], + max: [10, 20, 30], + }, + name: "bbox 1", + color: [1, 1, 1], + isVisible: true, + }, + { + id: 2, + boundingBox: { + min: [1, 10, 3], + max: [10, 20, 30], + }, + name: "bbox 2", + color: [1, 2, 3], + isVisible: true, + }, + ]; + const newBboxes: UserBoundingBox[] = [ + // Removed 1 + // Changed 2 + { + id: 2, + boundingBox: { + min: [1, 5, 3], // <-- changed + max: [10, 20, 30], + }, + name: "bbox 2", + color: [1, 1, 1], // <-- changed + isVisible: false, // <-- changed + }, + // Added 3 + { + id: 3, + boundingBox: { + min: [1, 10, 3], + max: [10, 20, 30], + }, + name: "bbox 3", + color: [1, 1, 1], + isVisible: false, + }, + ]; + + const diff = Array.from( + diffBoundingBoxes(oldBboxes, newBboxes, "skeletonTracing1", AnnotationLayerEnum.Skeleton), + ); + + expect(diff.length).toBe(4); + + const [deleteFirst, addThird, updateVisibilityForSecond, changeSecond] = diff; + + expect(deleteFirst.name).toBe("deleteUserBoundingBoxInSkeletonTracing"); + expect(deleteFirst.value).toEqual({ + boundingBoxId: 1, + actionTracingId: "skeletonTracing1", + }); + + expect(addThird.name).toBe("addUserBoundingBoxInSkeletonTracing"); + expect(addThird.value).toEqual({ + boundingBox: { + ...newBboxes[1], + boundingBox: { + topLeft: [1, 10, 3], + width: 9, + height: 10, + depth: 27, + }, + }, + actionTracingId: "skeletonTracing1", + }); + + expect(updateVisibilityForSecond.name).toBe("updateUserBoundingBoxVisibilityInSkeletonTracing"); + expect(updateVisibilityForSecond.value).toEqual({ + boundingBoxId: 2, + actionTracingId: "skeletonTracing1", + isVisible: false, + }); + + expect(changeSecond.name).toBe("updateUserBoundingBoxInSkeletonTracing"); + expect(changeSecond.value).toEqual({ + boundingBoxId: 2, + actionTracingId: "skeletonTracing1", + color: [1, 1, 1], + boundingBox: { + topLeft: [1, 5, 3], + width: 9, + height: 15, + depth: 27, + }, + }); + }); + it("UpdateUserBoundingBoxActions of the same tracing should be compacted", () => { const actions: SaveQueueEntry[] = [ { diff --git a/frontend/javascripts/viewer/model/sagas/skeletontracing_saga.ts b/frontend/javascripts/viewer/model/sagas/skeletontracing_saga.ts index 4f7fd7ab980..27fd8ebcb4d 100644 --- a/frontend/javascripts/viewer/model/sagas/skeletontracing_saga.ts +++ b/frontend/javascripts/viewer/model/sagas/skeletontracing_saga.ts @@ -655,15 +655,15 @@ export function* diffBoundingBoxes( } if (currentBbox === prevBbox) continue; - const diffBBox = Utils.diffObjects(currentBbox, prevBbox); - if (_.isEmpty(diffBBox)) continue; + const diffBbox = Utils.diffObjects(prevBbox, currentBbox); - const { isVisible: maybeIsVisible, ...changedKeys } = diffBBox; + const { isVisible: maybeIsVisible, ...changedKeys } = diffBbox; if (maybeIsVisible != null) { yield updateBBoxVisibilityAction(currentBbox.id, currentBbox.isVisible, tracingId); - continue; } - yield updateBBoxAction(currentBbox.id, changedKeys, tracingId); + if (!_.isEmpty(changedKeys)) { + yield updateBBoxAction(currentBbox.id, changedKeys, tracingId); + } } } From 1f70227384836481db411491dd74951a987415b9 Mon Sep 17 00:00:00 2001 From: Philipp Otto Date: Tue, 27 May 2025 12:16:29 +0200 Subject: [PATCH 40/41] rename and move bbox spec --- .../bounding_box_saving.spec.ts} | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) rename frontend/javascripts/test/{model/compact_save_queue.spec.ts => sagas/bounding_box_saving.spec.ts} (98%) diff --git a/frontend/javascripts/test/model/compact_save_queue.spec.ts b/frontend/javascripts/test/sagas/bounding_box_saving.spec.ts similarity index 98% rename from frontend/javascripts/test/model/compact_save_queue.spec.ts rename to frontend/javascripts/test/sagas/bounding_box_saving.spec.ts index df89be52e42..f7fd1e0e047 100644 --- a/frontend/javascripts/test/model/compact_save_queue.spec.ts +++ b/frontend/javascripts/test/sagas/bounding_box_saving.spec.ts @@ -10,8 +10,8 @@ import { import type { SaveQueueEntry, UserBoundingBox } from "viewer/store"; import { describe, expect, it } from "vitest"; -describe("Compact Save Queue", () => { - it("It should create correct update actions when diffing user bounding boxes", () => { +describe("Bounding box diffing and compaction", () => { + it("should create correct update actions when diffing user bounding boxes", () => { const oldBboxes: UserBoundingBox[] = [ { id: 1, From a2979be6b22caf7e6143903684d3f10116ceb2be Mon Sep 17 00:00:00 2001 From: Philipp Otto Date: Tue, 27 May 2025 13:31:27 +0200 Subject: [PATCH 41/41] fix inversion in didDatasourceChange --- .../javascripts/dashboard/dataset/dataset_settings_view.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frontend/javascripts/dashboard/dataset/dataset_settings_view.tsx b/frontend/javascripts/dashboard/dataset/dataset_settings_view.tsx index 062ffc6037f..9099c1ae5f9 100644 --- a/frontend/javascripts/dashboard/dataset/dataset_settings_view.tsx +++ b/frontend/javascripts/dashboard/dataset/dataset_settings_view.tsx @@ -313,7 +313,7 @@ class DatasetSettingsView extends React.PureComponent) { - return _.isEqual(dataSource, this.state.savedDataSourceOnServer || {}); + return !_.isEqual(dataSource, this.state.savedDataSourceOnServer || {}); } didDatasourceIdChange(dataSource: Record) {