Skip to content

Commit

Permalink
feat: map refactoring (#1837)
Browse files Browse the repository at this point in the history
Refactor Map component to remove numerous useEffect

Main motivation is to fix wrong view state computed when first rendering
is done with hidden flag.
First reproduced by application with tabbed layout where all the tabs
are rendered using hidden flag.
This use case has been added to the storybook to demonstrate the issue,
along with a 'triggerHome' control (in a previous PR).

The current PR brings:
- onResize() function to handle resizing
- use a React reducer to handle Z scaling
  Add support for PageUp/PageDown and Shift modifier
- use a React reducer to compute the global 3D bounding box from the
reportBoundingBox callback of the layers
- introduce a ViewController class to generate the DeckGL views and
viewState
This brings all the computations in one place/one step by providing
state change allowing to fine-tune the computations
- use bounding box from the 'cameraPosition' (if set as zoom field)
instead of the data bounding box

Fixes:
- initial viewState is correct, even when rendered with 'hidden' flag
set to true
- triggerHome now respects current zScale

In the end, the final implementation drops down to 6 React useEffect,
from the 16 initial ones !



Fix for #1820
w1nklr authored Dec 18, 2023
1 parent 452d512 commit 849b839
Showing 9 changed files with 1,011 additions and 667 deletions.
1 change: 1 addition & 0 deletions typescript/packages/subsurface-viewer/package.json
Original file line number Diff line number Diff line change
@@ -16,6 +16,7 @@
"test": "jest --coverage",
"test:update": "npm test -- --u",
"test:watch": "npm test -- --watch",
"test:debug": "node --inspect-brk ../../node_modules/jest/bin/jest.js --coverage=false --runInBand",
"doc": "git clean -xdff docs && typedoc src"
},
"author": "Equinor <[email protected]>",
Original file line number Diff line number Diff line change
@@ -31,6 +31,19 @@ exports[`Test Map component snapshot test 1`] = `
style="left: 0px; top: 0px; width: 100%; position: absolute; height: 100%;"
/>
</div>
<div
style="position: absolute; left: 10px; top: 10px;"
>
<label
style="left: 10px; top: 10px;"
>
3
km
</label>
<div
style="width: 100px; height: 4px; border: 2px solid; display: inline-block; margin-left: 3px;"
/>
</div>
<div>
<svg
aria-valuemax="100"
@@ -108,6 +121,19 @@ exports[`Test Map component snapshot test with edited data 1`] = `
style="left: 0px; top: 0px; width: 100%; position: absolute; height: 100%;"
/>
</div>
<div
style="position: absolute; left: 10px; top: 10px;"
>
<label
style="left: 10px; top: 10px;"
>
3
km
</label>
<div
style="width: 100px; height: 4px; border: 2px solid; display: inline-block; margin-left: 3px;"
/>
</div>
<div>
<svg
aria-valuemax="100"
@@ -185,6 +211,19 @@ exports[`Test Map component snapshot test with invalid array length 1`] = `
style="left: 0px; top: 0px; width: 100%; position: absolute; height: 100%;"
/>
</div>
<div
style="position: absolute; left: 10px; top: 10px;"
>
<label
style="left: 10px; top: 10px;"
>
3
km
</label>
<div
style="width: 100px; height: 4px; border: 2px solid; display: inline-block; margin-left: 3px;"
/>
</div>
<div>
<svg
aria-valuemax="100"
Original file line number Diff line number Diff line change
@@ -8,6 +8,7 @@ import type { LayersList } from "@deck.gl/core/typed";
import Map from "./Map";
import { EmptyWrapper } from "../test/TestWrapper";
import { colorTables } from "@emerson-eps/color-tables";
import type { colorTablesArray } from "@emerson-eps/color-tables/";
import {
ColormapLayer,
DrawingLayer,
@@ -21,7 +22,7 @@ import {

import mapData from "../../../../../example-data/deckgl-map.json";
import type { Unit } from "convert-units";
const colorTablesData = colorTables;
const colorTablesData = colorTables as colorTablesArray;
const testBounds = [432205, 6475078, 437720, 6481113] as [
number,
number,
1,470 changes: 831 additions & 639 deletions typescript/packages/subsurface-viewer/src/components/Map.tsx

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
@@ -31,6 +31,19 @@ exports[`Test Map component snapshot test 1`] = `
style="left: 0px; top: 0px; width: 100%; position: absolute; height: 100%;"
/>
</div>
<div
style="position: absolute; left: 10px; top: 10px;"
>
<label
style="left: 10px; top: 10px;"
>
3
km
</label>
<div
style="width: 100px; height: 4px; border: 2px solid; display: inline-block; margin-left: 3px;"
/>
</div>
<div>
<svg
aria-valuemax="100"
@@ -108,6 +121,19 @@ exports[`Test Map component snapshot test with edited data 1`] = `
style="left: 0px; top: 0px; width: 100%; position: absolute; height: 100%;"
/>
</div>
<div
style="position: absolute; left: 10px; top: 10px;"
>
<label
style="left: 10px; top: 10px;"
>
3
km
</label>
<div
style="width: 100px; height: 4px; border: 2px solid; display: inline-block; margin-left: 3px;"
/>
</div>
<div>
<svg
aria-valuemax="100"
Original file line number Diff line number Diff line change
@@ -462,38 +462,38 @@ ScaleZ.parameters = {
},
};

const ResetCameraPropertyDefaultCameraPosition = {
rotationOrbit: 0,
rotationX: 45,
target: [435775, 6478650, -2750],
zoom: -3.8,
};

export const ResetCameraProperty: ComponentStory<typeof SubsurfaceViewer> = (
args
) => {
const [home, setHome] = React.useState<number>(0);
const [camera, setCamera] = React.useState({
rotationOrbit: 0,
rotationX: 45,
target: [435775, 6477650, -1750],
zoom: -3.8,
});

const handleChange1 = () => {
setHome(home + 1);
};
const [camera, setCamera] = React.useState(
() => args.cameraPosition ?? ResetCameraPropertyDefaultCameraPosition
);

const handleChange2 = () => {
setCamera({ ...camera, rotationOrbit: camera.rotationOrbit + 5 });
const handleChange = () => {
setCamera({
...camera,
rotationOrbit: camera.rotationOrbit + 5,
});
};

const props = {
...args,
cameraPosition: camera,
triggerHome: home,
};

return (
<Root>
<div className={classes.main}>
<SubsurfaceViewer {...props} />
</div>
<button onClick={handleChange1}> Reset Camera to bounds</button>
<button onClick={handleChange2}> Change Camera </button>
<button onClick={handleChange}> Change Camera </button>
</Root>
);
};
@@ -503,22 +503,15 @@ ResetCameraProperty.args = {
layers: [axes_hugin, meshMapLayerPng, north_arrow_layer],

bounds: [432150, 6475800, 439400, 6481500] as NumberQuad,
cameraPosition: {
rotationOrbit: 0,
rotationX: 80,
target: [435775, 6478650, -1750],
zoom: -3.5109619192773796,
},
cameraPosition: ResetCameraPropertyDefaultCameraPosition,
views: DEFAULT_VIEWS,
};

ResetCameraProperty.parameters = {
docs: {
...defaultParameters.docs,
description: {
story: `Example using optional 'triggerHome' property.
When this property is changed camera will reset to home position.
Using the button the property will change its value.`,
story: `Pressing the button 'Change Camera' does rotate it.`,
},
},
};
Original file line number Diff line number Diff line change
@@ -126,11 +126,13 @@ export function getWellLayerByTypeAndSelectedWells(
type: string,
selectedWell: string
): LayersList {
if (!layers) return [];
if (!layers || !selectedWell) {
return [];
}
return layers.filter((l) => {
return (
l?.constructor.name === type &&
(l as NewLayersList).props.data.features.find(
(l as NewLayersList).props.data?.features?.find(
(item) => item.properties.name === selectedWell
)
);
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
import "jest";

import type { BoundingBox3D } from "./BoundingBox3D";
import { boxCenter, boxUnion } from "./BoundingBox3D";

describe("Test BoundingBox3D", () => {
it("boxUnion default box", () => {
const unitBox: BoundingBox3D = [0, 0, 0, 1, 1, 1];
const defaultBox: BoundingBox3D = [-1, -1, -1, 1, 1, 1];
const box1: BoundingBox3D = [0, 1, 2, 5, 6, 7];
const box2: BoundingBox3D = [1, 2, 3, 4, 5, 6];
expect(boxUnion(undefined, undefined)).toEqual(unitBox);
expect(boxUnion(undefined, undefined, defaultBox)).toEqual(defaultBox);
expect(boxUnion(box1, undefined, defaultBox)).toEqual(box1);
expect(boxUnion(undefined, box2, defaultBox)).toEqual(box2);
expect(boxUnion(box1, box2, defaultBox)).toEqual(box1);
});

it("boxUnion without default box", () => {
const box1: BoundingBox3D = [0, 1, 2, 5, 6, 7];
const box2: BoundingBox3D = [1, 2, 3, 4, 5, 6];
const box3: BoundingBox3D = [1, 2, 3, 6, 7, 8];
expect(boxUnion(box1, undefined)).toEqual(box1);
expect(boxUnion(undefined, box2)).toEqual(box2);
expect(boxUnion(box1, box2)).toEqual(box1);
expect(boxUnion(box1, box3)).toEqual([0, 1, 2, 6, 7, 8]);
});

it("boxCenter", () => {
const box1: BoundingBox3D = [0, 1, 2, 5, 6, 7];
const box2: BoundingBox3D = [1, 2, 3, 4, 5, 6];
const box3: BoundingBox3D = [1, 2, 3, 6, 7, 8];
expect(boxCenter(box1)).toEqual([2.5, 3.5, 4.5]);
expect(boxCenter(box2)).toEqual([2.5, 3.5, 4.5]);
expect(boxCenter(box3)).toEqual([3.5, 4.5, 5.5]);
});
});
53 changes: 53 additions & 0 deletions typescript/packages/subsurface-viewer/src/utils/BoundingBox3D.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
/**
* 3D bounding box defined as [xmin, ymin, zmin, xmax, ymax, zmax].
*/
export type BoundingBox3D = [number, number, number, number, number, number];

/**
* Returns the bounding box encompassing both boxes.
* @param box1 first box.
* @param box2 second box.
* @param defaultBox in case both boxes are undefined.
* @returns the bounding box encompassing both boxes.
*/
export const boxUnion = (
box1: BoundingBox3D | undefined,
box2: BoundingBox3D | undefined,
defaultBox: BoundingBox3D = [0, 0, 0, 1, 1, 1]
): BoundingBox3D => {
if (box1 === undefined) {
return box2 ?? defaultBox;
}
if (box2 === undefined) {
return box1 ?? defaultBox;
}

const xmin = Math.min(box1[0], box2[0]);
const ymin = Math.min(box1[1], box2[1]);
const zmin = Math.min(box1[2], box2[2]);

const xmax = Math.max(box1[3], box2[3]);
const ymax = Math.max(box1[4], box2[4]);
const zmax = Math.max(box1[5], box2[5]);
return [xmin, ymin, zmin, xmax, ymax, zmax];
};

/**
* Returns the center of the bounding box.
* @param box1 bounding box.
* @returns the center of the bounding box.
*/
export const boxCenter = (box: BoundingBox3D): [number, number, number] => {
const xmin = box[0];
const ymin = box[1];
const zmin = box[2];

const xmax = box[3];
const ymax = box[4];
const zmax = box[5];
return [
xmin + 0.5 * (xmax - xmin),
ymin + 0.5 * (ymax - ymin),
zmin + 0.5 * (zmax - zmin),
];
};

0 comments on commit 849b839

Please sign in to comment.