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

Display warning when resolution gets too low during zooming #6255

Merged
merged 8 commits into from
Jun 2, 2022
1 change: 1 addition & 0 deletions CHANGELOG.unreleased.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ For upgrade instructions, please check the [migration guide](MIGRATIONS.released
[Commits](https://github.com/scalableminds/webknossos/compare/22.06.0...HEAD)

### Added
Added a warning for when the resolution in the XY viewport on z=1-downsampled datasets becomes too low, explaining the problem and how to mitigate it. [#6255](https://github.com/scalableminds/webknossos/pull/6255)

### Changed
- For the api routes that return annotation info objects, the user field was renamed to owner. User still exists as an alias, but will be removed in a future release. [#6250](https://github.com/scalableminds/webknossos/pull/6250)
Expand Down
2 changes: 2 additions & 0 deletions frontend/javascripts/messages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -284,6 +284,8 @@ instead. Only enable this option if you understand its effect. All layers will n
"This dataset location is marked as 'scratch' and meant for testing only. Please move this dataset to a permanent storage location and reimport it.",
"dataset.resolution_mismatch":
"This dataset contains multiple layers which differ in their resolution. Please convert the layers to make their resolutions match. Otherwise, rendering errors cannot be avoided.",
"dataset.z1_downsampling_hint":
"The currently rendered quality is not optimal due to the available magnifications and the viewport arrangement. To improve the quality try to increase the size of the XY viewport (e.g. by maximizing it).",
Copy link
Member

Choose a reason for hiding this comment

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

@fm3 do you want to have a look at the wording? :) this is currently what @Dagobert42 and I came up with, but I know that you have a good eye for improving messages towards conciseness :)

Copy link
Member

Choose a reason for hiding this comment

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

I think the message is already very good, but I notice it is very general.
I’d say that the constant_z downsampling is a very special case and could be explicitly mentioned? “For datasets that were not downsampled in z, it is recommended to view only the XY viewport.” – what do you think?

Copy link
Member

Choose a reason for hiding this comment

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

The current code simply checks the rendered quality (ratio of #data_voxels to #rendered_pixels) and does not check whether the dataset was z1 downsampled. Therefore, the message is a bit generic. However, it would probably be alright to use your suggestion, anyway, since the quality should not drop for another reason 🤷 I don't have a strong opinion on this..

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, I already guessed that this may be the reason. Let’s stick with the original message then, not risking false positives there :)

Copy link
Contributor Author

@Dagobert42 Dagobert42 Jun 2, 2022

Choose a reason for hiding this comment

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

"The current resolution of the XY viewport is not optimal because the dataset is not downsampled along the Z-axis. To improve the quality try to increase the size of the XY viewport (e.g. by maximizing it)."

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe this could be a short, yet more specific compromise?

"annotation.finish": "Are you sure you want to permanently finish this annotation?",
"annotation.was_finished": "Annotation was archived",
"annotation.no_fallback_data_included":
Expand Down
46 changes: 44 additions & 2 deletions frontend/javascripts/oxalis/model/sagas/dataset_saga.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
/* eslint-disable import/prefer-default-export */
import type { Saga } from "oxalis/model/sagas/effect-generators";
import { select } from "oxalis/model/sagas/effect-generators";
import { takeEvery } from "typed-redux-saga";
import { call, take, takeEvery, takeLatest } from "typed-redux-saga";
import { sleep } from "libs/utils";
import { getEnabledLayers } from "oxalis/model/accessors/dataset_accessor";
import Toast from "libs/toast";
import messages from "messages";
import { getCurrentResolution } from "../accessors/flycam_accessor";
export function* watchMaximumRenderableLayers(): Saga<void> {
function* warnMaybe(): Saga<void> {
const maximumLayerCountToRender = yield* select(
Expand All @@ -31,3 +32,44 @@ export function* watchMaximumRenderableLayers(): Saga<void> {

yield* takeEvery(["WK_READY", "UPDATE_LAYER_SETTING", "UPDATE_DATASET_SETTING"], warnMaybe);
}

let userClosedWarning = false;
export function* watchZ1Downsampling(): Saga<void> {
function* maybeShowWarning(): Saga<void> {
// In combination with `takeLatest` sleeping here at the beginning of the saga
// effectively debounces the saga to avoid that it is executed unnecessarily often.
yield* call(sleep, 200);
const currentRes = yield* select((state) => getCurrentResolution(state));
const currentZoomStep = yield* select((state) => state.flycam.zoomStep);
Dagobert42 marked this conversation as resolved.
Show resolved Hide resolved
if (currentZoomStep < 1) {
// If the user has zoomed into the data,
// the rendering quality is expected to be relatively low.
// Don't show any warnings in that case.
return;
}
const minVoxelPerPixel = 0.1;
if (!userClosedWarning) {
// checking only the downsampled dimensions x and y
const showWarning =
currentZoomStep / currentRes[0] < minVoxelPerPixel ||
currentZoomStep / currentRes[1] < minVoxelPerPixel;
if (showWarning) {
Toast.warning(messages["dataset.z1_downsampling_hint"], {
sticky: true,
key: "DOWNSAMPLING_CAUSES_BAD_QUALITY",
onClose: () => {
userClosedWarning = true;
},
});
} else {
Toast.close("DOWNSAMPLING_CAUSES_BAD_QUALITY");
}
}
}
yield* take("WK_READY");
yield* call(maybeShowWarning);
yield* takeLatest(
["ZOOM_IN", "ZOOM_OUT", "ZOOM_BY_DELTA", "SET_ZOOM_STEP", "SET_STORED_LAYOUTS"],
maybeShowWarning,
);
}
3 changes: 2 additions & 1 deletion frontend/javascripts/oxalis/model/sagas/root_saga.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import SkeletontracingSagas from "oxalis/model/sagas/skeletontracing_saga";
import ErrorHandling from "libs/error_handling";
import handleMeshChanges from "oxalis/model/sagas/handle_mesh_changes";
import isosurfaceSaga from "oxalis/model/sagas/isosurface_saga";
import { watchMaximumRenderableLayers } from "oxalis/model/sagas/dataset_saga";
import { watchMaximumRenderableLayers, watchZ1Downsampling } from "oxalis/model/sagas/dataset_saga";
import { watchToolDeselection } from "oxalis/model/sagas/annotation_tool_saga";
import SettingsSaga from "oxalis/model/sagas/settings_saga";
import watchTasksAsync, { warnAboutMagRestriction } from "oxalis/model/sagas/task_saga";
Expand Down Expand Up @@ -47,6 +47,7 @@ function* restartableSaga(): Saga<void> {
...AnnotationSagas.map((saga) => call(saga)),
...SaveSagas.map((saga) => call(saga)),
...VolumetracingSagas.map((saga) => call(saga)),
call(watchZ1Downsampling),
]);
} catch (err) {
rootSagaCrashed = true;
Expand Down