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 unsafe integer usage by using string keys instead #5724

Merged
merged 4 commits into from
Sep 13, 2021
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 @@ -22,6 +22,7 @@ For upgrade instructions, please check the [migration guide](MIGRATIONS.released
- Fixed a bug where existing tasktypes with recommended configurations still had a property that is no longer valid. [#5707](https://github.com/scalableminds/webknossos/pull/5707)
- Fixed that segment IDs could not be copied to the clipboard. [#5709](https://github.com/scalableminds/webknossos/pull/5709)
- Fixed a bug where volume annotation version restore skipped buckets that were not yet touched in the version to be restored. [#5717](https://github.com/scalableminds/webknossos/pull/5717)
- Fixed a rendering bug showing non-existant or wrongly-colored edges that sometimes occurred after deleting edges, nodes, or trees. [#5724](https://github.com/scalableminds/webknossos/pull/5724)

### Removed
-
Expand Down
16 changes: 8 additions & 8 deletions frontend/javascripts/oxalis/geometries/skeleton.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ type BufferPosition = {

type BufferCollection = {
buffers: Array<Buffer>,
idToBufferPosition: Map<number, BufferPosition>,
idToBufferPosition: Map<string, BufferPosition>,
freeList: Array<BufferPosition>,
helper: BufferHelper,
material: typeof THREE.Material,
Expand Down Expand Up @@ -218,7 +218,7 @@ class Skeleton {
* @param updateBoundingSphere - toggle to update ThreeJS's internals
* @param createFunc - callback(buffer, index) that actually creates a node / edge
*/
create(id: number, collection: BufferCollection, createFunc: BufferOperation) {
create(id: string, collection: BufferCollection, createFunc: BufferOperation) {
let currentBuffer = collection.buffers[0];
if (collection.freeList.length === 0 && currentBuffer.nextIndex >= currentBuffer.capacity) {
currentBuffer = this.initializeBuffer(MAX_CAPACITY, collection.material, collection.helper);
Expand All @@ -242,7 +242,7 @@ class Skeleton {
* @param collection - collection of all buffers
* @param deleteFunc - callback(buffer, index) that actually deletes/invalidates a node / edge
*/
delete(id: number, collection: BufferCollection, deleteFunc: BufferOperation) {
delete(id: string, collection: BufferCollection, deleteFunc: BufferOperation) {
const bufferPosition = collection.idToBufferPosition.get(id);
if (bufferPosition != null) {
const changedAttributes = deleteFunc(bufferPosition);
Expand All @@ -262,7 +262,7 @@ class Skeleton {
* @param collection - collection of all buffers
* @param deleteFunc - callback(buffer, index) that actually updates a node / edge
*/
update(id: number, collection: BufferCollection, updateFunc: BufferOperation) {
update(id: string, collection: BufferCollection, updateFunc: BufferOperation) {
const bufferPosition = collection.idToBufferPosition.get(id);
if (bufferPosition != null) {
const changedAttributes = updateFunc(bufferPosition);
Expand Down Expand Up @@ -428,11 +428,11 @@ class Skeleton {
// ######### API ###############

/**
* Combined node/edge id and a treeId to a single unique id using recursive cantor pairings.
* Combine node, edge and tree ids to a single unique id.
* @param numbers - Array of node/edge id and treeId
*/
combineIds(...numbers: Array<number>) {
return numbers.reduce((acc, value) => 0.5 * (acc + value) * (acc + value + 1) + value);
combineIds(...numbers: Array<number>): string {
return numbers.join(",");
}

/**
Expand Down Expand Up @@ -513,7 +513,7 @@ class Skeleton {

const edgePositionUpdater = (edge: Edge, isIngoingEdge: boolean) => {
// The changed node is the target node of the edge which is saved
// after the source node in the buffer. THus we need an offset.
// after the source node in the buffer. Thus we need an offset.
const indexOffset = isIngoingEdge ? 3 : 0;
const bufferEdgeId = this.combineIds(treeId, edge.source, edge.target);
this.update(bufferEdgeId, this.edges, ({ buffer, index }) => {
Expand Down