Allow selective segment visibility regardless of proofreading tool #8281
Conversation
📝 Walkthrough📝 WalkthroughWalkthroughThe pull request introduces a new feature for selective segment visibility in the frontend application. This enhancement allows users to control the visibility of segments through a toggle in the layer settings tab. The changes span multiple files, including the material factory, shaders, store configuration, and UI components. The implementation adds a new boolean property Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🔇 Additional comments (4)frontend/javascripts/libs/input.ts (3)
The implementation follows existing patterns and includes proper checks for:
The double-click event is correctly:
Also applies to: 511-511
The method correctly:
frontend/javascripts/oxalis/controller/combinations/tool_controls.ts (1)
The implementation:
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
frontend/javascripts/oxalis/shaders/segmentation.glsl.ts (2)
358-392: Add documentation for the alpha increment calculation logic.The implementation is correct, but the complex visibility logic would benefit from documentation explaining:
- The different alpha values and their visual effects
- The interaction between proofreading and normal modes
- The purpose of each condition branch
Add a documentation block like this:
+ /* + * Calculates the alpha increment for segment visibility based on the segment state: + * + * Proofreading mode: + * - Active cell + hovered unmapped: +0.4 (highlight super-voxel) + * - Active cell + hovered: +0.15 (highlight non-hovered parts) + * - Hovered cell: +0.2 + * - Other cells: -alpha if selective visibility enabled + * + * Normal mode: + * - Hovered segment: +0.2 + * - Active cell with selective visibility: +0.15 + * - Other cells with selective visibility: -alpha (hide) + * - Default: no change + */ float getSegmentationAlphaIncrement(float alpha, bool isHoveredSegment, bool isHoveredUnmappedSegment, bool isActiveCell) {
358-392: Consider refactoring for improved readability.The nested conditional logic could be simplified using early returns and helper functions.
Consider this refactor:
- float getSegmentationAlphaIncrement(float alpha, bool isHoveredSegment, bool isHoveredUnmappedSegment, bool isActiveCell) { - if (isProofreading) { - if (isActiveCell) { - return (isHoveredUnmappedSegment - ? 0.4 - : (isHoveredSegment - ? 0.15 - : 0.0 - ) - ); - } else { - return (isHoveredSegment - ? 0.2 - : (selectiveVisibilityInProofreading ? -alpha : 0.0) - ); - } - } - - if (isHoveredSegment) { - return 0.2; - } else if (selectiveSegmentVisibility) { - return isActiveCell ? 0.15 : -alpha; - } else { - return 0.; - } - } + float getProofreadingAlphaIncrement(float alpha, bool isHoveredSegment, bool isHoveredUnmappedSegment, bool isActiveCell) { + if (isActiveCell) { + if (isHoveredUnmappedSegment) return 0.4; + if (isHoveredSegment) return 0.15; + return 0.0; + } + + if (isHoveredSegment) return 0.2; + return selectiveVisibilityInProofreading ? -alpha : 0.0; + } + + float getNormalModeAlphaIncrement(float alpha, bool isHoveredSegment, bool isActiveCell) { + if (isHoveredSegment) return 0.2; + if (!selectiveSegmentVisibility) return 0.0; + return isActiveCell ? 0.15 : -alpha; + } + + float getSegmentationAlphaIncrement(float alpha, bool isHoveredSegment, bool isHoveredUnmappedSegment, bool isActiveCell) { + return isProofreading + ? getProofreadingAlphaIncrement(alpha, isHoveredSegment, isHoveredUnmappedSegment, isActiveCell) + : getNormalModeAlphaIncrement(alpha, isHoveredSegment, isActiveCell); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
frontend/javascripts/oxalis/geometries/materials/plane_material_factory.ts(2 hunks)frontend/javascripts/oxalis/shaders/main_data_shaders.glsl.ts(4 hunks)frontend/javascripts/oxalis/shaders/segmentation.glsl.ts(1 hunks)frontend/javascripts/oxalis/store.ts(1 hunks)frontend/javascripts/oxalis/view/left-border-tabs/layer_settings_tab.tsx(5 hunks)frontend/javascripts/types/schemas/dataset_view_configuration.schema.ts(1 hunks)
🔇 Additional comments (8)
frontend/javascripts/types/schemas/dataset_view_configuration.schema.ts (1)
92-92: LGTM: Configuration property added correctly.
The new selectiveSegmentVisibility property is appropriately added to the dataset view configuration with a sensible default value of false.
frontend/javascripts/oxalis/shaders/main_data_shaders.glsl.ts (1)
9-9: LGTM: Clean integration of the new shader module.
The changes correctly:
- Import the new shader module
- Declare the required uniform
- Replace the inline alpha calculation with the new modular function
Also applies to: 114-114, 297-302
frontend/javascripts/oxalis/store.ts (1)
333-333: LGTM: Property addition follows TypeScript best practices.
The new selectiveSegmentVisibility property is correctly typed and well-placed within the DatasetConfiguration interface.
frontend/javascripts/oxalis/geometries/materials/plane_material_factory.ts (2)
149-151: LGTM: New uniform property follows existing patterns.
The new selectiveSegmentVisibility uniform is correctly initialized to false and follows the same pattern as other uniform properties in the shader material.
568-578: LGTM: Listener setup is consistent with existing patterns.
The listener for selectiveSegmentVisibility is properly implemented:
- Uses the established
listenToStorePropertypattern. - Updates the uniform value when the store property changes.
- Correctly placed with other similar listeners.
frontend/javascripts/oxalis/view/left-border-tabs/layer_settings_tab.tsx (3)
1589-1589: LGTM: Correctly added activeTool to props.
The activeTool is properly added to mapStateToProps to support the conditional disabling of selective visibility in proofreading mode.
1341-1341: LGTM: Added selectiveSegmentVisibility to saved settings.
The selectiveSegmentVisibility setting is correctly added to the list of settings that can be saved as default configuration.
902-928: LGTM: Well-implemented UI toggle for selective segment visibility.
The selective visibility toggle is properly implemented with:
- Clear tooltip explaining the override behavior in proofreading mode.
- Correct conditional disabling based on proofreading mode.
- Consistent styling and spacing with other settings.
URL of deployed dev instance (used for testing):
Steps to test:
Issues:
(Please delete unneeded items, merge only when none are left open)