webknossos icon indicating copy to clipboard operation
webknossos copied to clipboard
verified publisher icon scalableminds

Refactor labelling code in preparation to support transformed layers

Open philippotto opened this issue 4 months ago • 2 comments

This PR:

  • renames
    • the term "volume layer" to "section labeller" (at least in the context of section labelling) to avoid confusions with a normal volume layer
    • "add to layer" -> "add to contour list"
      • other renamings
  • ensures annotation geometries are also transformed in the scene (especially, the contour geometry)
  • refactors some transformation related accessors
  • refactors the SectionLabeler as well as the VoxelBuffer class
  • adds a TransformedSectionLabeler which is able to map the used viewport with a transform to another plane to achieve the rotation
    • I also added tests for that, but not all of them succeed yet
    • I want to fix the issue first that some layers are squished when viewed natively because the dataset scale is not respected properly

URL of deployed dev instance (used for testing):

  • https://___.webknossos.xyz

Steps to test:

  • test that volume annotation works as before
  • CI

Issues:

  • contributes to #8965

(Please delete unneeded items, merge only when none are left open)

  • [ ] ~Added changelog entry (create a $PR_NUMBER.md file in unreleased_changes or use ./tools/create-changelog-entry.py)~ no user facing changes

philippotto avatar Oct 27 '25 20:10 philippotto

📝 Walkthrough

Walkthrough

Extracts rotation construction into an exported helper; renames volume-tracing concepts (VolumeLayer/ADD_TO_LAYER → SectionLabeler/ADD_TO_CONTOUR_LIST) across actions, reducers, sagas and tests; shifts segmentation edits to layer-space via a new helper; tightens SceneController visibility and removes Drawing smoothing API.

Changes

Cohort / File(s) Summary
Dashboard Rotation UI
frontend/javascripts/dashboard/dataset/dataset_rotation_form_item.tsx		 Extracted rotational transform construction into exported getRotationalTransformation(datasetBoundingBox, rotationValues): AffineTransformation[]; AxisRotationFormItem now imports/uses it.
Drawing internals
frontend/javascripts/libs/drawing.ts		 Removed smoothing globals/properties and public smoothing API (alpha, smoothLength, smoothLine, setSmoothLength, setAlpha); made several methods private; added null-safety guard in fillArea.
Volume tracing actions & reducers
frontend/javascripts/viewer/model/actions/volumetracing_actions.ts, frontend/javascripts/viewer/model/reducers/volumetracing_reducer.ts, frontend/javascripts/viewer/model/reducers/volumetracing_reducer_helpers.ts, frontend/javascripts/viewer/model/helpers/action_logger_middleware.ts		 Renamed addToLayer / "ADD_TO_LAYER" → addToContourList / "ADD_TO_CONTOUR_LIST" (action creators, types, reducer routing, blacklist); payload fields renamed to positionInLayerSpace; updated related imports/types.
Section labeling refactor
frontend/javascripts/viewer/model/volumetracing/section_labeling.ts		 Replaced VolumeLayer with SectionLabeler; added TransformedSectionLabeler, mapTransformedPlane, getFast3DCoordinateFn, immutable VoxelBuffer2D changes, canonical bases/normals, and many signature/visibility updates.
Sagas & helpers: SectionLabeler migration
frontend/javascripts/viewer/model/sagas/volumetracing_saga.tsx, frontend/javascripts/viewer/model/sagas/volume/helpers.ts, frontend/javascripts/viewer/model/sagas/volume/quick_select/quick_select_heuristic_saga.ts, frontend/javascripts/viewer/model/sagas/volume/volume_interpolation_saga.ts		 Replaced createVolumeLayer → createSectionLabeler, updated flows to use SectionLabeler/TransformedSectionLabeler, renamed finishLayer → finishSectionLabeler, updated voxel-buffer calls and channel/action handling to ADD_TO_CONTOUR_LIST.
Controller: segmentation coordinate flow
frontend/javascripts/viewer/controller/combinations/volume_handlers.ts, frontend/javascripts/viewer/controller/combinations/tool_controls.ts, frontend/javascripts/viewer/view/context_menu.tsx		 Added getUntransformedSegmentationPosition(state, globalPosRounded) to compute layer-space positions; updated draw/erase/move/pick/flood-fill call sites to pass state and layer-space positions and to dispatch contour-list actions.
SceneController visibility & transforms
frontend/javascripts/viewer/controller/scene_controller.ts		 Added explicit private/public modifiers on many fields, introduced flycamRotationEuler, replaced updateUserBoundingBoxesAndMeshesAccordingToTransforms with updateGeometriesToTransforms, and now applies transforms to annotation tools geometry.
Transformation/accessor utilities
frontend/javascripts/viewer/model/accessors/dataset_layer_transformation_accessor.ts, frontend/javascripts/viewer/model/helpers/transformation_helpers.ts		 Added combineCoordinateTransformations(coordinateTransformations, scaleFactor): Transform and memoized getOriginalTransformsForLayerOrNull; thin-plate-spline uses passed scaleFactor; minor refactor in transformPointUnscaled.
Constants & import fixes
frontend/javascripts/viewer/constants.ts, frontend/javascripts/viewer/model/accessors/dataset_accessor.ts, frontend/javascripts/viewer/model/accessors/disabled_tool_accessor.ts, frontend/javascripts/viewer/model/bucket_data_handling/data_cube.ts		 Fixed VectorNIndicies → VectorNIndices renames and updated imports; adjusted imports from volumelayer → section_labeling where applicable.
Tests & mocks
frontend/javascripts/test/*
frontend/javascripts/test/global_mocks.ts, frontend/javascripts/test/model/transformed_section_labeler.spec.ts, frontend/javascripts/test/reducers/volumetracing_reducer.spec.ts, frontend/javascripts/test/sagas/volumetracing/*		 Added Typography.Text to antd mock; added transformed-plane tests; updated tests to use addToContourListAction and SectionLabeler APIs and adjusted helpers/fixtures.
Minor UI / Store tweaks
frontend/javascripts/viewer/view/right-border-tabs/segments_tab/segments_view.tsx, frontend/javascripts/viewer/store.ts		 Renamed parameter to indicate layer-space (somePositionInLayerSpace) in setActiveCell dispatch; updated inline comments to note layer-space coordinates.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

Possibly related PRs

  • scalableminds/webknossos#8159 — overlaps with extraction/changes to dataset_rotation_form_item.tsx and the rotation helper.
  • scalableminds/webknossos#8992 — touches dataset layer transformation utilities and controller world→layer position handling, aligning with getUntransformedSegmentationPosition.
  • scalableminds/webknossos#8493 — refactors SceneController transform application and geometry groups, related to updateGeometriesToTransforms edits.

Suggested reviewers

  • MichaelBuessemeyer

Poem

🐰 I hopped through code and pulled rotations out neat,
I renamed contours so layer edits meet.
I tucked fields away and rewired sagas' art,
Tests blinked awake — the rabbit did its part. 🥕

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 3.45% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately captures the main refactoring objective of renaming and restructuring labelling code to support transformed layers.
Description check ✅ Passed The description provides clear context about the refactoring changes, renamed terms, new classes, and known issues, all of which directly relate to the changeset.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • [ ] 📝 Generate docstrings
📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 94658f8bb06415892ad8f95d55424c00ae00cd30 and 86d70008176088f0457b1b0e350e60043a00cabc.

📒 Files selected for processing (7)
  • frontend/javascripts/viewer/constants.ts
  • frontend/javascripts/viewer/model/accessors/dataset_accessor.ts
  • frontend/javascripts/viewer/model/sagas/volume/floodfill_saga.tsx
  • frontend/javascripts/viewer/model/sagas/volume/proofread_saga.ts
  • frontend/javascripts/viewer/store.ts
  • frontend/javascripts/viewer/view/context_menu.tsx
  • frontend/javascripts/viewer/view/right-border-tabs/segments_tab/segments_view.tsx
🚧 Files skipped from review as they are similar to previous changes (4)
  • frontend/javascripts/viewer/constants.ts
  • frontend/javascripts/viewer/view/context_menu.tsx
  • frontend/javascripts/viewer/model/sagas/volume/floodfill_saga.tsx
  • frontend/javascripts/viewer/model/accessors/dataset_accessor.ts
🧰 Additional context used
🧠 Learnings (8)
📚 Learning: 2024-11-22T17:18:04.217Z 
Learnt from: dieknolle3333
Repo: scalableminds/webknossos PR: 8168
File: frontend/javascripts/oxalis/model/sagas/proofread_saga.ts:1039-1039
Timestamp: 2024-11-22T17:18:04.217Z
Learning: In `frontend/javascripts/oxalis/model/sagas/proofread_saga.ts`, when calling `getMagInfo`, the use of `volumeTracingLayer.resolutions` is intentional and should not be changed to `volumeTracingLayer.mags`.

Applied to files:

  • frontend/javascripts/viewer/store.ts
  • frontend/javascripts/viewer/model/sagas/volume/proofread_saga.ts
📚 Learning: 2025-09-08T13:33:47.097Z 
Learnt from: MichaelBuessemeyer
Repo: scalableminds/webknossos PR: 8824
File: frontend/javascripts/viewer/view/context_menu.tsx:1033-1039
Timestamp: 2025-09-08T13:33:47.097Z
Learning: In frontend/javascripts/viewer/view/context_menu.tsx, the segmentIdLabel intentionally shows the unmapped segment ID (maybeUnmappedSegmentId) in the "within Segment ${id}" text when proofreading is active, as this can be helpful for users. The mapped segment ID (segmentIdAtPosition) is only used as a fallback when the unmapped ID is not available.

Applied to files:

  • frontend/javascripts/viewer/store.ts
  • frontend/javascripts/viewer/model/sagas/volume/proofread_saga.ts
  • frontend/javascripts/viewer/view/right-border-tabs/segments_tab/segments_view.tsx
📚 Learning: 2025-07-22T12:56:47.507Z 
Learnt from: daniel-wer
Repo: scalableminds/webknossos PR: 8787
File: frontend/javascripts/viewer/model/accessors/dataset_layer_transformation_accessor.ts:132-136
Timestamp: 2025-07-22T12:56:47.507Z
Learning: Three.js Vector3 has explicitly defined x, y, and z properties that can be safely accessed via dynamic string indexing when using union types like "x" | "y" | "z". The pattern `vector[axis] = value` where axis is "x" | "y" | "z" does not cause TypeScript strict mode errors because TypeScript can infer the property access is valid.

Applied to files:

  • frontend/javascripts/viewer/store.ts
📚 Learning: 2025-12-11T15:25:53.526Z 
Learnt from: knollengewaechs
Repo: scalableminds/webknossos PR: 9117
File: frontend/javascripts/admin/statistic/time_tracking_overview.tsx:261-279
Timestamp: 2025-12-11T15:25:53.526Z
Learning: Ant Design v6 Select: when using the Select component, consider supplying a prefix prop to render an icon or element before the input for better visual context. Apply this guideline to TS and TSX files across the codebase where Ant Design Select is used; ensure prefix usage is accessible (e.g., provide meaningful aria-label if needed) and avoid unnecessary prefixes on simple inputs.

Applied to files:

  • frontend/javascripts/viewer/store.ts
  • frontend/javascripts/viewer/model/sagas/volume/proofread_saga.ts
  • frontend/javascripts/viewer/view/right-border-tabs/segments_tab/segments_view.tsx
📚 Learning: 2025-05-07T06:17:32.810Z 
Learnt from: philippotto
Repo: scalableminds/webknossos PR: 8602
File: frontend/javascripts/oxalis/model/volumetracing/volume_annotation_sampling.ts:365-366
Timestamp: 2025-05-07T06:17:32.810Z
Learning: The parameter in applyVoxelMap was renamed from `sliceCount` to `sliceOffset` to better reflect its purpose, but this doesn't affect existing call sites since JavaScript/TypeScript function calls are position-based.

Applied to files:

  • frontend/javascripts/viewer/view/right-border-tabs/segments_tab/segments_view.tsx
📚 Learning: 2025-12-11T15:33:06.880Z 
Learnt from: knollengewaechs
Repo: scalableminds/webknossos PR: 9117
File: frontend/javascripts/admin/task/task_search_form.tsx:151-151
Timestamp: 2025-12-11T15:33:06.880Z
Learning: In Ant Design v6, do not use optionFilterProp as a standalone prop on Select. Instead, pass it inside showSearch as optionFilterProp, e.g. showSearch={{ optionFilterProp: 'label' }}. showSearch accepts boolean or an object with keys like optionFilterProp, filterOption, autoClearSearchValue. Update all Select components that use optionFilterProp to adopt the new pattern and adjust types accordingly to maintain compatibility and avoid deprecation warnings.

Applied to files:

  • frontend/javascripts/viewer/view/right-border-tabs/segments_tab/segments_view.tsx
📚 Learning: 2025-12-18T13:11:55.113Z 
Learnt from: hotzenklotz
Repo: scalableminds/webknossos PR: 9156
File: frontend/javascripts/dashboard/dataset/dataset_settings_data_tab.tsx:10-10
Timestamp: 2025-12-18T13:11:55.113Z
Learning: Guideline: When using Ant Design's Flex component in this repository, the align prop accepts standard CSS flexbox values such as 'flex-start', 'flex-end', 'center', 'baseline', and 'stretch'. These map directly to the CSS align-items property. Do not propose or implement changes to use 'end'/'start' for Ant Design Flex components; continue using the standard CSS values (e.g., 'flex-start' and 'flex-end') to ensure correct alignment behavior.

Applied to files:

  • frontend/javascripts/viewer/view/right-border-tabs/segments_tab/segments_view.tsx
📚 Learning: 2025-12-11T15:54:47.778Z 
Learnt from: knollengewaechs
Repo: scalableminds/webknossos PR: 9117
File: frontend/javascripts/dashboard/advanced_dataset/dataset_table.tsx:131-134
Timestamp: 2025-12-11T15:54:47.778Z
Learning: In Ant Design v6, Dropdown uses a flat classNames structure: classNames={{ root: '…' }}. Other components (Select, AutoComplete, Cascader, TreeSelect) use a nested structure. The deprecated overlayClassName prop for Dropdown should be replaced with classNames.root. In reviews, flag Dropdown usage that relies on overlayClassName and replace it with classNames={{ root: '…' }}. If you encounter related components, verify the correct classNames shape (flat for Dropdown, nested for others) and update accordingly. This guideline covers TSX files under the frontend codebase where Ant Design components are used.

Applied to files:

  • frontend/javascripts/viewer/view/right-border-tabs/segments_tab/segments_view.tsx
🧬 Code graph analysis (2)
frontend/javascripts/viewer/store.ts (1)
frontend/javascripts/viewer/constants.ts (1)
  • Vector3 (15-15)
frontend/javascripts/viewer/view/right-border-tabs/segments_tab/segments_view.tsx (3)
frontend/javascripts/viewer/constants.ts (2)
  • Vector3 (15-15)
  • AdditionalCoordinate (3-3)
frontend/javascripts/types/api_types.ts (1)
  • AdditionalCoordinate (33-33)
frontend/javascripts/viewer/model/actions/volumetracing_actions.ts (1)
  • setActiveCellAction (214-226)
⏰ 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: backend-tests
  • GitHub Check: frontend-tests
  • GitHub Check: build-smoketest-push
🔇 Additional comments (4)
frontend/javascripts/viewer/model/sagas/volume/proofread_saga.ts (1)

1082-1082: LGTM!

Minor comment refinement that maintains clarity.

frontend/javascripts/viewer/view/right-border-tabs/segments_tab/segments_view.tsx (1)

259-265: LGTM! Clear parameter renaming improves code clarity.

The parameter rename from somePosition to somePositionInLayerSpace makes the coordinate space explicit, aligning with the broader refactoring effort to clarify layer-space terminology throughout the codebase. The dispatch call correctly passes the renamed parameter to setActiveCellAction.

frontend/javascripts/viewer/store.ts (2)

178-178: LGTM! Documentation clarifies coordinate space.

The comment addition clearly indicates that somePosition is stored in layer space, which aligns with the broader refactoring effort to migrate from global to layer-space coordinates.

210-210: LGTM! Comment accurately reflects coordinate space migration.

The comment update from "global coordinates" to "layer-space coordinates" correctly documents the coordinate space used by contourList, consistent with the broader refactoring to layer-space coordinates throughout the codebase.

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

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

coderabbitai[bot] avatar Oct 27 '25 20:10 coderabbitai[bot]

@MichaelBuessemeyer let me know if we should have a chat about this PR again :)

philippotto avatar Dec 04 '25 12:12 philippotto