Skip to content

Conversation

fm3
Copy link
Member

@fm3 fm3 commented Sep 30, 2025

When composing layers from datasets with different voxel sizes, finds a common voxel size and adapts the mags accordingly.

Using the existing functionality from dataset explorers also in compose.

Steps to test:

  • Compose datasets with differing voxel sizes. Example: A dataset with 8,8,8 nm voxel size and a dataset with 64,64,64 nm voxel size are composed. The first layer should then have the mags 1,2,4,8,... and the second 8,16,32,64,...

Issues:


  • Added changelog entry (create a $PR_NUMBER.md file in unreleased_changes or use ./tools/create-changelog-entry.py)
  • Considered common edge cases
  • Needs datastore update after deployment

@fm3 fm3 self-assigned this Sep 30, 2025
Copy link
Contributor

coderabbitai bot commented Sep 30, 2025

📝 Walkthrough

Walkthrough

ComposeService now derives and propagates voxel sizes from layers, adapting layers and voxel size when no transformations are present. It extends ExploreLayerUtils to access adaptation. getLayerFromComposeLayer returns both layer and voxel size. DataSource construction uses the derived voxel size. adaptLayersAndVoxelSize visibility is reduced to protected.

Changes

Cohort / File(s) Summary
Compose workflow and voxel-size propagation
app/models/dataset/ComposeService.scala
- Import ExploreLayerUtils and extend it
- Change getLayerFromComposeLayer to return (StaticLayer, VoxelSize)
- During createDatasource, derive layers and voxelSize from inputs; if no transforms, call adaptLayersAndVoxelSize; otherwise use composeRequest.voxelSize
- Build DataSource with derived voxelSize
Utility visibility tightening
webknossos-datastore/app/com/scalableminds/webknossos/datastore/explore/ExploreLayerUtils.scala
- Change adaptLayersAndVoxelSize visibility from public to protected; signature otherwise unchanged

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

I twitch my ears at scales so fine,
Voxels align—oh what a sign!
Layers hop in measured size,
No more guesses, no disguise.
With guarded tools and careful art,
We compose as one—a perfect start. 🐇✨

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)
Check name Status Explanation
Title Check ✅ Passed Title concisely and accurately summarizes the main functional change—making dataset composition voxel-size-aware—matching the code modifications to ComposeService and ExploreLayerUtils for voxel-size adaptation.
Linked Issues Check ✅ Passed The pull request implements the core objectives from #8879 by extending ComposeService with ExploreLayerUtils and invoking adaptLayersAndVoxelSize to compute a shared voxel size and adjust layer magnifications when composing datasets of differing resolutions, thereby replacing the previous identity transform behavior.
Out of Scope Changes Check ✅ Passed All modified files are within the compose and dataset explorer codepaths and solely introduce voxel-size adaptation logic without altering unrelated functionality, so no out-of-scope changes are present.
Description Check ✅ Passed The description directly addresses adapting voxel sizes when composing layers, matching the implemented changes and linked issue resolution by describing functional behavior and test steps.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch compose-voxel-size-aware

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9ba3843 and 640503f.

📒 Files selected for processing (1)
  • app/models/dataset/ComposeService.scala (5 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
app/models/dataset/ComposeService.scala (2)
webknossos-datastore/app/com/scalableminds/webknossos/datastore/explore/ExploreLayerUtils.scala (1)
  • adaptLayersAndVoxelSize (21-32)
util/src/main/scala/com/scalableminds/util/tools/Fox.scala (1)
  • successful (53-56)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: build-smoketest-push
  • GitHub Check: backend-tests
  • GitHub Check: frontend-tests
🔇 Additional comments (5)
app/models/dataset/ComposeService.scala (5)

6-6: LGTM!

The import is necessary for extending ExploreLayerUtils and accessing adaptLayersAndVoxelSize.


38-41: LGTM!

Extending ExploreLayerUtils provides access to adaptLayersAndVoxelSize, reusing existing functionality as intended.


58-77: LGTM!

The method now returns both the layer and its voxel size, enabling voxel-size-aware composition by propagating the source dataset's scale.


102-108: LGTM with dependency on conditional logic.

The DataSource construction correctly uses the derived voxelSize. However, ensure that the voxel size derivation in the conditional logic above (lines 97-101) is correct for all cases, especially when transformations are present.


97-101: Validate fallback voxelSize when transformations are present
The code bypasses adaptLayersAndVoxelSize whenever any layer has transformations and uses composeRequest.voxelSize directly. Confirm that under those transforms all source layers’ voxel sizes remain compatible with the requested voxelSize—or else derive the target voxelSize from the transformed layers or explicitly document that callers must supply a matching voxelSize.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
🧪 Early access (Sonnet 4.5): enabled

We are currently testing the Sonnet 4.5 model, which is expected to improve code review quality. However, this model may lead to increased noise levels in the review comments. Please disable the early access features if the noise level causes any inconvenience.

Note:

  • Public repositories are always opted into early access features.
  • You can enable or disable early access features from the CodeRabbit UI or by updating the CodeRabbit configuration file.

Comment @coderabbitai help to get the list of available commands and usage tips.

@fm3 fm3 marked this pull request as ready for review September 30, 2025 07:54
@fm3 fm3 requested a review from normanrz September 30, 2025 07:54
@fm3
Copy link
Member Author

fm3 commented Sep 30, 2025

@normanrz do you happen to have some real-world test data for this? I tested with a constructed example and it looked ok, but I think some real test data would be helpful. I’m also not sure if this voxel-size auto-detection breaks composing datasets with incompatible voxel sizes, there are now assertions for that. But that feature might still be desired if you know affine transforms to get everything straight again.

@fm3 fm3 enabled auto-merge (squash) October 1, 2025 08:14
@fm3 fm3 merged commit cd5289f into master Oct 1, 2025
5 checks passed
@fm3 fm3 deleted the compose-voxel-size-aware branch October 1, 2025 08:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Compose doesn't take voxel sizes into account

2 participants