Display warning when resolution gets too low during zooming #6255

Dagobert42 · 2022-06-01T15:50:29Z

URL of deployed dev instance (used for testing):

https://___.webknossos.xyz

Steps to test:

load a z=1 downsampled dataset and zoom out
minimizing the XY viewport enough should show an explanatory warning once per session

Issues:

fixes Show explaining hint when zooming out on z=1-downsampled datasets #6110

(Please delete unneeded items, merge only when none are left open)

Updated (unreleased) changelog
Ready for review

philippotto

Very cool stuff! I left some comments to tweak the feature a bit :)

frontend/javascripts/oxalis/model/sagas/dataset_saga.ts

philippotto · 2022-06-02T11:58:43Z

@Dagobert42 awesome stuff 🎉 I just tested it out and tweaked some small things, since they caught my eye:

there was a takeLatest usage for WK_READY which should be a take. The difference is that only the latter one actually blocks the control flow. This is why wk crashed for me in the dashboard (the rest of the saga executed even though WK_READY is not dispatched in the dashboard).
I tweaked the quality threshold even further because the old value still triggered the warning for the z1 dataset even when my viewport was maximized (at certain zoom levels).
I optimized the saga invocation a bit to avoid that it's called unnecessarily often. For that, I used takeLatest which aborts already running invocations of maybeShowWarning if a newer action arrives (e.g., when zooming out in a fluent motion).

…b.com:scalableminds/webknossos into show-warning-when-zooming-z1-downsampled-data

philippotto

really hyped about this :) see my last two suggestions, though 🤓

philippotto · 2022-06-02T11:59:20Z

CHANGELOG.unreleased.md

@@ -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 chunky, explaining the problem and how to mitigate it. [#6255](https://github.com/scalableminds/webknossos/pull/6255)


Suggested change

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

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)

philippotto · 2022-06-02T12:01:19Z

frontend/javascripts/oxalis/model/sagas/dataset_saga.ts

+      for (let i = 0; i < 2; i++) {
+        const voxelPerPixelXY = currentZoomStep / currentRes[i];
+        if (voxelPerPixelXY < minVoxelPerPixel) {
+          Toast.warning(messages["dataset.z1_downsampling_hint"], {
+            sticky: true,
+            key: "DOWNSAMPLING_CAUSES_BAD_QUALITY",
+            onClose: setUserClosedWarningTrue,
+          });
+        } else {
+          Toast.close("DOWNSAMPLING_CAUSES_BAD_QUALITY");
+        }
+      }


The Toast.warning and Toast.close calls should not happen in the for loop (right now, one dimension could have a poor quality, but the other dimension could immediately close the notification again). Since the for-loop only has two iterations, it might be easier to write it like this:

const showWarning = (currentZoomStep / currentRes[0] < minVoxelPerPixel) || (currentZoomStep / currentRes[1] < minVoxelPerPixel); if (showWarning) { ...

True, I believe I was originally thinking about iterating over the z dimension as well, but I guess there's really no use-case for that 😅

Actually, this also solves the linter complaining about multiple instantiations of the arrow function (which was the reason for setUserClosedWarningTrue) 👍

philippotto · 2022-06-02T12:21:14Z

frontend/javascripts/messages.ts

@@ -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).",


@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 :)

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?

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..

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

"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)."

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

philippotto

Awesome 🎉 Works great 👍 I'm already accepting it, but let's wait for @fm3's opinion on the wording before merging.

display warning when resolution gets too low during zooming

0cf3a1f

philippotto requested changes Jun 2, 2022

View reviewed changes

Dagobert42 and others added 4 commits June 2, 2022 10:33

Merge branch 'master' into show-warning-when-zooming-z1-downsampled-data

2171ba2

code review changes

aa459ae

updated unreleased changelog

bd38543

fix crashing dashboard; debounce z1 warnings and tweak quality threshold

67dc8f7

philippotto changed the title ~~display warning when resolution gets too low during zooming~~ Display warning when resolution gets too low during zooming Jun 2, 2022

Merge branch 'show-warning-when-zooming-z1-downsampled-data' of githu…

5b3d9d7

…b.com:scalableminds/webknossos into show-warning-when-zooming-z1-downsampled-data

philippotto requested changes Jun 2, 2022

View reviewed changes

philippotto assigned Dagobert42 Jun 2, 2022

Dagobert42 and others added 2 commits June 2, 2022 14:14

code review changes 2

39ffe00

Merge branch 'master' into show-warning-when-zooming-z1-downsampled-data

1cb6b31

philippotto reviewed Jun 2, 2022

View reviewed changes

philippotto approved these changes Jun 2, 2022

View reviewed changes

Dagobert42 merged commit 8ae68e4 into master Jun 2, 2022

Dagobert42 deleted the show-warning-when-zooming-z1-downsampled-data branch June 2, 2022 13:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Display warning when resolution gets too low during zooming #6255

Display warning when resolution gets too low during zooming #6255

Dagobert42 commented Jun 1, 2022 •

edited

Loading

philippotto left a comment

philippotto commented Jun 2, 2022

philippotto left a comment

philippotto Jun 2, 2022

philippotto Jun 2, 2022

Dagobert42 Jun 2, 2022

Dagobert42 Jun 2, 2022

philippotto Jun 2, 2022

fm3 Jun 2, 2022

philippotto Jun 2, 2022

fm3 Jun 2, 2022

Dagobert42 Jun 2, 2022 •

edited

Loading

Dagobert42 Jun 2, 2022

philippotto left a comment

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

Display warning when resolution gets too low during zooming #6255

Display warning when resolution gets too low during zooming #6255

Conversation

Dagobert42 commented Jun 1, 2022 • edited Loading

URL of deployed dev instance (used for testing):

Steps to test:

Issues:

philippotto left a comment

Choose a reason for hiding this comment

philippotto commented Jun 2, 2022

philippotto left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Dagobert42 Jun 2, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

philippotto left a comment

Choose a reason for hiding this comment

Dagobert42 commented Jun 1, 2022 •

edited

Loading

Dagobert42 Jun 2, 2022 •

edited

Loading