Skip to content

SpatialCanvas displays image along with other layers #18

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.

Sign up for GitHub

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

Open
xinaesthete wants to merge 6 commits into
base: main
Choose a base branch
from
Open

SpatialCanvas displays image along with other layers #18

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
68c2cb2
First pass on images with other elements
xinaesthete Feb 3, 2026
2ca9c02
add pnpm-lock missing from previous commit
xinaesthete Feb 3, 2026
6437972
ignore z coordinate on points
xinaesthete Feb 3, 2026
cd9bd7a
fix types in build, add some comments
xinaesthete Feb 4, 2026
7af367b
make transformations toArray/toMatrix operate only on spatial axes, t…
xinaesthete Feb 4, 2026
9508614
lint remove duplicate elementKey in ImageLayerConfig and avoid undefi…
xinaesthete Feb 4, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 42 additions & 8 deletions packages/core/src/schemas/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,12 @@ const transformationSchema: z.ZodType<Transformation> = z.lazy(() =>

export const coordinateTransformationSchema = z.array(transformationSchema).min(1);

const spaceUnitSchema = z.enum([
/**
* Spatial units from OME-NGFF specification.
* Valid UDUNITS-2 spatial units.
* @see https://github.com/ome/ngff/blob/26039d997f16509f4ef7f4006ea641bef73733f7/rfc/5/versions/1/index.md?plain=1#L131
*/
export const spaceUnitSchema = z.enum([
'angstrom',
'attometer',
'centimeter',
Expand Down Expand Up @@ -135,7 +140,13 @@ const spaceUnitSchema = z.enum([
'zeptometer',
'zettameter',
]);
const timeUnitSchema = z.enum([

/**
* Time units from OME-NGFF specification.
* Valid UDUNITS-2 time units.
* @see https://github.com/ome/ngff/blob/26039d997f16509f4ef7f4006ea641bef73733f7/rfc/5/versions/1/index.md?plain=1#L132
*/
export const timeUnitSchema = z.enum([
'attosecond',
'centisecond',
'day',
Expand All @@ -160,14 +171,37 @@ const timeUnitSchema = z.enum([
'zeptosecond',
'zettasecond',
]);

/**
* SHOULD contain the field “unit” to specify the physical unit of this dimension.
* Extract enum values from a zod enum schema as a Set of strings.
* Accesses the internal _def.values array from zod enum schemas.
*/
function getEnumValues(schema: z.ZodEnum<Record<string, string>>): Set<string> {
// zod enum schemas have an internal _def.values array
const values = (schema as unknown as { _def: { values: readonly string[] } })._def.values;
return new Set(values);
}
Comment on lines 175 to +183
Copy link
Contributor

@coderabbitai coderabbitai bot Feb 4, 2026

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

🧩 Analysis chain

🌐 Web query:

Zod enum options property public API documentation

💡 Result:

z.enum(...) (a ZodEnum) exposes an .options property as part of the documented API, which returns the allowed values as a tuple (e.g. ["Salmon", "Tuna", "Trout"]). See the enums documentation showing FishEnum.options. [1], [2]

🏁 Script executed:

# First, check the Zod version and the file in question
cd packages/core && cat package.json | grep -A 2 -B 2 '"zod"'

Repository: Taylor-CCB-Group/SpatialData.js

Length of output: 177

🏁 Script executed:

# Look at the actual file to confirm the code snippet
sed -n '175,183p' src/schemas/index.ts

Repository: Taylor-CCB-Group/SpatialData.js

Length of output: 138

🏁 Script executed:

# Check if there are other uses of this pattern in the codebase
rg "_def\.values" --type ts

Repository: Taylor-CCB-Group/SpatialData.js

Length of output: 395

🏁 Script executed:

# Check if .options is used elsewhere in the codebase
rg "\.options" packages/core/src --type ts -B 2 -A 2

Repository: Taylor-CCB-Group/SpatialData.js

Length of output: 396

🏁 Script executed:

# Navigate to the correct directory and read the file
cat -n packages/core/src/schemas/index.ts | sed -n '170,190p'

Repository: Taylor-CCB-Group/SpatialData.js

Length of output: 909

🏁 Script executed:

# Check for imports of zod to understand version
rg "import.*from.*['\"]zod" packages/core/src/schemas/index.ts -A 5 -B 5

Repository: Taylor-CCB-Group/SpatialData.js

Length of output: 497

🏁 Script executed:

# Check how getEnumValues is used in the codebase
rg "getEnumValues" packages/core --type ts -B 2 -A 2

Repository: Taylor-CCB-Group/SpatialData.js

Length of output: 1197

🏁 Script executed:

# Find the definitions of spaceUnitSchema and timeUnitSchema
rg "spaceUnitSchema\s*=" packages/core/src/schemas/index.ts -B 2 -A 5

Repository: Taylor-CCB-Group/SpatialData.js

Length of output: 313

🏁 Script executed:

# Find timeUnitSchema definition
rg "timeUnitSchema\s*=" packages/core/src/schemas/index.ts -B 2 -A 5

Repository: Taylor-CCB-Group/SpatialData.js

Length of output: 311

Replace _def.values with the public .options property.

The function accesses _def.values, which is an internal implementation detail of Zod that may break on version upgrades. Additionally, the type signature is incorrect—z.ZodEnum uses a tuple type, not Record<string, string>.

Use the public .options property instead:

♻️ Suggested refactor 
-/**
- * Extract enum values from a zod enum schema as a Set of strings.
- * Accesses the internal _def.values array from zod enum schemas.
- */
-function getEnumValues(schema: z.ZodEnum<Record<string, string>>): Set<string> {
-  // zod enum schemas have an internal _def.values array
-  const values = (schema as unknown as { _def: { values: readonly string[] } })._def.values;
-  return new Set(values);
-}
+/**
+ * Extract enum values from a zod enum schema as a Set of strings.
+ * Uses the public `.options` property available on zod enum schemas.
+ */
+function getEnumValues<T extends readonly [string, ...string[]]>(schema: z.ZodEnum<T>): Set<T[number]> {
+  return new Set(schema.options);
+}
🤖 Prompt for AI Agents 
In `@packages/core/src/schemas/index.ts` around lines 175 - 183, The getEnumValues
function is using the internal _def.values and an incorrect type for z.ZodEnum;
update it to use the public schema.options property and fix the type signature
to a Zod enum tuple (e.g., z.ZodEnum<readonly string[]> or z.ZodEnum<[string,
...string[]]>). Specifically, in getEnumValues replace the cast/access of
_def.values with schema.options and ensure the function signature and types
reflect a Zod enum tuple while still returning new Set<string>(schema.options).


/**
* Set of all valid spatial unit strings from OME-NGFF specification.
* Derived from spaceUnitSchema to ensure consistency.
* @see spaceUnitSchema
*/
export const SPATIAL_UNITS: Set<string> = getEnumValues(spaceUnitSchema);

/**
* Set of all valid time unit strings from OME-NGFF specification.
* Derived from timeUnitSchema to ensure consistency.
* @see timeUnitSchema
*/
export const TIME_UNITS: Set<string> = getEnumValues(timeUnitSchema);
/**
* SHOULD contain the field "unit" to specify the physical unit of this dimension.
* The value SHOULD be one of the following strings, which are valid units according to UDUNITS-2.
* https://github.com/ome/ngff/blob/26039d997f16509f4ef7f4006ea641bef73733f7/rfc/5/versions/1/index.md?plain=1#L130
* -- we could try to be better about distinguishing time/space units etc,
* formally expressing that relationship.
* For now, this whole type basically resolves to string, because other arbitrary values are also allowed
* but we'll certainly care about units more in future.
* @see https://github.com/ome/ngff/blob/26039d997f16509f4ef7f4006ea641bef73733f7/rfc/5/versions/1/index.md?plain=1#L130
*
* Note: This schema is relaxed to allow arbitrary string values (e.g., generic "unit" placeholder)
* for backward compatibility, but prefers validated spatial and time units when available.
*/
const axisUnitSchema = z.union([spaceUnitSchema, timeUnitSchema, z.string()]);
const axisSchema = z.union([
Expand Down
117 changes: 113 additions & 4 deletions packages/core/src/transformations/transformations.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,91 @@
import { Matrix4 } from '@math.gl/core';
import type { CoordinateTransformation } from '../schemas';
import type { CoordinateTransformation, Axis } from '../schemas';

/**
* Coordinate system reference from NGFF transformations.
* @see https://github.com/ome/ngff/blob/main/rfc/5/versions/1/index.md
*
* Note: `axes` is optional for backward compatibility with older NGFF data, but should be provided
* for proper transformation handling when coordinate systems include non-spatial axes (e.g., "cyx").
* Without axes, transformations fall back to direct mapping which assumes all values are spatial.
* ^^ Do we really need to be supporting this? ^^
*/
export interface CoordinateSystemRef {
name: string;
axes?: Array<{ name: string; type?: string; unit?: string }>;
axes?: Axis[];
}

/**
* Map spatial axis values to XYZ coordinates based on axis names.
*
* Axes should be validated to have proper `type` fields by the time they reach this function.
* We check `type === 'space'` to identify spatial axes and map them by name to x, y, z coordinates.
*
* Note: Future work could handle mixed units (e.g., different spatial units like 'micrometer' vs 'meter')
* by converting to a common unit for order-of-magnitude consistency in transformations.
*
* @param values - Full transformation value array (ordered according to input axes)
* @param axes - Array of axis definitions (should be validated Axis types)
* @param defaultValue - Default value to use when padding (1 for scale, 0 for translation)
* @returns Array of [x, y, z] values mapped based on axis names
*/
function mapSpatialValuesToXYZ(
values: number[],
axes?: Axis[],
defaultValue = 1
): [number, number, number] {
if (!axes || axes.length === 0) {
// No axes specified - use direct mapping (backward compatibility)
console.warn("legacy data with no input axis specification - not really expecting to get here?")
const [x = defaultValue, y = defaultValue, z = defaultValue] = values;
return [x, y, z];
}

// Map values to Matrix4 dimensions based on axis name
// Matrix4 uses standard x, y, z ordering
let xValue = defaultValue;
let yValue = defaultValue;
let zValue = defaultValue;

// Track spatial axes in order for fallback mapping
const spatialAxesInOrder: Array<{ name: string; value: number }> = [];

for (let i = 0; i < axes.length && i < values.length; i++) {
const axis = axes[i];
if (axis.type === 'space') {
const axisName = axis.name.toLowerCase();
const value = values[i] ?? defaultValue;

// Map by exact axis name match (most common case: "x", "y", "z")
if (axisName === 'x' && xValue === defaultValue) {
xValue = value;
} else if (axisName === 'y' && yValue === defaultValue) {
yValue = value;
} else if (axisName === 'z' && zValue === defaultValue) {
zValue = value;
} else {
// Store for fallback mapping if name doesn't match exactly
spatialAxesInOrder.push({ name: axisName, value });
}
}
}

// Fallback: if we have unmapped spatial axes, map them in order
// This handles cases where axis names don't match x/y/z exactly
// but preserves the spatial ordering (first → x, second → y, third → z)
let fallbackIndex = 0;
for (const { value } of spatialAxesInOrder) {
if (fallbackIndex === 0 && xValue === defaultValue) {
xValue = value;
} else if (fallbackIndex === 1 && yValue === defaultValue) {
yValue = value;
} else if (fallbackIndex === 2 && zValue === defaultValue) {
zValue = value;
}
fallbackIndex++;
}

return [xValue, yValue, zValue];
}
Comment on lines +32 to 89
Copy link
Contributor

@coderabbitai coderabbitai bot Feb 4, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Bug: Axis values equal to defaultValue can be incorrectly overwritten.

The conditions on lines 60-68 check xValue === defaultValue before assigning. If a legitimate axis value happens to equal the default (e.g., scale = [1, 2, 3] where x-scale is actually 1), the value won't be assigned initially, and a later fallback axis could overwrite it.

Consider tracking whether each dimension has been explicitly set instead of comparing against the default value:

🐛 Proposed fix using explicit assignment tracking 
   // Map values to Matrix4 dimensions based on axis name
   // Matrix4 uses standard x, y, z ordering
   let xValue = defaultValue;
   let yValue = defaultValue;
   let zValue = defaultValue;
+  let xSet = false;
+  let ySet = false;
+  let zSet = false;
   
   // Track spatial axes in order for fallback mapping
   const spatialAxesInOrder: Array<{ name: string; value: number }> = [];
   
   for (let i = 0; i < axes.length && i < values.length; i++) {
     const axis = axes[i];
     if (axis.type === 'space') {
       const axisName = axis.name.toLowerCase();
       const value = values[i] ?? defaultValue;
       
       // Map by exact axis name match (most common case: "x", "y", "z")
-      if (axisName === 'x' && xValue === defaultValue) {
+      if (axisName === 'x' && !xSet) {
         xValue = value;
-      } else if (axisName === 'y' && yValue === defaultValue) {
+        xSet = true;
+      } else if (axisName === 'y' && !ySet) {
         yValue = value;
-      } else if (axisName === 'z' && zValue === defaultValue) {
+        ySet = true;
+      } else if (axisName === 'z' && !zSet) {
         zValue = value;
+        zSet = true;
       } else {
         // Store for fallback mapping if name doesn't match exactly
         spatialAxesInOrder.push({ name: axisName, value });
       }
     }
   }
   
   // Fallback: if we have unmapped spatial axes, map them in order
-  let fallbackIndex = 0;
   for (const { value } of spatialAxesInOrder) {
-    if (fallbackIndex === 0 && xValue === defaultValue) {
+    if (!xSet) {
       xValue = value;
-    } else if (fallbackIndex === 1 && yValue === defaultValue) {
+      xSet = true;
+    } else if (!ySet) {
       yValue = value;
-    } else if (fallbackIndex === 2 && zValue === defaultValue) {
+      ySet = true;
+    } else if (!zSet) {
       zValue = value;
+      zSet = true;
     }
-    fallbackIndex++;
   }
🤖 Prompt for AI Agents 
In `@packages/core/src/transformations/transformations.ts` around lines 32 - 89,
The function mapSpatialValuesToXYZ can misinterpret legitimate values equal to
defaultValue as unassigned; change the logic to track explicit assignment
booleans (e.g., xSet, ySet, zSet) instead of comparing against defaultValue: set
the corresponding flag to true whenever you assign xValue/yValue/zValue inside
the first loop (for axis.type === 'space'), use those flags when deciding
whether to skip assignment, and in the fallback loop only assign to dimensions
whose flag is still false (and then set the flag). This preserves real values
equal to defaultValue and prevents incorrect overwrites from the
spatialAxesInOrder fallback.


/**
Expand Down Expand Up @@ -55,7 +134,12 @@ export class Translation extends BaseTransformation {
get type() { return 'translation' as const; }

toArray(): number[] {
const [tx, ty, tz = 0] = this.translation;
// Transformation values are ordered according to the input coordinate system axes.
// For example, if input axes are ["c", "y", "x"], then translation[0] corresponds to "c",
// translation[1] to "y", and translation[2] to "x". We map spatial values to Matrix4
// dimensions based on axis names (x→x, y→y, z→z) to preserve correct orientation.
const [tx, ty, tz] = mapSpatialValuesToXYZ(this.translation, this.input?.axes, 0);

return [
1, 0, 0, 0,
0, 1, 0, 0,
Expand All @@ -76,7 +160,12 @@ export class Scale extends BaseTransformation {
get type() { return 'scale' as const; }

toArray(): number[] {
const [sx, sy, sz = 1] = this.scale;
// Transformation values are ordered according to the input coordinate system axes.
// For example, if input axes are ["c", "y", "x"], then scale[0] corresponds to "c",
// scale[1] to "y", and scale[2] to "x". We map spatial values to Matrix4
// dimensions based on axis names (x→x, y→y, z→z) to preserve correct orientation.
const [sx, sy, sz] = mapSpatialValuesToXYZ(this.scale, this.input?.axes);

return [
sx, 0, 0, 0,
0, sy, 0, 0,
Expand All @@ -99,6 +188,26 @@ export class Affine extends BaseTransformation {
toArray(): number[] {
const { affine } = this;

// Validate that affine matrix dimensions match expected spatial dimensions
// when axes are specified. We assume affine matrices already represent spatial dimensions only.
if (this.input?.axes) {
const expectedSpatialDims = this.input.axes.filter(axis => axis.type === 'space').length;
const actualDims = affine.length;

// For 2D affine: 2x3 or 3x3 matrices
// For 3D affine: 3x4 or 4x4 matrices
// Warn if there's a mismatch
if (expectedSpatialDims === 2 && actualDims !== 2 && actualDims !== 3) {
console.warn(
`Affine matrix dimensions (${actualDims}x${affine[0]?.length}) don't match expected 2D spatial dimensions. Input axes indicate ${expectedSpatialDims} spatial dimensions. Assuming affine matrix represents spatial dimensions only.`
);
} else if (expectedSpatialDims === 3 && actualDims !== 3 && actualDims !== 4) {
console.warn(
`Affine matrix dimensions (${actualDims}x${affine[0]?.length}) don't match expected 3D spatial dimensions. Input axes indicate ${expectedSpatialDims} spatial dimensions. Assuming affine matrix represents spatial dimensions only.`
);
}
}

if (affine.length === 2 && affine[0].length === 3) {
// 2x3 affine (2D) - common spatialdata format: [[a, b, tx], [c, d, ty]]
const [[a, b, tx], [c, d, ty]] = affine;
Expand Down
1 change: 1 addition & 0 deletions packages/vis/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
"@uiw/react-json-view": "2.0.0-alpha.39",
"@hms-dbmi/viv": "catalog:",
"deck.gl": "catalog:",
"fast-deep-equal": "^3.1.3",
"geotiff": "2.1.4-beta.0",
"zustand": "^5.0.8"
},
Expand Down
Loading