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

Use mag in data requests, no more zoomStep/exponent #6159

Merged
merged 20 commits into from
May 9, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
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
2 changes: 1 addition & 1 deletion .eslintrc.json
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@
"react/state-in-constructor": "off", // probably want "never",
"react/default-props-match-prop-types": "warn", // probably should be enabled
"react/jsx-fragments": "off", // debateable. could be autofixed with eslint --fix ...
"max-classes-per-file": "warn", // seems useful, but requires some refactoring
"max-classes-per-file": "off", // seems useful, but requires some refactoring, set to off because it's visually distracting
"prefer-object-spread": "off", // debateable. could be autofixed with eslint --fix ...
"@typescript-eslint/lines-between-class-members": "off",
"@typescript-eslint/naming-convention": [
Expand Down
1 change: 1 addition & 0 deletions CHANGELOG.unreleased.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ For upgrade instructions, please check the [migration guide](MIGRATIONS.released
- Added support to stream zarr files using the corresponding [zarr spec](https://zarr.readthedocs.io/en/stable/spec/v2.html#storage). [#6144](https://github.com/scalableminds/webknossos/pull/6144)

### Changed
- Changed the internal protocol for requesting image data. The zoomStep parameter has been replaced by mag. This increases the datastore API version to 2.0 [#6159](https://github.com/scalableminds/webknossos/pull/6159)

### Fixed
- Fixed a bug in the task cration, where creation for some tasks with initial volume data would fail. [#6178](https://github.com/scalableminds/webknossos/pull/6178)
Expand Down
2 changes: 2 additions & 0 deletions MIGRATIONS.unreleased.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,6 @@ User-facing changes are documented in the [changelog](CHANGELOG.released.md).
## Unreleased
[Commits](https://github.com/scalableminds/webknossos/compare/22.05.1...HEAD)

- Note that the datastore API version has changed to 2.0. If you use webknossos-connect alongside your webKnossos instance, you will need to upgrade that one as well, compare [webknossos-connect#129](https://github.com/scalableminds/webknossos-connect/pull/129). [#6159](https://github.com/scalableminds/webknossos/pull/6159)

### Postgres Evolutions:
7 changes: 2 additions & 5 deletions app/models/binary/WKRemoteDataStoreClient.scala
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,9 @@ package models.binary

import com.scalableminds.util.geometry.Vec3Int
import com.scalableminds.util.tools.Fox
import com.scalableminds.webknossos.datastore.models.ImageThumbnail
import com.scalableminds.webknossos.datastore.rpc.RPC
import com.typesafe.scalalogging.LazyLogging
import controllers.RpcTokenHolder
import org.apache.commons.codec.binary.Base64
import play.api.libs.json.JsObject
import play.utils.UriEncoding

Expand All @@ -21,15 +19,14 @@ class WKRemoteDataStoreClient(dataStore: DataStore, dataSet: DataSet, rpc: RPC)
zoom: Option[Double],
center: Option[Vec3Int]): Fox[Array[Byte]] = {
logger.debug(s"Thumbnail called for: $organizationName-${dataSet.name} Layer: $dataLayerName")
rpc(s"${dataStore.url}/data/datasets/${urlEncode(organizationName)}/${dataSet.urlEncodedName}/layers/$dataLayerName/thumbnail.json")
rpc(s"${dataStore.url}/data/datasets/${urlEncode(organizationName)}/${dataSet.urlEncodedName}/layers/$dataLayerName/thumbnail.jpg")
.addQueryString("token" -> RpcTokenHolder.webKnossosToken)
.addQueryString("width" -> width.toString, "height" -> height.toString)
.addQueryStringOptional("zoom", zoom.map(_.toString))
.addQueryStringOptional("centerX", center.map(_.x.toString))
.addQueryStringOptional("centerY", center.map(_.y.toString))
.addQueryStringOptional("centerZ", center.map(_.z.toString))
.getWithJsonResponse[ImageThumbnail]
.map(thumbnail => Base64.decodeBase64(thumbnail.value))
.getWithBytesResponse
}

def findPositionWithData(organizationName: String, dataLayerName: String): Fox[JsObject] =
Expand Down
5 changes: 3 additions & 2 deletions app/models/task/TaskCreationService.scala
Original file line number Diff line number Diff line change
Expand Up @@ -45,11 +45,12 @@ class TaskCreationService @Inject()(taskTypeService: TaskTypeService,
extends FoxImplicits
with ProtoGeometryImplicits {

def assertBatchLimit(batchSize: Int, taskTypeIds: List[String])(implicit ctx: DBAccessContext): Fox[Unit] =
def assertBatchLimit(batchSize: Int, taskTypeIds: List[String])(implicit ctx: DBAccessContext,
m: MessagesProvider): Fox[Unit] =
for {
isVolumeOrHybrid <- taskTypeService.containsVolumeOrHybridTaskType(taskTypeIds.toSet.toList)
batchLimit = if (isVolumeOrHybrid) 100 else 1000
_ <- bool2Fox(batchSize <= batchLimit) ?~> "task.create.limitExceeded"
_ <- bool2Fox(batchSize <= batchLimit) ?~> Messages("task.create.limitExceeded", batchLimit)
} yield ()

// Used in create (without files) in case of base annotation
Expand Down
1 change: 1 addition & 0 deletions conf/messages
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,7 @@ dataLayer.notFound=DataLayer “{0}” not found
dataLayer.mustBeSegmentation=DataLayer “{0}” is not a segmentation layer

dataLayer.wrongMag=DataLayer “{0}” does not have mag “{1}”
dataLayer.invalidMag=Supplied “{0}” is not a valid mag format. Please use “x-y-z”

zarr.invalidChunkCoordinates=The requested chunk coordinates are in an invalid format. Expected c.x.y.z
zarr.invalidFirstChunkCoord="First Channel must be 0"
Expand Down
2 changes: 2 additions & 0 deletions docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,8 @@ services:
- CIRCLE_TAG=${CIRCLE_TAG}
- CIRCLE_BUILD_NUM=${CIRCLE_BUILD_NUM}
- NODE_OPTIONS=--max_old_space_size=2048
# This will be picked up by ava so that the tests fail if a snapshot doesn't exist during CI, see https://github.com/avajs/ava/issues/1585
- CI
working_dir: /home/${USER_NAME:-sbt-user}/webknossos
volumes:
- ".:/home/${USER_NAME:-sbt-user}/webknossos"
Expand Down
6 changes: 3 additions & 3 deletions frontend/javascripts/admin/admin_rest_api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1780,7 +1780,7 @@ export function getMeshData(id: string): Promise<ArrayBuffer> {
// receives too many parameters, since this doesn't play well with the saga typings.
type IsosurfaceRequest = {
position: Vector3;
zoomStep: number;
mag: Vector3;
segmentId: number;
subsamplingStrides: Vector3;
cubeSize: Vector3;
Expand All @@ -1798,7 +1798,7 @@ export function computeIsosurface(
}> {
const {
position,
zoomStep,
mag,
segmentId,
subsamplingStrides,
cubeSize,
Expand All @@ -1816,7 +1816,7 @@ export function computeIsosurface(
// is added here to the position and bbox size.
position: V3.toArray(V3.sub(position, subsamplingStrides)),
cubeSize: V3.toArray(V3.add(cubeSize, subsamplingStrides)),
zoomStep,
mag,
// Segment to build mesh for
segmentId,
// Name and type of mapping to apply before building mesh (optional)
Expand Down
1 change: 1 addition & 0 deletions frontend/javascripts/libs/window.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ const _window =
pageYOffset: 0,
addEventListener,
removeEventListener,
open: (_url: string) => {},
}
: window;

Expand Down
62 changes: 40 additions & 22 deletions frontend/javascripts/oxalis/api/api_latest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ import {
} from "oxalis/model/accessors/volumetracing_accessor";
import { getHalfViewportExtentsFromState } from "oxalis/model/sagas/saga_selectors";
import {
getDatasetResolutionInfo,
getLayerBoundaries,
getLayerByName,
getResolutionInfo,
Expand Down Expand Up @@ -1459,6 +1460,33 @@ class DataApi {
return result;
}

/**
* Helper method to build the download URL for a raw data cuboid.
*
* @ignore
*/
_getDownloadUrlForRawDataCuboid(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice refactoring 👍

layerName: string,
topLeft: Vector3,
bottomRight: Vector3,
token: string,
): string {
const { dataset } = Store.getState();
const resolutionInfo = getDatasetResolutionInfo(dataset);
const resolution = resolutionInfo.getLowestResolution();
const magString = resolution.join("-");
return (
`${dataset.dataStore.url}/data/datasets/${dataset.owningOrganization}/${dataset.name}/layers/${layerName}/data?mag=${magString}&` +
`token=${token}&` +
`x=${topLeft[0]}&` +
`y=${topLeft[1]}&` +
`z=${topLeft[2]}&` +
`width=${bottomRight[0] - topLeft[0]}&` +
`height=${bottomRight[1] - topLeft[1]}&` +
`depth=${bottomRight[2] - topLeft[2]}`
);
}

/**
* Downloads a cuboid of raw data from a dataset (not tracing) layer. A new window is opened for the download -
* if that is not the case, please check your pop-up blocker.
Expand All @@ -1467,37 +1495,27 @@ class DataApi {
* api.data.downloadRawDataCuboid("segmentation", [0,0,0], [100,200,100]);
*/
downloadRawDataCuboid(layerName: string, topLeft: Vector3, bottomRight: Vector3): Promise<void> {
const { dataset } = Store.getState();
return doWithToken((token) => {
const downloadUrl =
`${dataset.dataStore.url}/data/datasets/${dataset.owningOrganization}/${dataset.name}/layers/${layerName}/data?resolution=0&` +
`token=${token}&` +
`x=${topLeft[0]}&` +
`y=${topLeft[1]}&` +
`z=${topLeft[2]}&` +
`width=${bottomRight[0] - topLeft[0]}&` +
`height=${bottomRight[1] - topLeft[1]}&` +
`depth=${bottomRight[2] - topLeft[2]}`;
// @ts-expect-error ts-migrate(2339) FIXME: Property 'open' does not exist on type '(Window & ... Remove this comment to see the full error message
const downloadUrl = this._getDownloadUrlForRawDataCuboid(
layerName,
topLeft,
bottomRight,
token,
);
window.open(downloadUrl);
// Theoretically the window.open call could fail if the token is expired, but that would be hard to check
return Promise.resolve();
});
}

getRawDataCuboid(layerName: string, topLeft: Vector3, bottomRight: Vector3): Promise<void> {
const { dataset } = Store.getState();
return doWithToken((token) => {
const downloadUrl =
`${dataset.dataStore.url}/data/datasets/${dataset.owningOrganization}/${dataset.name}/layers/${layerName}/data?resolution=0&` +
`token=${token}&` +
`x=${topLeft[0]}&` +
`y=${topLeft[1]}&` +
`z=${topLeft[2]}&` +
`width=${bottomRight[0] - topLeft[0]}&` +
`height=${bottomRight[1] - topLeft[1]}&` +
`depth=${bottomRight[2] - topLeft[2]}`;
// Theoretically the window.open call could fail if the token is expired, but that would be hard to check
const downloadUrl = this._getDownloadUrlForRawDataCuboid(
layerName,
topLeft,
bottomRight,
token,
);
return Request.receiveArraybuffer(downloadUrl);
});
}
Expand Down
11 changes: 8 additions & 3 deletions frontend/javascripts/oxalis/api/api_v2.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,10 @@ import {
getActiveTree,
getTree,
} from "oxalis/model/accessors/skeletontracing_accessor";
import { getLayerBoundaries } from "oxalis/model/accessors/dataset_accessor";
import {
getDatasetResolutionInfo,
getLayerBoundaries,
} from "oxalis/model/accessors/dataset_accessor";
import { setActiveCellAction } from "oxalis/model/actions/volumetracing_actions";
import { getActiveCellId } from "oxalis/model/accessors/volumetracing_accessor";
import type { Vector3, AnnotationTool, ControlMode } from "oxalis/constants";
Expand Down Expand Up @@ -636,17 +639,19 @@ class DataApi {
*/
downloadRawDataCuboid(layerName: string, topLeft: Vector3, bottomRight: Vector3): Promise<void> {
const { dataset } = Store.getState();
const resolutionInfo = getDatasetResolutionInfo(dataset);
const resolution = resolutionInfo.getLowestResolution();
const magString = resolution.join("-");
return doWithToken((token) => {
const downloadUrl =
`${dataset.dataStore.url}/data/datasets/${dataset.name}/layers/${layerName}/data?resolution=0&` +
`${dataset.dataStore.url}/data/datasets/${dataset.name}/layers/${layerName}/data?mag=${magString}&` +
`token=${token}&` +
`x=${topLeft[0]}&` +
`y=${topLeft[1]}&` +
`z=${topLeft[2]}&` +
`width=${bottomRight[0] - topLeft[0]}&` +
`height=${bottomRight[1] - topLeft[1]}&` +
`depth=${bottomRight[2] - topLeft[2]}`;
// @ts-expect-error ts-migrate(2339) FIXME: Property 'open' does not exist on type '(Window & ... Remove this comment to see the full error message
window.open(downloadUrl);
// Theoretically the window.open call could fail if the token is expired, but that would be hard to check
return Promise.resolve();
Expand Down
24 changes: 24 additions & 0 deletions frontend/javascripts/oxalis/model/accessors/dataset_accessor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,20 @@ export class ResolutionInfo {
return resolution;
}

getIndexByResolution(resolution: Vector3): number {
const index = Math.log2(Math.max(...resolution));

// Assert that the index exists and that the resolution at that index
// equals the resolution argument
const resolutionMaybe = this.getResolutionByIndex(index);
if (!_.isEqual(resolution, resolutionMaybe)) {
throw new Error(
`Resolution ${resolution} with index ${index} is not equal to existing resolution at that index: ${resolutionMaybe}.`,
);
}
return index;
}

getResolutionByIndexWithFallback(
index: number,
fallbackResolutionInfo: ResolutionInfo | null | undefined,
Expand Down Expand Up @@ -167,6 +181,16 @@ export class ResolutionInfo {
return Math.log2(this.getLowestResolutionPowerOf2());
}

getHighestResolution(): Vector3 {
// @ts-ignore
return this.getResolutionByPowerOf2(this.getHighestResolutionPowerOf2());
}
Comment on lines +184 to +187
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just noticed that this method isn't used anywhere. Only getLowestResolution is being used.

But I think it is fine to keep this method in case someone wants the highest resolution in the future.


getLowestResolution(): Vector3 {
// @ts-ignore
return this.getResolutionByPowerOf2(this.getLowestResolutionPowerOf2());
}

getAllIndices(): Array<number> {
return this.getResolutionsWithIndices().map((entry) => entry[0]);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
// eslint-disable-next-line max-classes-per-file
import _ from "lodash";
import type { Area } from "oxalis/model/accessors/flycam_accessor";
import { ResolutionInfo } from "oxalis/model/accessors/dataset_accessor";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ const compressionPool = new WorkerPool(
export const REQUEST_TIMEOUT = 60000;
export type SendBucketInfo = {
position: Vector3;
zoomStep: number;
mag: Vector3;
cubeSize: number;
};
type RequestBucketInfo = SendBucketInfo & {
Expand Down Expand Up @@ -68,7 +68,7 @@ const createRequestBucketInfo = (
function createSendBucketInfo(zoomedAddress: Vector4, resolutions: Array<Vector3>): SendBucketInfo {
return {
position: bucketPositionToGlobalAddress(zoomedAddress, resolutions),
zoomStep: zoomedAddress[3],
mag: resolutions[zoomedAddress[3]],
cubeSize: constants.BUCKET_WIDTH,
};
}
Expand Down
3 changes: 2 additions & 1 deletion frontend/javascripts/oxalis/model/sagas/isosurface_saga.ts
Original file line number Diff line number Diff line change
Expand Up @@ -368,6 +368,7 @@ function* maybeLoadIsosurface(
const volumeTracing = yield* select((state) => getActiveSegmentationTracing(state));
// Fetch from datastore if no volumetracing exists or if the tracing has a fallback layer.
const useDataStore = volumeTracing == null || volumeTracing.fallbackLayer != null;
const mag = resolutionInfo.getResolutionByIndexOrThrow(zoomStep);

if (isInitialRequest) {
sendAnalyticsEvent("request_isosurface", {
Expand All @@ -387,7 +388,7 @@ function* maybeLoadIsosurface(
useDataStore ? dataStoreUrl : tracingStoreUrl,
{
position: clippedPosition,
zoomStep,
mag,
segmentId,
subsamplingStrides,
cubeSize,
Expand Down
5 changes: 3 additions & 2 deletions frontend/javascripts/oxalis/model/sagas/save_saga.ts
Original file line number Diff line number Diff line change
Expand Up @@ -915,11 +915,12 @@ function* markBucketsAsNotDirty(saveQueue: Array<SaveQueueEntry>, tracingId: str
for (const saveEntry of saveQueue) {
for (const updateAction of saveEntry.actions) {
if (updateAction.name === "updateBucket") {
const { position, zoomStep } = updateAction.value;
const { position, mag } = updateAction.value;
const resolutionIndex = segmentationResolutionInfo.getIndexByResolution(mag);
const zoomedBucketAddress = globalPositionToBucketPosition(
position,
segmentationResolutionInfo.getDenseResolutions(),
zoomStep,
resolutionIndex,
);
const bucket = segmentationLayer.cube.getOrCreateBucket(zoomedBucketAddress);

Expand Down
2 changes: 1 addition & 1 deletion frontend/javascripts/test/api/api_volume_latest.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ test("Data API: downloadRawDataCuboid should open a popup with the correct URL",
t.true(window.open.calledOnce);
t.true(
window.open.calledWith(
"http://localhost:9000/data/datasets/Connectomics department/ROI2017_wkw/layers/color/data?resolution=0&token=secure-token&x=1&y=2&z=3&width=8&height=6&depth=4",
"http://localhost:9000/data/datasets/Connectomics department/ROI2017_wkw/layers/color/data?mag=1-1-1&token=secure-token&x=1&y=2&z=3&width=8&height=6&depth=4",
),
);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -148,13 +148,13 @@ function createExpectedOptions(fourBit: boolean = false) {
data: [
{
position: [0, 0, 0],
zoomStep: 0,
mag: [1, 1, 1],
cubeSize: 32,
fourBit,
},
{
position: [64, 64, 64],
zoomStep: 1,
mag: [2, 2, 2],
cubeSize: 32,
fourBit,
},
Expand Down Expand Up @@ -234,7 +234,7 @@ test.serial("sendToStore: Request Handling should send the correct request param
name: "updateBucket",
value: {
position: [0, 0, 0],
zoomStep: 0,
mag: [1, 1, 1],
cubeSize: 32,
base64Data: byteArrayToLz4Base64(data),
},
Expand All @@ -243,7 +243,7 @@ test.serial("sendToStore: Request Handling should send the correct request param
name: "updateBucket",
value: {
position: [64, 64, 64],
zoomStep: 1,
mag: [2, 2, 2],
cubeSize: 32,
base64Data: byteArrayToLz4Base64(data),
},
Expand Down
Loading