Skip to content

Commit

Permalink
Fix right-clicking mesh of invisible layer (#7811)
Browse files Browse the repository at this point in the history
* fix rightclicking mesh of not-active segmentation layer

* rename DiffableMap.get to getOrThrow

* update changelog

* fix accidentally changed gets
  • Loading branch information
philippotto authored May 21, 2024
1 parent 0202754 commit 813003d
Show file tree
Hide file tree
Showing 20 changed files with 87 additions and 80 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.unreleased.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ For upgrade instructions, please check the [migration guide](MIGRATIONS.released
- Fixed a bug where some annotation times would be shown double. [#7787](https://github.com/scalableminds/webknossos/pull/7787)
- Fixed a bug where no columns were shown in the time tracking overview. [#7803](https://github.com/scalableminds/webknossos/pull/7803)
- Fixed a bug where ad-hoc meshes for coarse magnifications would have gaps. [#7799](https://github.com/scalableminds/webknossos/pull/7799)
- Fixed that right-clicking a mesh in the 3D viewport did crash when the corresponding segmentation layer was not visible. [#7811](https://github.com/scalableminds/webknossos/pull/7811)

### Removed

Expand Down
4 changes: 2 additions & 2 deletions frontend/javascripts/libs/diffable_map.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ class DiffableMap<K extends number, V> {
this[idSymbol] = id;
}

get(key: K): V {
getOrThrow(key: K): V {
const value = this.getNullable(key);

if (value !== undefined) {
Expand Down Expand Up @@ -325,7 +325,7 @@ export function diffDiffableMaps<K extends number, V>(
const newOnlyB = onlyB.filter((id) => !missingChangedIdSet.has(id));
// Ensure that these elements are not equal before adding them to "changed"
const newChanged = changed.concat(
missingChangedIds.filter((id) => mapA.get(id) !== mapB.get(id)),
missingChangedIds.filter((id) => mapA.getOrThrow(id) !== mapB.getOrThrow(id)),
);
return {
changed: newChanged,
Expand Down
15 changes: 9 additions & 6 deletions frontend/javascripts/oxalis/api/api_latest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -379,7 +379,10 @@ class TracingApi {
if (treeId != null) {
tree = skeletonTracing.trees[treeId];
assertExists(tree, `Couldn't find tree ${treeId}.`);
assertExists(tree.nodes.get(nodeId), `Couldn't find node ${nodeId} in tree ${treeId}.`);
assertExists(
tree.nodes.getOrThrow(nodeId),
`Couldn't find node ${nodeId} in tree ${treeId}.`,
);
} else {
tree = _.values(skeletonTracing.trees).find((__) => __.nodes.has(nodeId));
assertExists(tree, `Couldn't find node ${nodeId}.`);
Expand Down Expand Up @@ -599,7 +602,7 @@ class TracingApi {
* console.log(segment.groupId)
*/
getSegment(segmentId: number, layerName: string): Segment {
const segment = getSegmentsForLayer(Store.getState(), layerName).get(segmentId);
const segment = getSegmentsForLayer(Store.getState(), layerName).getOrThrow(segmentId);
// Return a copy to avoid mutations by third-party code.
return { ...segment };
}
Expand Down Expand Up @@ -1070,8 +1073,8 @@ class TracingApi {
const getPos = (node: Readonly<MutableNode>) => getNodePosition(node, state);

for (const edge of tree.edges.all()) {
const sourceNode = tree.nodes.get(edge.source);
const targetNode = tree.nodes.get(edge.target);
const sourceNode = tree.nodes.getOrThrow(edge.source);
const targetNode = tree.nodes.getOrThrow(edge.target);
lengthNmAcc += V3.scaledDist(getPos(sourceNode), getPos(targetNode), datasetScale);
lengthVxAcc += V3.length(V3.sub(getPos(sourceNode), getPos(targetNode)));
}
Expand Down Expand Up @@ -1155,12 +1158,12 @@ class TracingApi {

while (priorityQueue.length > 0) {
const [nextNodeId, distance] = priorityQueue.dequeue();
const nextNodePosition = getPos(sourceTree.nodes.get(nextNodeId));
const nextNodePosition = getPos(sourceTree.nodes.getOrThrow(nextNodeId));

// Calculate the distance to all neighbours and update the distances.
for (const { source, target } of sourceTree.edges.getEdgesForNode(nextNodeId)) {
const neighbourNodeId = source === nextNodeId ? target : source;
const neighbourPosition = getPos(sourceTree.nodes.get(neighbourNodeId));
const neighbourPosition = getPos(sourceTree.nodes.getOrThrow(neighbourNodeId));
const neighbourDistance =
distance + V3.scaledDist(nextNodePosition, neighbourPosition, datasetScale);

Expand Down
5 changes: 4 additions & 1 deletion frontend/javascripts/oxalis/api/api_v2.ts
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,10 @@ class TracingApi {
if (treeId != null) {
tree = skeletonTracing.trees[treeId];
assertExists(tree, `Couldn't find tree ${treeId}.`);
assertExists(tree.nodes.get(nodeId), `Couldn't find node ${nodeId} in tree ${treeId}.`);
assertExists(
tree.nodes.getNullable(nodeId),
`Couldn't find node ${nodeId} in tree ${treeId}.`,
);
} else {
tree = _.values(skeletonTracing.trees).find((__) => __.nodes.has(nodeId));
assertExists(tree, `Couldn't find node ${nodeId}.`);
Expand Down
8 changes: 4 additions & 4 deletions frontend/javascripts/oxalis/geometries/skeleton.ts
Original file line number Diff line number Diff line change
Expand Up @@ -346,8 +346,8 @@ class Skeleton {

case "createEdge": {
const tree = skeletonTracing.trees[update.value.treeId];
const source = tree.nodes.get(update.value.source);
const target = tree.nodes.get(update.value.target);
const source = tree.nodes.getOrThrow(update.value.source);
const target = tree.nodes.getOrThrow(update.value.target);
this.createEdge(tree.treeId, source, target);
break;
}
Expand Down Expand Up @@ -507,8 +507,8 @@ class Skeleton {
}

for (const edge of tree.edges.all()) {
const source = tree.nodes.get(edge.source);
const target = tree.nodes.get(edge.target);
const source = tree.nodes.getOrThrow(edge.source);
const target = tree.nodes.getOrThrow(edge.target);
this.createEdge(tree.treeId, source, target);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ export function getActiveNode(skeletonTracing: SkeletonTracing): Maybe<Node> {
const { activeTreeId, activeNodeId } = skeletonTracing;

if (activeTreeId != null && activeNodeId != null) {
return Maybe.Just(skeletonTracing.trees[activeTreeId].nodes.get(activeNodeId));
return Maybe.Just(skeletonTracing.trees[activeTreeId].nodes.getOrThrow(activeNodeId));
}

return Maybe.Nothing();
Expand Down Expand Up @@ -95,7 +95,7 @@ export function getActiveNodeFromTree(skeletonTracing: SkeletonTracing, tree: Tr
const { activeNodeId } = skeletonTracing;

if (activeNodeId != null) {
return Maybe.Just(tree.nodes.get(activeNodeId));
return Maybe.Just(tree.nodes.getOrThrow(activeNodeId));
}

return Maybe.Nothing();
Expand Down Expand Up @@ -159,12 +159,12 @@ export function getNodeAndTree(
let node = null;

if (nodeId != null) {
node = tree.nodes.get(nodeId);
node = tree.nodes.getOrThrow(nodeId);
} else {
const { activeNodeId } = skeletonTracing;

if (activeNodeId != null) {
node = tree.nodes.get(activeNodeId);
node = tree.nodes.getOrThrow(activeNodeId);
}
}

Expand Down
6 changes: 3 additions & 3 deletions frontend/javascripts/oxalis/model/edge_collection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ export function diffEdgeCollections(
const mapDiff = diffDiffableMaps(edgeCollectionA.outMap, edgeCollectionB.outMap);

const getEdgesForNodes = (nodeIds: number[], diffableMap: EdgeMap) =>
_.flatten(nodeIds.map((nodeId) => diffableMap.get(nodeId)));
_.flatten(nodeIds.map((nodeId) => diffableMap.getOrThrow(nodeId)));

const edgeDiff = {
onlyA: getEdgesForNodes(mapDiff.onlyA, edgeCollectionA.outMap),
Expand All @@ -179,8 +179,8 @@ export function diffEdgeCollections(
// For each changedNodeIndex there is at least one outgoing edge which was added or removed.
// So, check for each outgoing edge whether it only exists in A or B
const outgoingEdgesDiff = Utils.diffArrays(
edgeCollectionA.outMap.get(changedNodeIndex),
edgeCollectionB.outMap.get(changedNodeIndex),
edgeCollectionA.outMap.getOrThrow(changedNodeIndex),
edgeCollectionB.outMap.getOrThrow(changedNodeIndex),
);
edgeDiff.onlyA = edgeDiff.onlyA.concat(outgoingEdgesDiff.onlyA);
edgeDiff.onlyB = edgeDiff.onlyB.concat(outgoingEdgesDiff.onlyB);
Expand Down
2 changes: 1 addition & 1 deletion frontend/javascripts/oxalis/model/helpers/nml_helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -679,7 +679,7 @@ function splitTreeIntoComponents(
color: tree.color,
name: `${tree.name}_${i}`,
comments: tree.comments.filter((comment) => nodeIdsSet.has(comment.nodeId)),
nodes: new DiffableMap(nodeIds.map((nodeId) => [nodeId, tree.nodes.get(nodeId)])),
nodes: new DiffableMap(nodeIds.map((nodeId) => [nodeId, tree.nodes.getOrThrow(nodeId)])),
branchPoints: tree.branchPoints.filter((bp) => nodeIdsSet.has(bp.nodeId)),
timestamp: tree.timestamp,
edges: EdgeCollection.loadFromArray(edges),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,7 @@ function splitTreeByNodes(
// ts-expect-error ts-migrate(2345) FIXME: Argument of type 'number | undefined' is not assig... Remove this comment to see the full error message
const edges = activeTree.edges.getEdgesForNode(nodeId);
visitedNodes[nodeId] = true;
newTree.nodes.mutableSet(nodeId, activeTree.nodes.get(nodeId));
newTree.nodes.mutableSet(nodeId, activeTree.nodes.getOrThrow(nodeId));

for (const edge of edges) {
const edgeHash = getEdgeHash(edge);
Expand Down Expand Up @@ -888,7 +888,7 @@ export function extractPathAsNewTree(
).map((newTree) => {
let lastNodeId = null;
for (const nodeId of pathOfNodeIds) {
const node: MutableNode = { ...sourceTree.nodes.get(nodeId) };
const node: MutableNode = { ...sourceTree.nodes.getOrThrow(nodeId) };
newTree.nodes.mutableSet(nodeId, node);
if (lastNodeId != null) {
const newEdge: Edge = {
Expand Down
4 changes: 2 additions & 2 deletions frontend/javascripts/oxalis/model/sagas/proofread_saga.ts
Original file line number Diff line number Diff line change
Expand Up @@ -340,8 +340,8 @@ function* handleSkeletonProofreadingAction(action: Action): Saga<void> {
Toast.error("Proofreading is currently not supported when the skeleton layer is transformed.");
return;
}
const sourceNodePosition = sourceTree.nodes.get(sourceNodeId).untransformedPosition;
const targetNodePosition = targetTree.nodes.get(targetNodeId).untransformedPosition;
const sourceNodePosition = sourceTree.nodes.getOrThrow(sourceNodeId).untransformedPosition;
const targetNodePosition = targetTree.nodes.getOrThrow(targetNodeId).untransformedPosition;

const idInfos = yield* call(getAgglomerateInfos, preparation.getMappedAndUnmapped, [
sourceNodePosition,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -496,13 +496,13 @@ function* diffNodes(
}

for (const nodeId of addedNodeIds) {
const node = nodes.get(nodeId);
const node = nodes.getOrThrow(nodeId);
yield createNode(treeId, node);
}

for (const nodeId of changedNodeIds) {
const node = nodes.get(nodeId);
const prevNode = prevNodes.get(nodeId);
const node = nodes.getOrThrow(nodeId);
const prevNode = prevNodes.getOrThrow(nodeId);

if (updateNodePredicate(prevNode, node)) {
yield updateNode(treeId, node);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -647,7 +647,7 @@ function* uncachedDiffSegmentLists(
}

for (const segmentId of addedSegmentIds) {
const segment = newSegments.get(segmentId);
const segment = newSegments.getOrThrow(segmentId);
yield createSegmentVolumeAction(
segment.id,
segment.somePosition,
Expand All @@ -658,8 +658,8 @@ function* uncachedDiffSegmentLists(
}

for (const segmentId of bothSegmentIds) {
const segment = newSegments.get(segmentId);
const prevSegment = prevSegments.get(segmentId);
const segment = newSegments.getOrThrow(segmentId);
const prevSegment = prevSegments.getOrThrow(segmentId);

if (segment !== prevSegment) {
yield updateSegmentVolumeAction(
Expand Down
2 changes: 1 addition & 1 deletion frontend/javascripts/oxalis/view/context_menu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1653,7 +1653,7 @@ function ContextMenuInner(propsWithInputRef: Props) {
);
}
if (segments != null && maybeClickedMeshId != null) {
const segmentName = segments.get(maybeClickedMeshId)?.name;
const segmentName = segments.getNullable(maybeClickedMeshId)?.name;
if (segmentName != null) {
const maxSegmentNameLength = 18;
infoRows.push(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1374,7 +1374,7 @@ class SegmentsView extends React.Component<Props, State> {
getSelectedSegments = (): Segment[] => {
const allSegments = this.props.segments;
if (allSegments == null) return [];
return this.props.selectedIds.segments.map((segmentId) => allSegments.get(segmentId));
return this.props.selectedIds.segments.map((segmentId) => allSegments.getOrThrow(segmentId));
};

getSelectedItemKeys = () => {
Expand Down
4 changes: 2 additions & 2 deletions frontend/javascripts/test/geometries/skeleton.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,8 +93,8 @@ test.serial("Skeleton should initialize correctly using the store's state", (t)
}

for (const edge of tree.edges.all()) {
const sourcePosition = tree.nodes.get(edge.source).untransformedPosition;
const targetPosition = tree.nodes.get(edge.target).untransformedPosition;
const sourcePosition = tree.nodes.getOrThrow(edge.source).untransformedPosition;
const targetPosition = tree.nodes.getOrThrow(edge.target).untransformedPosition;
edgePositions = edgePositions.concat(sourcePosition).concat(targetPosition);
edgeTreeIds.push(tree.treeId, tree.treeId);
}
Expand Down
24 changes: 12 additions & 12 deletions frontend/javascripts/test/libs/diffable_map.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ test("DiffableMap should be empty", (t) => {
const emptyMap = new DiffableMap<number, number>();
t.is(emptyMap.size(), 0);
t.false(emptyMap.has(1));
t.throws(() => emptyMap.get(1));
t.throws(() => emptyMap.getOrThrow(1));
});
test("DiffableMap should behave immutable on set/delete operations", (t) => {
const emptyMap = new DiffableMap();
Expand All @@ -19,22 +19,22 @@ test("DiffableMap should behave immutable on set/delete operations", (t) => {
t.false(emptyMap.has(1));
t.is(map1.size(), 1);
t.true(map1.has(1));
t.is(map1.get(1), 1);
t.is(map1.getOrThrow(1), 1);
const map2 = map1.set(1, 2);
t.is(map1.get(1), 1);
t.is(map2.get(1), 2);
t.is(map1.getOrThrow(1), 1);
t.is(map2.getOrThrow(1), 2);
const map3 = map2.delete(1);
t.is(map2.get(1), 2);
t.is(map2.getOrThrow(1), 2);
t.false(map3.has(1));
});
test("DiffableMap should be clonable and mutable on clone/mutableSet", (t) => {
const map1 = new DiffableMap().set(1, 1);
const map2 = map1.clone();
map2.mutableSet(1, 2);
map2.mutableSet(2, 2);
t.is(map2.get(1), 2);
t.is(map2.get(2), 2);
t.is(map1.get(1), 1);
t.is(map2.getOrThrow(1), 2);
t.is(map2.getOrThrow(2), 2);
t.is(map1.getOrThrow(1), 1);
t.false(map1.has(2));
// Id should be the same since the internal structures look the same
t.is(map1.getId(), map2.getId());
Expand All @@ -46,8 +46,8 @@ test("DiffableMap should be instantiable with Array<[key, value]>", (t) => {
[1, 2],
[3, 4],
]);
t.is(map.get(1), 2);
t.is(map.get(3), 4);
t.is(map.getOrThrow(1), 2);
t.is(map.getOrThrow(3), 4);
t.is(map.size(), 2);
});
test("DiffableMap should work properly when it handles more items than the batch size", (t) => {
Expand All @@ -61,7 +61,7 @@ test("DiffableMap should work properly when it handles more items than the batch

// Check for [i, 2*i] values
for (let i = 0; i < 100; i++) {
t.is(currentMap.get(i), 2 * i);
t.is(currentMap.getOrThrow(i), 2 * i);
}

t.is(emptyMap.size(), 0);
Expand All @@ -78,7 +78,7 @@ test("DiffableMap should work properly when it handles more items than the batch
if (i % 10 === 0) {
t.false(currentMap.has(i));
} else {
t.is(currentMap.get(i), 2 * i);
t.is(currentMap.getOrThrow(i), 2 * i);
}
}
});
Expand Down
10 changes: 5 additions & 5 deletions frontend/javascripts/test/libs/nml.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,7 @@ test("NML serializing and parsing should yield the same state even when addition
if (existingNodeMap == null) {
throw new Error("Unexpected null value.");
}
const existingNode = existingNodeMap.get(1);
const existingNode = existingNodeMap.getOrThrow(1);
const newNodeMap = existingNodeMap.set(1, {
...existingNode,
additionalCoordinates: [{ name: "t", value: 123 }],
Expand Down Expand Up @@ -416,7 +416,7 @@ test("NML serializer should produce correct NMLs with additional coordinates", (
if (existingNodeMap == null) {
throw new Error("Unexpected null value.");
}
const existingNode = existingNodeMap.get(1);
const existingNode = existingNodeMap.getOrThrow(1);
const newNodeMap = existingNodeMap.set(1, {
...existingNode,
additionalCoordinates: [{ name: "t", value: 123 }],
Expand Down Expand Up @@ -694,10 +694,10 @@ test("addTreesAndGroups reducer should assign new node and tree ids", (t) => {
t.is(newSkeletonTracing.trees[3].treeId, 3);
t.is(newSkeletonTracing.trees[4].treeId, 4);
t.is(newSkeletonTracing.trees[3].nodes.size(), 4);
t.is(newSkeletonTracing.trees[3].nodes.get(8).id, 8);
t.is(newSkeletonTracing.trees[3].nodes.get(9).id, 9);
t.is(newSkeletonTracing.trees[3].nodes.getOrThrow(8).id, 8);
t.is(newSkeletonTracing.trees[3].nodes.getOrThrow(9).id, 9);
t.is(newSkeletonTracing.trees[4].nodes.size(), 3);
t.is(newSkeletonTracing.trees[4].nodes.get(12).id, 12);
t.is(newSkeletonTracing.trees[4].nodes.getOrThrow(12).id, 12);

const getSortedEdges = (edges: EdgeCollection) => _.sortBy(edges.asArray(), "source");

Expand Down
10 changes: 5 additions & 5 deletions frontend/javascripts/test/model/edge_collection.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,13 @@ test("EdgeCollection should have symmetrical inMap and outMap", (t) => {
t.is(edgeCollection.size(), 3);
t.is(edgeCollection.outMap.size(), 3);
t.is(edgeCollection.inMap.size(), 2);
t.deepEqual(edgeCollection.outMap.get(0), [edgeA]);
t.deepEqual(edgeCollection.outMap.getOrThrow(0), [edgeA]);
t.false(edgeCollection.outMap.has(1));
t.deepEqual(edgeCollection.outMap.get(2), [edgeB]);
t.deepEqual(edgeCollection.outMap.get(3), [edgeC]);
t.deepEqual(edgeCollection.outMap.getOrThrow(2), [edgeB]);
t.deepEqual(edgeCollection.outMap.getOrThrow(3), [edgeC]);
t.false(edgeCollection.inMap.has(0));
t.deepEqual(edgeCollection.inMap.get(1), [edgeA, edgeB]);
t.deepEqual(edgeCollection.inMap.get(2), [edgeC]);
t.deepEqual(edgeCollection.inMap.getOrThrow(1), [edgeA, edgeB]);
t.deepEqual(edgeCollection.inMap.getOrThrow(2), [edgeC]);
t.false(edgeCollection.inMap.has(3));
t.deepEqual(edgeCollection.getEdgesForNode(2), [edgeB, edgeC]);
});
Expand Down
Loading

0 comments on commit 813003d

Please sign in to comment.