From 040dc2404e841638c056ee996c9b47cf549084ca Mon Sep 17 00:00:00 2001 From: Florian M <florian@scm.io> Date: Thu, 30 Jan 2025 16:25:47 +0100 Subject: [PATCH 1/4] Fix merging editable mappings where self-merge is generated; fix compacted update action writes --- conf/application.conf | 2 +- .../EditableMappingUpdater.scala | 42 ++++++++++++------- .../tracings/volume/VolumeUpdateActions.scala | 3 +- 3 files changed, 31 insertions(+), 16 deletions(-) diff --git a/conf/application.conf b/conf/application.conf index a775b1ae0a3..f62ca861b1b 100644 --- a/conf/application.conf +++ b/conf/application.conf @@ -161,7 +161,7 @@ features { exportTiffMaxVolumeMVx = 1024 exportTiffMaxEdgeLengthVx = 8192 defaultToLegacyBindings = false - editableMappingsEnabled = false + editableMappingsEnabled = true # The only valid item value is currently "ConnectomeView": optInTabs = [] openIdConnectEnabled = false diff --git a/webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/tracings/editablemapping/EditableMappingUpdater.scala b/webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/tracings/editablemapping/EditableMappingUpdater.scala index ea2a6f620e8..65419d8b75c 100644 --- a/webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/tracings/editablemapping/EditableMappingUpdater.scala +++ b/webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/tracings/editablemapping/EditableMappingUpdater.scala @@ -334,13 +334,15 @@ class EditableMappingUpdater( agglomerateId2 <- agglomerateIdForSegmentId(segmentId2) ?~> "Failed to look up agglomerate ids for merge action segments" agglomerateGraph1 <- agglomerateGraphForIdWithFallback(mapping, agglomerateId1) ?~> s"Failed to get agglomerate graph for id $agglomerateId1" agglomerateGraph2 <- agglomerateGraphForIdWithFallback(mapping, agglomerateId2) ?~> s"Failed to get agglomerate graph for id $agglomerateId2" - _ <- bool2Fox(agglomerateGraph2.segments.contains(segmentId2)) ?~> s"Segment $segmentId2 as queried by position ${update.segmentPosition2} is not contained in fetched agglomerate graph for agglomerate $agglomerateId2" + _ <- bool2Fox(agglomerateGraph2.segments.contains(segmentId2)) ?~> s"Segment $segmentId2 as queried by position ${update.segmentPosition2} is not contained in fetched agglomerate graph for agglomerate $agglomerateId2. actionTimestamp: ${update.actionTimestamp}, graph segments: ${agglomerateGraph2.segments}" mergedGraphOpt = mergeGraph(agglomerateGraph1, agglomerateGraph2, segmentId1, segmentId2) _ <- Fox.runOptional(mergedGraphOpt) { mergedGraph => for { _ <- updateSegmentToAgglomerate(agglomerateGraph2.segments, agglomerateId1) ?~> s"Failed to update segment to agglomerate buffer" _ = updateAgglomerateGraph(agglomerateId1, mergedGraph) - _ = updateAgglomerateGraph(agglomerateId2, AgglomerateGraph(List.empty, List.empty, List.empty, List.empty)) + _ = if (agglomerateId1 != agglomerateId2) + // The second agglomerate vanishes, all its segments have been moved to agglomerateId1 + updateAgglomerateGraph(agglomerateId2, AgglomerateGraph(List.empty, List.empty, List.empty, List.empty)) } yield () } } yield mapping @@ -349,19 +351,31 @@ class EditableMappingUpdater( agglomerateGraph2: AgglomerateGraph, segmentId1: Long, segmentId2: Long): Option[AgglomerateGraph] = { - val segment1IsValid = agglomerateGraph1.segments.contains(segmentId1) - val segment2IsValid = agglomerateGraph2.segments.contains(segmentId2) - if (segment1IsValid && segment2IsValid) { + val newEdgeAffinity = 255.0f + val newEdge = AgglomerateEdge(segmentId1, segmentId2) + if (agglomerateGraph1 == agglomerateGraph2) { + // Agglomerate is merged with itself. Insert new edge anyway, if it does not exist yet val newEdge = AgglomerateEdge(segmentId1, segmentId2) - val newEdgeAffinity = 255.0f - Some( - AgglomerateGraph( - segments = agglomerateGraph1.segments ++ agglomerateGraph2.segments, - edges = newEdge +: (agglomerateGraph1.edges ++ agglomerateGraph2.edges), - affinities = newEdgeAffinity +: (agglomerateGraph1.affinities ++ agglomerateGraph2.affinities), - positions = agglomerateGraph1.positions ++ agglomerateGraph2.positions - )) - } else None + if (agglomerateGraph1.edges.contains(newEdge)) { + Some(agglomerateGraph1) + } else { + Some( + agglomerateGraph1.copy(edges = newEdge +: agglomerateGraph1.edges, + affinities = newEdgeAffinity +: agglomerateGraph1.affinities)) + } + } else { + val segment1IsValid = agglomerateGraph1.segments.contains(segmentId1) + val segment2IsValid = agglomerateGraph2.segments.contains(segmentId2) + if (segment1IsValid && segment2IsValid) { + Some( + AgglomerateGraph( + segments = agglomerateGraph1.segments ++ agglomerateGraph2.segments, + edges = newEdge +: (agglomerateGraph1.edges ++ agglomerateGraph2.edges), + affinities = newEdgeAffinity +: (agglomerateGraph1.affinities ++ agglomerateGraph2.affinities), + positions = agglomerateGraph1.positions ++ agglomerateGraph2.positions + )) + } else None + } } def revertToVersion(sourceVersion: Long)(implicit ec: ExecutionContext): Fox[Unit] = 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 c4e5fae0be6..9307df81d1c 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 @@ -373,7 +373,8 @@ object CompactVolumeUpdateAction { "name" -> o.name, "value" -> (Json.obj("actionTracingId" -> o.actionTracingId, "actionTimestamp" -> o.actionTimestamp, - "actionAuthorId" -> o.actionAuthorId) ++ o.value) + "actionAuthorId" -> o.actionAuthorId) ++ o.value), + "isCompacted" -> true ) } } From 068112c65c1e9a946d5b3017c99e48b37aa61e00 Mon Sep 17 00:00:00 2001 From: Florian M <fm3@users.noreply.github.com> Date: Thu, 30 Jan 2025 16:28:59 +0100 Subject: [PATCH 2/4] Update application.conf --- conf/application.conf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/conf/application.conf b/conf/application.conf index f62ca861b1b..a775b1ae0a3 100644 --- a/conf/application.conf +++ b/conf/application.conf @@ -161,7 +161,7 @@ features { exportTiffMaxVolumeMVx = 1024 exportTiffMaxEdgeLengthVx = 8192 defaultToLegacyBindings = false - editableMappingsEnabled = true + editableMappingsEnabled = false # The only valid item value is currently "ConnectomeView": optInTabs = [] openIdConnectEnabled = false From 76f89bf33025a899e28a6f3f52d544c5cc79f270 Mon Sep 17 00:00:00 2001 From: Florian M <florian@scm.io> Date: Thu, 30 Jan 2025 16:30:53 +0100 Subject: [PATCH 3/4] changelog --- CHANGELOG.unreleased.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.unreleased.md b/CHANGELOG.unreleased.md index 9039e0c0d38..c3b778ae19e 100644 --- a/CHANGELOG.unreleased.md +++ b/CHANGELOG.unreleased.md @@ -19,6 +19,7 @@ For upgrade instructions, please check the [migration guide](MIGRATIONS.released ### Fixed - Fixed a bug that lead to trees being dropped when merging to trees together. [#8359](https://github.com/scalableminds/webknossos/pull/8359) +- Fixed a bug where merging edigable mapping (“proofreading”) annotations would sometimes fail. [#8367](https://github.com/scalableminds/webknossos/pull/8367) ### Removed - Removed the feature to downsample existing volume annotations. All new volume annotations had a whole mag stack since [#4755](https://github.com/scalableminds/webknossos/pull/4755) (four years ago). [#7917](https://github.com/scalableminds/webknossos/pull/7917) From ad11ca3f6cf40a43be0e97a6f80e8e08431f6154 Mon Sep 17 00:00:00 2001 From: Florian M <fm3@users.noreply.github.com> Date: Thu, 30 Jan 2025 17:01:02 +0100 Subject: [PATCH 4/4] Apply suggestions from code review Co-authored-by: MichaelBuessemeyer <39529669+MichaelBuessemeyer@users.noreply.github.com> --- CHANGELOG.unreleased.md | 2 +- .../tracings/editablemapping/EditableMappingUpdater.scala | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.unreleased.md b/CHANGELOG.unreleased.md index c3b778ae19e..b1f4d521117 100644 --- a/CHANGELOG.unreleased.md +++ b/CHANGELOG.unreleased.md @@ -19,7 +19,7 @@ For upgrade instructions, please check the [migration guide](MIGRATIONS.released ### Fixed - Fixed a bug that lead to trees being dropped when merging to trees together. [#8359](https://github.com/scalableminds/webknossos/pull/8359) -- Fixed a bug where merging edigable mapping (“proofreading”) annotations would sometimes fail. [#8367](https://github.com/scalableminds/webknossos/pull/8367) +- Fixed a bug where merging editable mapping (“proofreading”) annotations would sometimes fail. [#8367](https://github.com/scalableminds/webknossos/pull/8367) ### Removed - Removed the feature to downsample existing volume annotations. All new volume annotations had a whole mag stack since [#4755](https://github.com/scalableminds/webknossos/pull/4755) (four years ago). [#7917](https://github.com/scalableminds/webknossos/pull/7917) diff --git a/webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/tracings/editablemapping/EditableMappingUpdater.scala b/webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/tracings/editablemapping/EditableMappingUpdater.scala index 65419d8b75c..29d3af64380 100644 --- a/webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/tracings/editablemapping/EditableMappingUpdater.scala +++ b/webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/tracings/editablemapping/EditableMappingUpdater.scala @@ -341,7 +341,7 @@ class EditableMappingUpdater( _ <- updateSegmentToAgglomerate(agglomerateGraph2.segments, agglomerateId1) ?~> s"Failed to update segment to agglomerate buffer" _ = updateAgglomerateGraph(agglomerateId1, mergedGraph) _ = if (agglomerateId1 != agglomerateId2) - // The second agglomerate vanishes, all its segments have been moved to agglomerateId1 + // The second agglomerate vanishes, as all its segments have been moved to agglomerateId1 updateAgglomerateGraph(agglomerateId2, AgglomerateGraph(List.empty, List.empty, List.empty, List.empty)) } yield () } @@ -355,7 +355,6 @@ class EditableMappingUpdater( val newEdge = AgglomerateEdge(segmentId1, segmentId2) if (agglomerateGraph1 == agglomerateGraph2) { // Agglomerate is merged with itself. Insert new edge anyway, if it does not exist yet - val newEdge = AgglomerateEdge(segmentId1, segmentId2) if (agglomerateGraph1.edges.contains(newEdge)) { Some(agglomerateGraph1) } else {