Use mapping from viewconfig when generating thumbnail #8157
Conversation
WalkthroughThe pull request introduces several enhancements and bug fixes to the WEBKNOSSOS application. Key updates include the ability to add metadata in annotations for Trees and Segments, improved slider functionality, and enhanced search capabilities for unnamed segments. Additionally, changes were made to thumbnail handling in the Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
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
🧹 Outside diff range and nitpick comments (2)
CHANGELOG.unreleased.md (1)
28-28: Enhance the changelog entry with deployment requirements and compatibility notes.
While the current entry correctly describes the feature, it should be expanded to include important deployment considerations and compatibility information.
Consider updating the entry to:
- Thumbnails for datasets now use the selected mapping from the view configuration if available. [#8157](https://github.com/scalableminds/webknossos/pull/8157)
+ Thumbnails for datasets now use the selected mapping from the view configuration if available. Requires a datastore update after deployment. The default behavior remains unchanged when no mapping is specified. [#8157](https://github.com/scalableminds/webknossos/pull/8157)Additionally, consider adding a note in the "Breaking Changes" section if this change affects any existing API contracts or behaviors.
app/models/dataset/ThumbnailService.scala (1)
Line range hint 104-135: Consider introducing a case class for the return type.
The method returns a complex 5-tuple which makes the code harder to maintain and understand. Consider introducing a case class to encapsulate these related parameters.
case class ThumbnailParameters(
boundingBox: BoundingBox,
mag: Vec3Int,
intensityRange: Option[(Double, Double)],
colorSettings: Option[ThumbnailColorSettings],
mapping: Option[String]
)Then update the method signature:
private def selectParameters(
viewConfiguration: DatasetViewConfiguration,
usableDataSource: GenericDataSource[DataLayerLike],
layerName: String,
layer: DataLayerLike,
targetMagWidth: Int,
targetMagHeigt: Int,
mappingName: Option[String]
): ThumbnailParameters📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- CHANGELOG.unreleased.md (1 hunks)
- app/models/dataset/ThumbnailService.scala (4 hunks)
🔇 Additional comments (3)
app/models/dataset/ThumbnailService.scala (3)
17-17: LGTM: Import addition is appropriate.
The added import for JsObject is necessary for handling the mapping configuration in the new functionality.
90-90: LGTM: Consistent mapping parameter propagation.
The mapping parameter is correctly propagated through the thumbnail generation and caching pipeline.
Also applies to: 97-97
158-164: LGTM: Mapping extraction implementation is robust.
The implementation follows the established pattern for configuration readers and properly handles JSON validation. However, let's verify that the JSON structure matches the expected format across the codebase.
✅ Verification successful
The previous regex had syntax errors. Let's try with simpler patterns to find related configuration usage and validate the JSON structure consistency.
Implementation is consistent with codebase patterns
The verification confirms that the readMappingName implementation in ThumbnailService follows established patterns in the codebase. The search results show:
- The method's JSON structure for mapping configuration (with "name" field) is consistent with other parts of the codebase, particularly in mesh and volume tracing services
- The
viewConfiguration.get("layers")pattern is used consistently within theThumbnailServiceclass itself - The implementation properly handles optional values and validation, similar to other mapping-related code in the codebase
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other usages of the mapping configuration to verify consistency
rg -A 2 '"mapping".*:.*{.*"name"' --type scala
Length of output: 169
Script:
#!/bin/bash
# Search for mapping configuration patterns in Scala files
rg -A 5 'mapping.*name' --type scala
# Search for layer configuration patterns
rg -A 5 '"layers".*mapping' --type scala
# Search for view configuration usage patterns
ast-grep --pattern 'viewConfiguration.get("layers")'
Length of output: 9037
URL of deployed dev instance (used for testing):
Steps to test:
Issues:
(Please delete unneeded items, merge only when none are left open)
Summary by CodeRabbit
New Features
Bug Fixes
Improvements