Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix right-clicking mesh of invisible layer #7811

Merged
merged 5 commits into from
May 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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