From 6df0620124d3086ba1a21753a0ca9b78c8bb861c Mon Sep 17 00:00:00 2001 From: Peyton Lee Date: Fri, 16 Aug 2024 14:42:04 -0700 Subject: [PATCH 1/8] fix: Fix bug where unspecified camera would cause black screen in 2D view modes --- src/aics-image-viewer/shared/constants.ts | 22 +++++++++----- website/utils/test/url_utils.test.ts | 35 +++++++++++++++++++++-- website/utils/url_utils.ts | 14 +++++++-- 3 files changed, 59 insertions(+), 12 deletions(-) diff --git a/src/aics-image-viewer/shared/constants.ts b/src/aics-image-viewer/shared/constants.ts index f9765090..884558ee 100644 --- a/src/aics-image-viewer/shared/constants.ts +++ b/src/aics-image-viewer/shared/constants.ts @@ -1,3 +1,5 @@ +import { CameraState } from "@aics/volume-viewer"; + import { ChannelState, ViewerState } from "../components/ViewerStateProvider/types"; import { ViewMode, RenderMode, ImageType } from "./enums"; import { ColorArray } from "./utils/colorRepresentations"; @@ -93,6 +95,18 @@ export const PRESET_COLOR_MAP = Object.freeze([ }, ]); +/** + * Reflects the default camera settings the 3D viewer uses on volume load. + * These SHOULD NOT be changed. + */ +export const getDefaultCameraState = (): CameraState => ({ + position: [0, 0, 5], + target: [0, 0, 0], + up: [0, 1, 0], + fov: 20, + orthoScale: 0.5, +}); + export const getDefaultViewerState = (): ViewerState => ({ viewMode: ViewMode.threeD, // "XY", "XZ", "YZ" renderMode: RenderMode.volumetric, // "pathtrace", "maxproject" @@ -110,13 +124,7 @@ export const getDefaultViewerState = (): ViewerState => ({ region: { x: [0, 1], y: [0, 1], z: [0, 1] }, slice: { x: 0.5, y: 0.5, z: 0.5 }, time: 0, - cameraState: { - position: [0, 0, 5], - target: [0, 0, 0], - up: [0, 1, 0], - fov: 20, - orthoScale: 0.5, - }, + cameraState: undefined, }); export const getDefaultChannelState = (): ChannelState => ({ diff --git a/website/utils/test/url_utils.test.ts b/website/utils/test/url_utils.test.ts index 7ad3adf3..6317e597 100644 --- a/website/utils/test/url_utils.test.ts +++ b/website/utils/test/url_utils.test.ts @@ -16,11 +16,17 @@ import { serializeViewerUrlParams, CONTROL_POINTS_REGEX, LEGACY_CONTROL_POINTS_REGEX, + serializeCameraState, } from "../url_utils"; import { ChannelState, ViewerState } from "../../../src/aics-image-viewer/components/ViewerStateProvider/types"; import { ImageType, RenderMode, ViewMode } from "../../../src/aics-image-viewer/shared/enums"; import { ViewerChannelSetting } from "../../../src/aics-image-viewer/shared/utils/viewerChannelSettings"; -import { getDefaultChannelState, getDefaultViewerState } from "../../../src/aics-image-viewer/shared/constants"; +import { + getDefaultCameraState, + getDefaultChannelState, + getDefaultViewerState, +} from "../../../src/aics-image-viewer/shared/constants"; +import { CameraState } from "@aics/volume-viewer"; const defaultSettings: ViewerChannelSetting = { match: 0, @@ -390,7 +396,7 @@ describe("Channel state serialization", () => { }); }); -describe("Viewer state serialization", () => { +describe("Viewer state", () => { const DEFAULT_VIEWER_STATE: ViewerState = { viewMode: ViewMode.threeD, // "XY", "XZ", "YZ" renderMode: RenderMode.volumetric, // "pathtrace", "maxproject" @@ -529,6 +535,31 @@ describe("Viewer state serialization", () => { }); }); +describe("Camera state", () => { + it("uses default camera state when choosing elements to exclude/ignore", () => { + let cameraState: CameraState = { + ...getDefaultCameraState(), + }; + // No changes from default + expect(serializeCameraState(cameraState, true)).toEqual(undefined); + + cameraState = { ...cameraState, position: [1, 2, 3] }; + expect(serializeCameraState(cameraState, true)).toEqual("pos:1:2:3"); + }); + + it("default camera state has not been changed", () => { + // The default camera state should NOT change unless backwards compatibility + // is added to ensure old links still work. + expect(getDefaultCameraState()).toEqual({ + position: [0, 0, 5], + target: [0, 0, 0], + up: [0, 1, 0], + fov: 20, + orthoScale: 0.5, + }); + }); +}); + //// DESERIALIZE STATES /////////////////////// describe("Channel state deserialization", () => { diff --git a/website/utils/url_utils.ts b/website/utils/url_utils.ts index 7c03e610..fe4dcde4 100644 --- a/website/utils/url_utils.ts +++ b/website/utils/url_utils.ts @@ -17,7 +17,11 @@ import { PerAxis } from "../../src/aics-image-viewer/shared/types"; import { clamp } from "./math_utils"; import { removeMatchingProperties, removeUndefinedProperties } from "./datatype_utils"; import { isEqual } from "lodash"; -import { getDefaultChannelState, getDefaultViewerState } from "../../src/aics-image-viewer/shared/constants"; +import { + getDefaultCameraState, + getDefaultChannelState, + getDefaultViewerState, +} from "../../src/aics-image-viewer/shared/constants"; export const ENCODED_COMMA_REGEX = /%2C/g; export const ENCODED_COLON_REGEX = /%3A/g; @@ -573,9 +577,13 @@ function parseCameraState(cameraSettings: string | undefined): Partial, removeDefaults: boolean): string | undefined { +export function serializeCameraState(cameraState: Partial, removeDefaults: boolean): string | undefined { if (removeDefaults) { - cameraState = removeMatchingProperties(cameraState, getDefaultViewerState().cameraState ?? {}); + // Note that we use the `getDefaultCameraState()` to get the defaults here, + // instead of `getDefaultViewerState().cameraState`. The default viewer state's camera state + // is undefined, which signals that the camera should not be modified for backwards compatibility + // with old URLs that don't specify it. + cameraState = removeMatchingProperties(cameraState, getDefaultCameraState()); if (Object.keys(cameraState).length === 0) { return undefined; } From 46636474210a79455d89ca71ddc2366174d5ce6a Mon Sep 17 00:00:00 2001 From: Peyton Lee Date: Fri, 16 Aug 2024 14:47:09 -0700 Subject: [PATCH 2/8] refactor: Sorted imports --- website/utils/test/url_utils.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/website/utils/test/url_utils.test.ts b/website/utils/test/url_utils.test.ts index 6317e597..e304007d 100644 --- a/website/utils/test/url_utils.test.ts +++ b/website/utils/test/url_utils.test.ts @@ -1,3 +1,4 @@ +import { CameraState } from "@aics/volume-viewer"; import { describe, expect, it } from "@jest/globals"; import { @@ -26,7 +27,6 @@ import { getDefaultChannelState, getDefaultViewerState, } from "../../../src/aics-image-viewer/shared/constants"; -import { CameraState } from "@aics/volume-viewer"; const defaultSettings: ViewerChannelSetting = { match: 0, From 722ac2c0e74db82014c58783cbc09da3ecad1e63 Mon Sep 17 00:00:00 2001 From: Peyton Lee Date: Fri, 16 Aug 2024 14:53:21 -0700 Subject: [PATCH 3/8] refactor: Updated comment --- website/utils/url_utils.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/website/utils/url_utils.ts b/website/utils/url_utils.ts index fe4dcde4..ff908bbc 100644 --- a/website/utils/url_utils.ts +++ b/website/utils/url_utils.ts @@ -580,9 +580,8 @@ function parseCameraState(cameraSettings: string | undefined): Partial, removeDefaults: boolean): string | undefined { if (removeDefaults) { // Note that we use the `getDefaultCameraState()` to get the defaults here, - // instead of `getDefaultViewerState().cameraState`. The default viewer state's camera state - // is undefined, which signals that the camera should not be modified for backwards compatibility - // with old URLs that don't specify it. + // instead of `getDefaultViewerState().cameraState`. The latter is undefined, which signals + // that the camera should not be modified for URLs that don't specify it. cameraState = removeMatchingProperties(cameraState, getDefaultCameraState()); if (Object.keys(cameraState).length === 0) { return undefined; From 74bd936a80fce689fdeddcceccf033b884a0a00f Mon Sep 17 00:00:00 2001 From: Peyton Lee Date: Mon, 19 Aug 2024 11:30:41 -0700 Subject: [PATCH 4/8] feat: Added GTM tag to 3DVV --- public/index.html | 71 ++++++++++++++++++++++++++++++++++------------- 1 file changed, 51 insertions(+), 20 deletions(-) diff --git a/public/index.html b/public/index.html index cba8fd86..a3c0f246 100644 --- a/public/index.html +++ b/public/index.html @@ -1,29 +1,60 @@ - - - + + + + + + + + + + AICS 3D Volume Viewer - - - - - - + + + + +
- + From 35a1f24e93efbd97a455c1f7f72a1bd29e255bbc Mon Sep 17 00:00:00 2001 From: Peyton Lee Date: Mon, 19 Aug 2024 11:37:09 -0700 Subject: [PATCH 5/8] fix: Moved `iframe` GTM snippet to HTML body --- public/index.html | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/public/index.html b/public/index.html index a3c0f246..99d45df1 100644 --- a/public/index.html +++ b/public/index.html @@ -15,16 +15,6 @@ })(window, document, "script", "dataLayer", "GTM-5J7562WW"); - - - @@ -55,6 +45,17 @@ /> + + + +
From 1b0a457736f5d671cbc7fe413ac565c8e8f7b086 Mon Sep 17 00:00:00 2001 From: Peyton Lee Date: Mon, 19 Aug 2024 14:48:41 -0700 Subject: [PATCH 6/8] doc: Added comments about new camera states --- src/aics-image-viewer/shared/constants.ts | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/aics-image-viewer/shared/constants.ts b/src/aics-image-viewer/shared/constants.ts index 884558ee..1a5ff7cb 100644 --- a/src/aics-image-viewer/shared/constants.ts +++ b/src/aics-image-viewer/shared/constants.ts @@ -95,9 +95,13 @@ export const PRESET_COLOR_MAP = Object.freeze([ }, ]); +/** Allows the 3D viewer to apply the default camera settings for the view mode. */ +const USE_VIEW_MODE_DEFAULT_CAMERA = undefined; + /** * Reflects the default camera settings the 3D viewer uses on volume load. - * These SHOULD NOT be changed. + * These SHOULD NOT be changed; otherwise, existing shared links that don't specify the + * camera settings will use the new defaults and may be in unexpected orientations or positions. */ export const getDefaultCameraState = (): CameraState => ({ position: [0, 0, 5], @@ -124,7 +128,11 @@ export const getDefaultViewerState = (): ViewerState => ({ region: { x: [0, 1], y: [0, 1], z: [0, 1] }, slice: { x: 0.5, y: 0.5, z: 0.5 }, time: 0, - cameraState: undefined, + // Do not override camera position, target, etc. by default; + // instead, let the viewer apply default camera settings based on the view mode. + // This prevents a bug where the camera's position and view mode are set to + // incompatible states and the viewport becomes blank. + cameraState: USE_VIEW_MODE_DEFAULT_CAMERA, }); export const getDefaultChannelState = (): ChannelState => ({ From e099434f42b88712af1a149a8cfa43a84e6cdfe7 Mon Sep 17 00:00:00 2001 From: Peyton Lee Date: Tue, 20 Aug 2024 15:32:16 -0700 Subject: [PATCH 7/8] fix: Formatting on HTML tags in `index.html` --- public/index.html | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/public/index.html b/public/index.html index 99d45df1..816fba75 100644 --- a/public/index.html +++ b/public/index.html @@ -46,14 +46,15 @@ - + > + +
From 79e58dd8957e09a4cce5105858b4e9b57a96d93c Mon Sep 17 00:00:00 2001 From: Peyton Lee Date: Tue, 20 Aug 2024 15:34:26 -0700 Subject: [PATCH 8/8] doc: Updated comment in url_utils unit tests --- website/utils/test/url_utils.test.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/website/utils/test/url_utils.test.ts b/website/utils/test/url_utils.test.ts index e304007d..adc8f823 100644 --- a/website/utils/test/url_utils.test.ts +++ b/website/utils/test/url_utils.test.ts @@ -549,7 +549,8 @@ describe("Camera state", () => { it("default camera state has not been changed", () => { // The default camera state should NOT change unless backwards compatibility - // is added to ensure old links still work. + // is added to ensure old links still maintain the same camera orientation; + // otherwise, cameras will appear in the new default orientation unexpectedly. expect(getDefaultCameraState()).toEqual({ position: [0, 0, 5], target: [0, 0, 0],