Skip to content

Commit

Permalink
clean up code for pr review
Browse files Browse the repository at this point in the history
  • Loading branch information
Michael Büßemeyer authored and Michael Büßemeyer committed Dec 6, 2024
1 parent 47e5173 commit 1cd5578
Show file tree
Hide file tree
Showing 7 changed files with 85 additions and 78 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import {
IDENTITY_TRANSFORM,
getTranslationBackToOriginalPosition,
AXIS_TO_TRANSFORM_INDEX,
haveAllLayersSameRotation,
doAllLayersHaveTheSameRotation,
} from "oxalis/model/accessors/dataset_layer_rotation_accessor";
import BoundingBox from "oxalis/model/bucket_data_handling/bounding_box";

Expand Down Expand Up @@ -43,12 +43,16 @@ export const AxisRotationFormItem: React.FC<AxisRotationFormItemProps> = ({
() => getDatasetBoundingBoxFromLayers(dataLayers),
[dataLayers],
);
// Update the transformations in case the user changes the dataset bounding box
// Update the transformations in case the user changes the dataset bounding box.
useEffect(() => {
if (datasetBoundingBox == null || dataLayers[0].coordinateTransformations?.length !== 5) {
if (
datasetBoundingBox == null ||
dataLayers[0].coordinateTransformations?.length !== 5 ||
!form
) {
return;
}
const rotationValues = form?.getFieldValue(["datasetRotation"]);
const rotationValues = form.getFieldValue(["datasetRotation"]);
const transformations = [
getTranslationToOrigin(datasetBoundingBox),
getRotationMatrixAroundAxis("x", rotationValues["x"]),
Expand All @@ -62,19 +66,22 @@ export const AxisRotationFormItem: React.FC<AxisRotationFormItemProps> = ({
coordinateTransformations: transformations,
};
});
form?.setFieldValue(["dataSource", "dataLayers"], dataLayersWithUpdatedTransforms);
form.setFieldValue(["dataSource", "dataLayers"], dataLayersWithUpdatedTransforms);
}, [datasetBoundingBox, dataLayers, form]);

const setMatrixRotationsForAllLayer = useCallback(
(rotationInDegrees: number): void => {
const rotationInRadians = rotationInDegrees * (Math.PI / 180);
const rotationMatrix = getRotationMatrixAroundAxis(axis, rotationInRadians);
const dataLayers: APIDataLayer[] = form?.getFieldValue(["dataSource", "dataLayers"]);
if (!form) {
return;
}
const dataLayers: APIDataLayer[] = form.getFieldValue(["dataSource", "dataLayers"]);
const datasetBoundingBox = getDatasetBoundingBoxFromLayers(dataLayers);
if (datasetBoundingBox == null) {
return;
}

const rotationInRadians = rotationInDegrees * (Math.PI / 180);
const rotationMatrix = getRotationMatrixAroundAxis(axis, rotationInRadians);
const dataLayersWithUpdatedTransforms: APIDataLayer[] = dataLayers.map((layer) => {
let transformations = layer.coordinateTransformations;
if (transformations == null || transformations.length !== 5) {
Expand All @@ -92,7 +99,7 @@ export const AxisRotationFormItem: React.FC<AxisRotationFormItemProps> = ({
coordinateTransformations: transformations,
};
});
form?.setFieldValue(["dataSource", "dataLayers"], dataLayersWithUpdatedTransforms);
form.setFieldValue(["dataSource", "dataLayers"], dataLayersWithUpdatedTransforms);
},
[axis, form],
);
Expand All @@ -112,7 +119,7 @@ export const AxisRotationFormItem: React.FC<AxisRotationFormItemProps> = ({
<FormItem
name={["datasetRotation", axis]}
colon={false}
label=" " /* Empty label is useful for correct formatting*/
label=" " /* Whitespace label is needed for correct formatting*/
>
<InputNumber
min={0}
Expand Down Expand Up @@ -141,7 +148,7 @@ export const AxisRotationSettingForDataset: React.FC<AxisRotationSettingForDatas
form,
}: AxisRotationSettingForDatasetProps) => {
const dataLayers: APIDataLayer[] = form?.getFieldValue(["dataSource", "dataLayers"]);
const isRotationOnly = useMemo(() => haveAllLayersSameRotation(dataLayers), [dataLayers]);
const isRotationOnly = useMemo(() => doAllLayersHaveTheSameRotation(dataLayers), [dataLayers]);

if (!isRotationOnly) {
return (
Expand All @@ -158,7 +165,7 @@ export const AxisRotationSettingForDataset: React.FC<AxisRotationSettingForDatas
<li>Translation back to the original position</li>
</ul>
To easily enable this setting, delete all coordinateTransformations of all layers in the
advanced tab, save and reload.
advanced tab, save and reload the dataset settings.
</div>
}
>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ import { defaultContext } from "@tanstack/react-query";
import { getReadableURLPart } from "oxalis/model/accessors/dataset_accessor";
import type { DatasetRotation } from "./dataset_rotation_form_item";
import {
haveAllLayersSameRotation,
doAllLayersHaveTheSameRotation,
getRotationFromTransformation,
} from "oxalis/model/accessors/dataset_layer_rotation_accessor";

Expand Down Expand Up @@ -200,7 +200,8 @@ class DatasetSettingsView extends React.PureComponent<PropsWithFormAndRouter, St
form.setFieldsValue({
dataSource,
});
if (haveAllLayersSameRotation(dataSource.dataLayers)) {
// Retrieve the initial dataset rotation settings from the data source config.
if (doAllLayersHaveTheSameRotation(dataSource.dataLayers)) {
const firstLayerTransformations = dataSource.dataLayers[0].coordinateTransformations;
let initialDatasetRotationSettings: DatasetRotation;
if (!firstLayerTransformations || firstLayerTransformations.length !== 5) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -262,12 +262,11 @@ class PlaneMaterialFactory {
value: 0,
};
const layer = getLayerByName(dataset, dataLayer.name);
const res = invertAndTranspose(
getTransformsForLayer(dataset, layer, nativelyRenderedLayerName).affineMatrix,
);

this.uniforms[`${layerName}_transform`] = {
value: res,
value: invertAndTranspose(
getTransformsForLayer(dataset, layer, nativelyRenderedLayerName).affineMatrix,
),
};
this.uniforms[`${layerName}_has_transform`] = {
value: !_.isEqual(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ export const IDENTITY_TRANSFORM: CoordinateTransformation = {
matrix: IDENTITY_MATRIX,
};

// cf. https://en.wikipedia.org/wiki/Rotation_matrix#In_three_dimensions
const sinusLocationOfRotationInMatrix = {
x: [2, 1],
y: [0, 2],
Expand Down Expand Up @@ -77,7 +78,7 @@ export function getRotationFromTransformation(
const sinOfAngle = matrix[sinusLocation[0]][sinusLocation[1]];
const cosOfAngle = matrix[cosineLocation[0]][cosineLocation[1]];
const rotation =
Math.abs(cosOfAngle) > 1e-6
Math.abs(cosOfAngle) > 1e-6 // Avoid division by zero
? Math.atan2(sinOfAngle, cosOfAngle)
: sinOfAngle > 0
? Math.PI / 2
Expand All @@ -92,15 +93,15 @@ export function getTranslationToOrigin(bbox: BoundingBox): AffineTransformation
const center = bbox.getCenter();
const translationMatrix = new THREE.Matrix4()
.makeTranslation(-center[0], -center[1], -center[2])
.transpose();
.transpose(); // Column-major to row-major
return { type: "affine", matrix: flatToNestedMatrix(translationMatrix.toArray()) };
}

export function getTranslationBackToOriginalPosition(bbox: BoundingBox): AffineTransformation {
const center = bbox.getCenter();
const translationMatrix = new THREE.Matrix4()
.makeTranslation(center[0], center[1], center[2])
.transpose();
.transpose(); // Column-major to row-major
return { type: "affine", matrix: flatToNestedMatrix(translationMatrix.toArray()) };
}
export function getRotationMatrixAroundAxis(
Expand All @@ -109,13 +110,38 @@ export function getRotationMatrixAroundAxis(
): AffineTransformation {
const euler = new THREE.Euler();
euler[axis] = angleInRadians;
const rotationMatrix = new THREE.Matrix4().makeRotationFromEuler(euler).transpose();
const rotationMatrix = new THREE.Matrix4().makeRotationFromEuler(euler).transpose(); // Column-major to row-major
const matrixWithoutNearlyZeroValues = rotationMatrix
.toArray()
// Avoid nearly zero values due to floating point arithmetic inaccuracies.
.map((value) => (Math.abs(value) < Number.EPSILON ? 0 : value)) as Matrix4x4;
return { type: "affine", matrix: flatToNestedMatrix(matrixWithoutNearlyZeroValues) };
}

function memoizeWithThreeKeys<A, B, C, T>(fn: (a: A, b: B, c: C) => T) {
const map = new MultiKeyMap<A | B | C, T, [A, B, C]>();
return (a: A, b: B, c: C): T => {
let res = map.get([a, b, c]);
if (res === undefined) {
res = fn(a, b, c);
map.set([a, b, c], res);
}
return res;
};
}

function memoizeWithTwoKeys<A, B, T>(fn: (a: A, b: B) => T) {
const map = new MultiKeyMap<A | B, T, [A, B]>();
return (a: A, b: B): T => {
let res = map.get([a, b]);
if (res === undefined) {
res = fn(a, b);
map.set([a, b], res);
}
return res;
};
}

// Returns the transforms (if they exist) for a layer as
// they are defined in the dataset properties.
function _getOriginalTransformsForLayerOrNull(
Expand Down Expand Up @@ -150,7 +176,7 @@ export const getOriginalTransformsForLayerOrNull = memoizeWithTwoKeys(
_getOriginalTransformsForLayerOrNull,
);

export function isLayerWithoutTransformConfigSupport(layer: APIDataLayer | APISkeletonLayer) {
export function isLayerWithoutTransformationConfigSupport(layer: APIDataLayer | APISkeletonLayer) {
return (
layer.category === "skeleton" || (layer.category === "segmentation" && !layer.fallbackLayer)
);
Expand All @@ -161,7 +187,7 @@ function _getTransformsForLayerOrNull(
layer: APIDataLayer | APISkeletonLayer,
nativelyRenderedLayerName: string | null,
): Transform | null {
if (isLayerWithoutTransformConfigSupport(layer)) {
if (isLayerWithoutTransformationConfigSupport(layer)) {
return getTransformsForLayerWithoutTransformationConfigOrNull(
dataset,
nativelyRenderedLayerName,
Expand Down Expand Up @@ -193,30 +219,6 @@ function _getTransformsForLayerOrNull(
return chainTransforms(layerTransforms, inverseNativeTransforms);
}

function memoizeWithThreeKeys<A, B, C, T>(fn: (a: A, b: B, c: C) => T) {
const map = new MultiKeyMap<A | B | C, T, [A, B, C]>();
return (a: A, b: B, c: C): T => {
let res = map.get([a, b, c]);
if (res === undefined) {
res = fn(a, b, c);
map.set([a, b, c], res);
}
return res;
};
}

function memoizeWithTwoKeys<A, B, T>(fn: (a: A, b: B) => T) {
const map = new MultiKeyMap<A | B, T, [A, B]>();
return (a: A, b: B): T => {
let res = map.get([a, b]);
if (res === undefined) {
res = fn(a, b);
map.set([a, b], res);
}
return res;
};
}

export const getTransformsForLayerOrNull = memoizeWithThreeKeys(_getTransformsForLayerOrNull);
export function getTransformsForLayer(
dataset: APIDataset,
Expand All @@ -237,28 +239,26 @@ function _getTransformsForLayerWithoutTransformationConfigOrNull(
nativelyRenderedLayerName: string | null,
): Transform | null {
const layers = dataset.dataSource.dataLayers;
const doAllLayersHaveTheSameRotation = haveAllLayersSameRotation(layers);
const allLayersSameRotation = doAllLayersHaveTheSameRotation(layers);
if (nativelyRenderedLayerName == null) {
// No layer is requested to be rendered natively. -> We can use each layer's transforms as is.
if (!doAllLayersHaveTheSameRotation) {
if (!allLayersSameRotation) {
// If the dataset's layers do not have a consistent transformation (which only rotates the dataset),
// we cannot guess what transformation should be applied to the layer.
// As skeleton layer and volume layer without fallback don't have a transforms property currently.
return null;
}

// The skeleton layer needs transformed just like the other layers. Thus, we simply use the first usable layer.
// Filtering for a layer that might actually have transforms prevents an infinite loop
// between cyclic calls of _getTransformsForLayerWithoutTransformationConfigOrNull and getTransformsForLayerOrNull.
// The skeleton layer / volume layer without fallback needs transformed just like the other layers.
// Thus, we simply use the first usable layer which supports transforms.
const usableReferenceLayer = layers.find(
(layer) =>
layer.category === "color" || (layer.category === "segmentation" && layer.fallbackLayer),
(layer) => !isLayerWithoutTransformationConfigSupport(layer),
);
const someLayersTransformsMaybe = usableReferenceLayer
? getTransformsForLayerOrNull(dataset, usableReferenceLayer, nativelyRenderedLayerName)
: null;
return someLayersTransformsMaybe;
} else if (nativelyRenderedLayerName != null && doAllLayersHaveTheSameRotation) {
} else if (nativelyRenderedLayerName != null && allLayersSameRotation) {
// If all layers have the same transformations and at least one is rendered natively, this means that all layer should be rendered natively.
return null;
}
Expand Down Expand Up @@ -303,7 +303,7 @@ function _getTransformsPerLayer(
return transformsPerLayer;
}

export const getTransformsPerLayer = memoizeOne(_getTransformsPerLayer);
export const getTransformsPerLayer = memoizeWithTwoKeys(_getTransformsPerLayer);

export function getInverseSegmentationTransformer(
state: OxalisState,
Expand Down Expand Up @@ -367,7 +367,7 @@ function isRotationOnly(transformation?: AffineTransformation) {
/* This function checks if all layers have the same transformation settings that represent
* a translation to the dataset center and a rotation around each axis and a translation back.
* All together this makes 5 affine transformation matrices. */
function _haveAllLayersSameRotation(dataLayers: Array<APIDataLayer>): boolean {
function _doAllLayersHaveTheSameRotation(dataLayers: Array<APIDataLayer>): boolean {
const firstDataLayerTransformations = dataLayers[0]?.coordinateTransformations;
if (firstDataLayerTransformations == null || firstDataLayerTransformations.length === 0) {
// No transformations in all layers compatible with setting a rotation for the whole dataset.
Expand Down Expand Up @@ -411,4 +411,4 @@ function _haveAllLayersSameRotation(dataLayers: Array<APIDataLayer>): boolean {
return true;
}

export const haveAllLayersSameRotation = _.memoize(_haveAllLayersSameRotation);
export const doAllLayersHaveTheSameRotation = _.memoize(_doAllLayersHaveTheSameRotation);
6 changes: 3 additions & 3 deletions frontend/javascripts/oxalis/model_initialization.ts
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ import {
isFeatureAllowedByPricingPlan,
} from "admin/organization/pricing_plan_utils";
import { convertServerAdditionalAxesToFrontEnd } from "./model/reducers/reducer_helpers";
import { haveAllLayersSameRotation } from "./model/accessors/dataset_layer_rotation_accessor";
import { doAllLayersHaveTheSameRotation } from "./model/accessors/dataset_layer_rotation_accessor";
import type { Mutable } from "types/globals";

export const HANDLED_ERROR = "error_was_handled";
Expand Down Expand Up @@ -478,7 +478,7 @@ function getMergedDataLayersFromDatasetAndVolumeTracings(

const originalLayers = dataset.dataSource.dataLayers;
const newLayers = originalLayers.slice();
const doAllLayersHaveTheSameRotation = haveAllLayersSameRotation(originalLayers);
const allLayersSameRotation = doAllLayersHaveTheSameRotation(originalLayers);

for (const tracing of tracings) {
// The tracing always contains the layer information for the user segmentation.
Expand All @@ -497,7 +497,7 @@ function getMergedDataLayersFromDatasetAndVolumeTracings(
const mags = tracing.mags || [];
const tracingHasMagList = mags.length > 0;
let coordinateTransformsMaybe = {};
if (doAllLayersHaveTheSameRotation) {
if (allLayersSameRotation) {
coordinateTransformsMaybe = {
coordinateTransformations: originalLayers?.[0].coordinateTransformations,
};
Expand Down
6 changes: 3 additions & 3 deletions frontend/javascripts/oxalis/store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -339,9 +339,9 @@ export type DatasetConfiguration = {
// layer cancel each other out.
// If nativelyRenderedLayerName is null, all layers are rendered
// as their transforms property signal it.
// Currently, the skeleton layer does not have transforms as a stored
// property. So, to render the skeleton layer natively, nativelyRenderedLayerName
// can be set to null.
// Currently, the skeleton layer and volume layer without fallback do not have transforms
// as a stored property. So, to render the skeleton layer natively,
// nativelyRenderedLayerName can be set to null.
readonly nativelyRenderedLayerName: string | null;
};

Expand Down
Loading

0 comments on commit 1cd5578

Please sign in to comment.