Support rendering more dtypes #8325
Conversation
📝 WalkthroughWalkthroughThis pull request introduces extensive modifications across both frontend and backend codebases. Highlights include a new linter rule to flag unused imports, updates to TypedArray types and method signatures, and refactoring of shader modules and texture handling logic. Several unused imports and redundant parameters have been removed throughout the code, and test suites have been revised to reflect these changes. Additionally, backend Scala services now better handle signed versus unsigned data. Overall, the changes enhance type safety, streamline functionality, and improve code clarity. Changes
Suggested labels
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 (
|
…ds are handled as if they would be positive)
| @@ -593,12 +611,11 @@ export class DataBucket { | |||
|
|
|||
| switch (this.state) { | |||
| case BucketStateEnum.REQUESTED: { | |||
| // Clone the data for the unmergedBucketDataLoaded event, | |||
| // as the following merge operation is done in-place. | |||
| const dataClone = new TypedArrayClass(data); | |||
There was a problem hiding this comment.
this is unrelated to the actual PR. the cloning of data was always done even though it's only necessary before doing a merge.
tools/test.sh
Outdated
| @@ -109,7 +109,8 @@ then | |||
| elif [ $cmd == "test-screenshot" ] | |||
| then | |||
| ensureUpToDateTests | |||
| export NODE_PATH="$testBundlePath" && ava $(find "$testBundlePath" -name "*.screenshot.js") "$@" | |||
| # todop: revert | |||
| const PAGE_WIDTH = 1920; | ||
| const PAGE_HEIGHT = 1080; | ||
|
|
||
| // todop: change to false before merging |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🔭 Outside diff range comments (1)
frontend/javascripts/test/puppeteer/dataset_rendering_helpers.ts (1)
445-454: Fix incorrect usage ofpage.evaluatewithbrowserstack_executor.The current implementation of
getBrowserstackSessionIdincorrectly usespage.evaluate. The function should pass the BrowserStack command directly as a string topage.evaluatewithout the no-op function.Apply this diff to correct the usage:
- const response = (await page.evaluate( - (_) => {}, - `browserstack_executor: ${JSON.stringify({ action: "getSessionDetails" })}`, - )) as unknown as string; + const response = (await page.evaluate( + `browserstack_executor: ${JSON.stringify({ action: "getSessionDetails" })}` + )) as string;
♻️ Duplicate comments (2)
frontend/javascripts/test/puppeteer/dataset_rendering_helpers.ts (1)
23-24: 🛠️ Refactor suggestionFix the typo in the comment.
There's a typo in the comment at line 23. It should be corrected for clarity.
Apply this diff to fix the typo:
-// todop: change to false before merging +// TODO: change to false before mergingtools/test.sh (1)
112-113:⚠️ Potential issueRevert temporary test scope limitation.
This change was previously requested to be reverted. The narrowed test scope could miss potential regressions in other screenshot tests.
Apply this diff to restore the original behavior:
- # todop: revert - export NODE_PATH="$testBundlePath" && ava $(find "$testBundlePath" -name "dtype_datasets_rendering.screenshot.js") "$@" + export NODE_PATH="$testBundlePath" && ava $(find "$testBundlePath" -name "*.screenshot.js") "$@"🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 113-113: Quote this to prevent word splitting.
(SC2046)
🧹 Nitpick comments (14)
webknossos-datastore/app/com/scalableminds/webknossos/datastore/services/DataConverter.scala (1)
50-58: LGTM! Consider adding documentation.The changes to
toMaybeUnsignedlook good. The method name change and type adjustments better reflect its behavior of handling both signed and unsigned types, which aligns with the PR's goal of supporting more dtypes.Consider adding ScalaDoc to document:
- The method's purpose
- Why some types are converted to unsigned while others remain signed
- The relationship with the supported dtypes (uint32, int8, int16, int32, int64)
+ /** + * Converts numeric arrays to their unsigned counterparts where appropriate. + * Bytes are kept as-is to support int8, while other numeric types are converted + * to their unsigned versions to support uint32, int16, int32, and int64. + * + * @param data Array of numeric values + * @return Array with appropriate unsigned conversions + */ def toMaybeUnsigned(data: Array[_ >: Byte with Short with Int with Long with Float])frontend/javascripts/oxalis/geometries/materials/plane_material_factory_helpers.ts (1)
26-32: Consider enhancing the documentation with specific examples.The implementation looks good and the comment provides a helpful reference. Consider adding examples of when manual internal format setting is needed, particularly for the new data types (uint32, int8, int16, int32, int64) that this PR introduces.
webknossos-datastore/app/com/scalableminds/webknossos/datastore/services/FindDataService.scala (1)
160-163: Consider adding documentation and optimizing the index calculation.While the implementation correctly handles signed bytes, consider these improvements:
- Add a comment explaining the +128 offset for handling signed bytes
- Consider using a constant for the offset value
case byteData: Array[Byte] => { + // Add 128 to handle signed bytes (-128 to 127) in the histogram array indices (0 to 255) + private val SIGNED_BYTE_OFFSET = 128 - byteData.foreach(el => counts(el.toInt + 128) += 1) + byteData.foreach(el => counts(el.toInt + SIGNED_BYTE_OFFSET) += 1) extrema = (-128, 128) }frontend/javascripts/test/puppeteer/dtype_datasets_rendering.screenshot.ts (1)
23-27: Remove debugging flags before merging.The
USE_LOCAL_CHROMEflag is set tofalsewith a comment indicating a pending change. Ensure that all debugging flags and comments are appropriately set or removed before merging.Apply this diff to address the issue:
-// todop: change to false before merging const USE_LOCAL_CHROME = false; -// Only relevant when USE_LOCAL_CHROME. Set to false to actually see the browser open. +// Only relevant when USE_LOCAL_CHROME is true. Set HEADLESS to false to see the browser open. const HEADLESS = true;frontend/javascripts/test/puppeteer/screenshot_helpers.ts (1)
11-28: Consider adding URL validation.While the function handles the https:// prefix, it might be good to validate the URL format to prevent invalid URLs.
export function getUrlForScreenshotTests() { let URL = "https://master.webknossos.xyz/"; if (!process.env.URL) { console.warn( "[Warning] No url specified, assuming dev master. If you want to specify a URL, prepend URL=<url> to the command.", ); } else { URL = process.env.URL; // Prepend https:// if not specified if (!/^https?:\/\//i.test(URL)) { URL = `https://${URL}`; } + // Validate URL format + try { + new URL(URL); + } catch (e) { + throw new Error(`Invalid URL format: ${URL}`); + } } return URL; }frontend/javascripts/oxalis/model/helpers/shader_editor.ts (3)
86-86: Set Ace Editor mode to 'glsl' for accurate syntax highlightingCurrently, the Ace Editor mode is set to
'c_cpp', which may not provide optimal syntax highlighting for GLSL shader code. Changing the mode to'glsl'will enhance the editing experience with appropriate syntax highlighting.Apply this diff to set the mode to
'glsl':- mode: "c_cpp", + mode: "glsl",
118-125: Use CSS classes instead of inline styles for 'buttonContainer'Setting styles directly via
setAttributewith inline CSS can make the code harder to maintain and read. Consider assigning a CSS class to thebuttonContainerelement and defining styles in a stylesheet to improve readability and promote reusability.
141-148: Use CSS classes instead of inline styles for 'outerContainer'Applying styles using
setAttributeand inline styles can lead to code that is difficult to maintain. Defining styles in a CSS file and applying them via classes makes the code cleaner and more maintainable.frontend/javascripts/oxalis/shaders/segmentation.glsl.ts (2)
99-207: Optimize 'convertCellIdToRGB' to avoid unnecessary conversionsIn
convertCellIdToRGB,idHigh_uintandidLow_uintare converted back tovec4s usinguintToVec4, only to perform operations on them. Since you're working withuints, consider rewriting the function to operate directly onuints and avoid unnecessary conversions to improve performance.
18-79: Add comments to explain bit manipulation logicFunctions involving bit manipulation, such as
vec4ToUint,vec4ToInt, and the various*ToUint64functions, can be difficult to understand. Adding detailed comments explaining each step can improve readability and help future maintainers understand the code.frontend/javascripts/oxalis/shaders/main_data_shaders.glsl.ts (1)
291-323: Refactor uint32 aggregation to use bitwise operationsIn the uint32 case, current implementation uses floating-point calculations with
pow(256., n), which may introduce precision issues. Refactoring to use bitwise shifts enhances performance and accuracy.Apply this diff to refactor the uint32 value aggregation:
// Combine bytes into a uint32 (assuming little-endian order) - // Scale from [0,1] to [0,255] so that we can convert to an uint - // below. - uvec4 four_bytes = uvec4(255. * maybe_filtered_color.color); - highp uint hpv = - uint(four_bytes.a) * uint(pow(256., 3.)) - + uint(four_bytes.b) * uint(pow(256., 2.)) - + uint(four_bytes.g) * 256u - + uint(four_bytes.r); + uvec4 four_bytes = uvec4(255. * maybe_filtered_color.color); + highp uint hpv = four_bytes.r | (four_bytes.g << 8) | (four_bytes.b << 16) | (four_bytes.a << 24); uint typed_min = <%= name %>_min; uint typed_max = <%= name %>_max; hpv = clamp(hpv, typed_min, typed_max); color_value = vec3( - float(hpv - typed_min) / (float(typed_max - typed_min) + is_max_and_min_equal) + scaleUintToFloat(hpv, typed_min, typed_max) );This change avoids potential floating-point precision errors and aligns the approach with the
int32case.frontend/javascripts/oxalis/model/bucket_data_handling/texture_bucket_manager.ts (1)
51-71: Improve type safety inmaybePadRgbData.The function uses
@ts-ignorecomments with a brief explanation about BigInt. Consider adding runtime type checks or using a more specific type thanTypedArrayto avoid potential issues.Consider this safer implementation:
-function maybePadRgbData(src: TypedArray, elementClass: ElementClass) { +function maybePadRgbData(src: Uint8Array | Uint16Array | Uint32Array, elementClass: ElementClass) { if (elementClass !== "uint24") { return src; } let idx = 0; let srcIdx = 0; while (srcIdx < 3 * constants.BUCKET_SIZE) { - // @ts-ignore BigInt is not a problem as this code here only handles uint24 data tmpPaddingBuffer[idx++] = src[srcIdx++]; - // @ts-ignore BigInt is not a problem as this code here only handles uint24 data tmpPaddingBuffer[idx++] = src[srcIdx++]; - // @ts-ignore BigInt is not a problem as this code here only handles uint24 data tmpPaddingBuffer[idx++] = src[srcIdx++]; tmpPaddingBuffer[idx++] = 255; } return tmpPaddingBuffer; }frontend/javascripts/oxalis/shaders/utils.glsl.ts (1)
383-394: Consider using a more robust approach for large interval handling.The current approach for handling large intervals uses a fixed multiplier (0.25) which might still lead to precision issues for extremely large intervals.
Consider using logarithmic scaling or interval subdivision for more robust handling of large ranges:
- float mul = 0.25; - float nom = mul * x - mul * a; - float denom = mul * b - mul * a; - return nom / denom; + // Use log scale for extremely large intervals + float logScale = log(abs(b - a)); + float scaledX = x / exp(logScale); + float scaledA = a / exp(logScale); + float scaledB = b / exp(logScale); + return (scaledX - scaledA) / (scaledB - scaledA);frontend/javascripts/oxalis/shaders/texture_access.glsl.ts (1)
43-71: LGTM! The texture access logic is correctly updated.The changes correctly handle different texture layer counts and element classes, particularly for
int16types. The normalization logic is well-structured and maintainable.Consider extracting the normalization logic into a separate function for better reusability and maintainability:
+function getNormalizationFactor(textureLayerInfo) { + if (textureLayerInfo.isColor && !elementClass.endsWith("int8")) { + return 1; + } else if ( + textureLayerInfo.isSigned && !elementClass.endsWith("int32") && !elementClass.endsWith("int64") + ) { + return 127; + } else { + return 255; + } +} -float dtype_normalizer = <%= - formatNumberAsGLSLFloat((() => { - if (textureLayerInfo.isColor && !elementClass.endsWith("int8")) { - return 1; - } else if ( - textureLayerInfo.isSigned && !elementClass.endsWith("int32") && !elementClass.endsWith("int64") - ) { - return 127; - } else { - return 255; - } - })()) -%>; +float dtype_normalizer = <%= formatNumberAsGLSLFloat(getNormalizationFactor(textureLayerInfo)) %>;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (39)
frontend/javascripts/test/screenshots/dtype_float32_color_zoomed_in.pngis excluded by!**/*.pngfrontend/javascripts/test/screenshots/dtype_float32_color_zoomed_out.pngis excluded by!**/*.pngfrontend/javascripts/test/screenshots/dtype_int16_color_zoomed_in.pngis excluded by!**/*.pngfrontend/javascripts/test/screenshots/dtype_int16_color_zoomed_out.pngis excluded by!**/*.pngfrontend/javascripts/test/screenshots/dtype_int16_segmentation_zoomed_in.pngis excluded by!**/*.pngfrontend/javascripts/test/screenshots/dtype_int16_segmentation_zoomed_in_selective_segment.pngis excluded by!**/*.pngfrontend/javascripts/test/screenshots/dtype_int16_segmentation_zoomed_out.pngis excluded by!**/*.pngfrontend/javascripts/test/screenshots/dtype_int32_color_zoomed_in.pngis excluded by!**/*.pngfrontend/javascripts/test/screenshots/dtype_int32_color_zoomed_out.pngis excluded by!**/*.pngfrontend/javascripts/test/screenshots/dtype_int32_segmentation_zoomed_in.pngis excluded by!**/*.pngfrontend/javascripts/test/screenshots/dtype_int32_segmentation_zoomed_in_selective_segment.pngis excluded by!**/*.pngfrontend/javascripts/test/screenshots/dtype_int32_segmentation_zoomed_out.pngis excluded by!**/*.pngfrontend/javascripts/test/screenshots/dtype_int64_segmentation_zoomed_in.pngis excluded by!**/*.pngfrontend/javascripts/test/screenshots/dtype_int64_segmentation_zoomed_in_selective_segment.pngis excluded by!**/*.pngfrontend/javascripts/test/screenshots/dtype_int64_segmentation_zoomed_out.pngis excluded by!**/*.pngfrontend/javascripts/test/screenshots/dtype_int8_color_zoomed_in.pngis excluded by!**/*.pngfrontend/javascripts/test/screenshots/dtype_int8_color_zoomed_out.pngis excluded by!**/*.pngfrontend/javascripts/test/screenshots/dtype_int8_segmentation_zoomed_in.pngis excluded by!**/*.pngfrontend/javascripts/test/screenshots/dtype_int8_segmentation_zoomed_in_selective_segment.pngis excluded by!**/*.pngfrontend/javascripts/test/screenshots/dtype_int8_segmentation_zoomed_out.pngis excluded by!**/*.pngfrontend/javascripts/test/screenshots/dtype_uint16_color_zoomed_in.pngis excluded by!**/*.pngfrontend/javascripts/test/screenshots/dtype_uint16_color_zoomed_out.pngis excluded by!**/*.pngfrontend/javascripts/test/screenshots/dtype_uint16_segmentation_zoomed_in.pngis excluded by!**/*.pngfrontend/javascripts/test/screenshots/dtype_uint16_segmentation_zoomed_in_selective_segment.pngis excluded by!**/*.pngfrontend/javascripts/test/screenshots/dtype_uint16_segmentation_zoomed_out.pngis excluded by!**/*.pngfrontend/javascripts/test/screenshots/dtype_uint32_color_zoomed_in.pngis excluded by!**/*.pngfrontend/javascripts/test/screenshots/dtype_uint32_color_zoomed_out.pngis excluded by!**/*.pngfrontend/javascripts/test/screenshots/dtype_uint32_segmentation_zoomed_in.pngis excluded by!**/*.pngfrontend/javascripts/test/screenshots/dtype_uint32_segmentation_zoomed_in_selective_segment.pngis excluded by!**/*.pngfrontend/javascripts/test/screenshots/dtype_uint32_segmentation_zoomed_out.pngis excluded by!**/*.pngfrontend/javascripts/test/screenshots/dtype_uint64_segmentation_zoomed_in.pngis excluded by!**/*.pngfrontend/javascripts/test/screenshots/dtype_uint64_segmentation_zoomed_in_selective_segment.pngis excluded by!**/*.pngfrontend/javascripts/test/screenshots/dtype_uint64_segmentation_zoomed_out.pngis excluded by!**/*.pngfrontend/javascripts/test/screenshots/dtype_uint8_color_zoomed_in.pngis excluded by!**/*.pngfrontend/javascripts/test/screenshots/dtype_uint8_color_zoomed_out.pngis excluded by!**/*.pngfrontend/javascripts/test/screenshots/dtype_uint8_segmentation_zoomed_in.pngis excluded by!**/*.pngfrontend/javascripts/test/screenshots/dtype_uint8_segmentation_zoomed_in_selective_segment.pngis excluded by!**/*.pngfrontend/javascripts/test/screenshots/dtype_uint8_segmentation_zoomed_out.pngis excluded by!**/*.pngyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (45)
biome.json(1 hunks)frontend/javascripts/libs/UpdatableTexture.ts(2 hunks)frontend/javascripts/libs/cuckoo/abstract_cuckoo_table.ts(1 hunks)frontend/javascripts/libs/utils.ts(3 hunks)frontend/javascripts/messages.tsx(0 hunks)frontend/javascripts/oxalis/api/cross_origin_api.ts(0 hunks)frontend/javascripts/oxalis/constants.ts(1 hunks)frontend/javascripts/oxalis/geometries/materials/plane_material_factory.ts(7 hunks)frontend/javascripts/oxalis/geometries/materials/plane_material_factory_helpers.ts(2 hunks)frontend/javascripts/oxalis/model.ts(0 hunks)frontend/javascripts/oxalis/model/accessors/dataset_accessor.ts(4 hunks)frontend/javascripts/oxalis/model/bucket_data_handling/bucket.ts(4 hunks)frontend/javascripts/oxalis/model/bucket_data_handling/data_rendering_logic.tsx(5 hunks)frontend/javascripts/oxalis/model/bucket_data_handling/layer_rendering_manager.ts(1 hunks)frontend/javascripts/oxalis/model/bucket_data_handling/texture_bucket_manager.ts(6 hunks)frontend/javascripts/oxalis/model/helpers/shader_editor.ts(1 hunks)frontend/javascripts/oxalis/model/reducers/skeletontracing_reducer.ts(0 hunks)frontend/javascripts/oxalis/model/reducers/ui_reducer.ts(1 hunks)frontend/javascripts/oxalis/model/sagas/proofread_saga.ts(1 hunks)frontend/javascripts/oxalis/model/sagas/quick_select_heuristic_saga.ts(1 hunks)frontend/javascripts/oxalis/model/sagas/saga_helpers.ts(1 hunks)frontend/javascripts/oxalis/model/sagas/volume/floodfill_saga.tsx(1 hunks)frontend/javascripts/oxalis/model_initialization.ts(0 hunks)frontend/javascripts/oxalis/shaders/main_data_shaders.glsl.ts(13 hunks)frontend/javascripts/oxalis/shaders/segmentation.glsl.ts(7 hunks)frontend/javascripts/oxalis/shaders/texture_access.glsl.ts(3 hunks)frontend/javascripts/oxalis/shaders/utils.glsl.ts(3 hunks)frontend/javascripts/oxalis/view/action-bar/toolbar_view.tsx(0 hunks)frontend/javascripts/oxalis/view/context_menu.tsx(1 hunks)frontend/javascripts/oxalis/view/left-border-tabs/histogram_view.tsx(4 hunks)frontend/javascripts/oxalis/view/left-border-tabs/layer_settings_tab.tsx(1 hunks)frontend/javascripts/test/model/binary/data_rendering_logic.spec.ts(0 hunks)frontend/javascripts/test/model/texture_bucket_manager.spec.ts(2 hunks)frontend/javascripts/test/puppeteer/dataset_rendering.screenshot.ts(8 hunks)frontend/javascripts/test/puppeteer/dataset_rendering.wkorg_screenshot.ts(2 hunks)frontend/javascripts/test/puppeteer/dataset_rendering_helpers.ts(13 hunks)frontend/javascripts/test/puppeteer/dtype_datasets_rendering.screenshot.ts(1 hunks)frontend/javascripts/test/puppeteer/screenshot_helpers.ts(4 hunks)frontend/javascripts/test/shaders/shader_syntax.spec.ts(6 hunks)frontend/javascripts/types/globals.d.ts(1 hunks)package.json(1 hunks)tools/test.sh(1 hunks)webknossos-datastore/app/com/scalableminds/webknossos/datastore/services/AdHocMeshService.scala(1 hunks)webknossos-datastore/app/com/scalableminds/webknossos/datastore/services/DataConverter.scala(1 hunks)webknossos-datastore/app/com/scalableminds/webknossos/datastore/services/FindDataService.scala(3 hunks)
💤 Files with no reviewable changes (7)
- frontend/javascripts/oxalis/model.ts
- frontend/javascripts/oxalis/api/cross_origin_api.ts
- frontend/javascripts/oxalis/view/action-bar/toolbar_view.tsx
- frontend/javascripts/messages.tsx
- frontend/javascripts/oxalis/model/reducers/skeletontracing_reducer.ts
- frontend/javascripts/oxalis/model_initialization.ts
- frontend/javascripts/test/model/binary/data_rendering_logic.spec.ts
✅ Files skipped from review due to trivial changes (2)
- frontend/javascripts/oxalis/model/sagas/saga_helpers.ts
- frontend/javascripts/oxalis/model/reducers/ui_reducer.ts
🧰 Additional context used
📓 Learnings (1)
frontend/javascripts/oxalis/geometries/materials/plane_material_factory.ts (1)
Learnt from: dieknolle3333
PR: scalableminds/webknossos#8168
File: frontend/javascripts/oxalis/geometries/materials/plane_material_factory.ts:568-585
Timestamp: 2024-11-22T17:18:43.411Z
Learning: In the file `frontend/javascripts/oxalis/geometries/materials/plane_material_factory.ts`, the uniform name `allResolutions` should remain unchanged to maintain consistency with the shader code.
🪛 Shellcheck (0.10.0)
tools/test.sh
[warning] 113-113: Quote this to prevent word splitting.
(SC2046)
🔇 Additional comments (50)
frontend/javascripts/oxalis/geometries/materials/plane_material_factory_helpers.ts (2)
5-12: LGTM! Good improvement in the function signature.The change from deriving format based on channel count to explicit format specification is a good practice. It makes the texture format handling more flexible and reduces potential runtime errors.
5-35: Verify that all callers are updated with the new parameters.Let's ensure all callers of
createUpdatableTextureare updated to provide the requiredformatparameter.✅ Verification successful
🏁 Script executed:
#!/bin/bash # Search for all calls to createUpdatableTexture ast-grep --pattern 'createUpdatableTexture($$$)'Length of output: 2605
Callers of createUpdatableTexture are updated correctly.
- In
frontend/javascripts/libs/cuckoo/abstract_cuckoo_table.ts, one call provides all six parameters (including the optional internal format) while another intentionally relies on the optional parameter.- In
frontend/javascripts/oxalis/model/bucket_data_handling/texture_bucket_manager.ts, the full parameter set is provided.All usages align with the updated signature.
webknossos-datastore/app/com/scalableminds/webknossos/datastore/services/FindDataService.scala (3)
142-142: LGTM: Method signature updated to support int8 data type.The addition of
Byteto the union type aligns with the PR objective to support int8 data type for color and segmentation layers.
166-167: LGTM: Variable name change improves clarity.The rename from
intDatatouintDatabetter reflects the unsigned nature of the data being handled.
193-194: Verify the behavior of toMaybeUnsigned for all supported data types.The change from
toUnsignedtotoMaybeUnsignedsuggests a more flexible conversion approach. Please ensure that:
- The conversion works correctly for both signed and unsigned types
- The behavior is consistent across all supported data types (uint32, int8, int16, int32, int64)
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Search for tests covering toMaybeUnsigned conversion rg -l "toMaybeUnsigned.*test" . # Search for all usages of toMaybeUnsigned ast-grep --pattern 'toMaybeUnsigned($_)'Length of output: 667
Let's search for the definition of
toMaybeUnsignedto review its implementation and verify its support for both signed and unsigned types.
🏁 Script executed:
#!/bin/bash # Locate the definition of toMaybeUnsigned in the repository rg -n "def toMaybeUnsigned" .Length of output: 216
🏁 Script executed:
#!/bin/bash # Extract the implementation of toMaybeUnsigned along with surrounding context from DataConverter.scala sed -n '30,150p' webknossos-datastore/app/com/scalableminds/webknossos/datastore/services/DataConverter.scalaLength of output: 3755
Conversion behavior verified: The implementation of
toMaybeUnsignedcorrectly handles all supported data types. Specifically:
- Byte arrays: Returned as-is, which is appropriate since they represent int8.
- Short arrays: Each element is converted to
UShort, supporting int16 unsigned conversion.- Int arrays: Each element is converted to
UInt, matching the expected treatment for int32.- Long arrays: Each element is converted to
ULong, aligning with int64 unsigned conversion.- Float arrays: Returned as-is because there is no unsigned float type.
The method’s behavior is consistent with the conversion requirements for both signed and unsigned types.
frontend/javascripts/test/puppeteer/dataset_rendering.wkorg_screenshot.ts (1)
19-22: Approved: Variable renaming enhances clarity.Renaming
BASE_PATHtoSCREENSHOTS_BASE_PATHimproves code readability and accurately reflects its purpose.frontend/javascripts/test/puppeteer/screenshot_helpers.ts (3)
6-9: LGTM! Path construction for screenshots base path.The constant is well-defined using path.join for cross-platform compatibility.
55-67: LGTM! Robust error handling in deleteScreenshotIfExists.The function properly handles file deletion with Promise-based fs operations and silently catches errors.
105-109: LGTM! Enhanced screenshot comparison with dimension checks.The added dimension validation prevents misleading comparisons of differently sized screenshots.
frontend/javascripts/test/puppeteer/dataset_rendering.screenshot.ts (3)
4-9: LGTM! Well-organized imports.The imports are properly grouped and named exports are used consistently.
35-35: LGTM! Simplified URL initialization.Using the new helper function improves code maintainability and consistency.
136-139: LGTM! Improved parameter organization.Grouping related parameters into an options object enhances readability and maintainability.
frontend/javascripts/oxalis/shaders/main_data_shaders.glsl.ts (8)
9-9: ImportingElementClassThe addition of
ElementClassimport from"types/api_flow_types"is appropriate for defining type information used later in the code.
34-34: Including utility functions in importsThe inclusion of
glslTypeForElementClassandscaleToFloatenhances the shader utility functions, ensuring correct type handling and value scaling.Also applies to: 38-38
42-42: ExportingParamstypeExporting the
Paramstype promotes reusability, allowing it to be imported and utilized in other modules.
47-57: ExtendingtextureLayerInfosstructureThe expansion of
textureLayerInfoswith properties likeisSigned,glslPrefix,elementClass,unsanitizedName, andisColoris essential for supporting additional data types and enhancing texture layer information.
93-94: Dynamic GLSL uniform types for min and max valuesUsing
glslTypeForElementClassto determine the GLSL type for<%= name %>_minand<%= name %>_maxensures that shaders handle various data types correctly.
106-107: Updating uniform declarations touintChanging uniforms such as
activeCellIdHigh,activeCellIdLow,hoveredSegmentIdLow,hoveredSegmentIdHigh,hoveredUnmappedSegmentIdLow, andhoveredUnmappedSegmentIdHightouintimproves type safety when dealing with segmentation IDs.Also applies to: 144-147
199-199: IncludingscaleToFloatandglslTypeForElementClassin shader compilationAdding
scaleToFloatandglslTypeForElementClassto the compiled shader functions ensures proper scaling and type handling within the shader logic.Also applies to: 411-411
217-255: Handling high and low components of IDsImplementing high and low components for IDs enhances support for 64-bit segmentation IDs in shaders. The use of appropriate conversion functions based on
elementClassensures accurate ID processing.frontend/javascripts/types/globals.d.ts (2)
1-1: ImportingApiTypeThe addition of
import { ApiType } from "oxalis/api/api_loader"allows access to the type definitions of the API, ensuring type safety.
9-9: ExtendingWindow.webknossoswithapiReadyAdding
apiReady: ApiType["apiReady"]to thewebknossosobject enhances the globalWindowinterface with theapiReadyproperty, providing better type definitions and ensuring that the API readiness state is correctly typed.frontend/javascripts/test/model/texture_bucket_manager.spec.ts (3)
63-64: UpdatingTextureBucketManagerconstructor parametersThe removal of the fourth parameter in
new TextureBucketManager(2048, 1, "uint8")and adjusting the parameters accordingly reflects changes in theTextureBucketManagerconstructor. Ensure that the constructor signature matches the updated implementation.
64-64: UpdatingsetupDataTexturesmethod callThe removal of the first parameter in
tbm.setupDataTextures(new CuckooTableVec5(CUCKOO_TEXTURE_WIDTH), LAYER_INDEX);aligns with changes to the method's signature. Confirm thatsetupDataTexturesno longer requires the first parameter and functions as expected.
77-78: Adjusting constructor and method calls forTextureBucketManagerConsistently updating the constructor call to
new TextureBucketManager(2048, 2, "uint8")and correspondingsetupDataTexturesmethod aligns with the refactoredTextureBucketManager. This ensures that the test cases are compatible with the updated class definition.frontend/javascripts/libs/UpdatableTexture.ts (1)
1-1: LGTM! The changes improve type flexibility.The switch to
TypedArraytype enhances flexibility and aligns with the PR's objective to support more data types.Also applies to: 79-79
frontend/javascripts/oxalis/model/bucket_data_handling/layer_rendering_manager.ts (1)
153-165: LGTM! The changes simplify texture setup.The removal of the
bytesparameter and reliance onelementClassfor type information improves code maintainability.frontend/javascripts/oxalis/model/bucket_data_handling/texture_bucket_manager.ts (2)
96-106: LGTM! Constructor simplification improves code clarity.The removal of the
bytesparameter and use ofgetDtypeConfigForElementClassfor determiningpackingDegreeimproves code maintainability.
261-276: LGTM! Improved texture setup with better type handling.The changes to
setupDataTexturesimprove type safety by usinggetDtypeConfigForElementClassto determine texture properties.frontend/javascripts/libs/cuckoo/abstract_cuckoo_table.ts (1)
50-57: LGTM! Good refactoring of texture initialization.The changes improve code maintainability by consolidating the internal format handling into the texture creation call.
frontend/javascripts/oxalis/model/sagas/volume/floodfill_saga.tsx (1)
30-30: LGTM! Clean removal of unused import.The removal of the unused
takeimport improves code cleanliness.frontend/javascripts/oxalis/view/left-border-tabs/histogram_view.tsx (1)
24-24: LGTM! Good enhancement of histogram flexibility.The addition of
supportFractionalsandmaybeCeilFnprovides a clean way to handle both fractional and integer-only cases.Also applies to: 278-278, 290-290
frontend/javascripts/oxalis/shaders/utils.glsl.ts (1)
358-396: LGTM! Well-implemented scaling functions with good edge case handling.The implementation includes:
- Safe type conversions using uint
- Edge case handling for zero range
- Clear comments explaining the logic
frontend/javascripts/oxalis/constants.ts (1)
352-365: LGTM! The type definitions are correctly updated to support int64.The changes correctly add
BigInt64Arrayto theTypedArraytype and updateTypedArrayWithoutBigIntto exclude bothBigUint64ArrayandBigInt64Array. This aligns with the PR objectives to support more data types.Let's verify that these changes are consistent with the TypeScript documentation:
✅ Verification successful
🌐 Web query:
What is the difference between BigInt64Array and BigUint64Array in TypeScript?💡 Result:
The `BigInt64Array` and `BigUint64Array` in TypeScript/JavaScript both handle 64-bit integer data but differ in signedness and value ranges: ## Key Differences | Feature | **BigInt64Array** | **BigUint64Array** | |-----------------------|--------------------------------------------|--------------------------------------------| | **Signedness** | Signed integers (positive/negative values)| Unsigned integers (non-negative values) | | **Value Range** | -9,223,372,036,854,775,808 to<br>9,223,372,036,854,775,807 | 0 to<br>18,446,744,073,709,551,615 | | **Use Case** | Applications requiring negative numbers | Scenarios needing large non-negative values| Both inherit from `TypedArray` and share these properties: - Store elements as `bigint` values (e.g., `42n`) - Fixed size: `BYTES_PER_ELEMENT = 8` - Initialized with `0n` by default - Support identical construction methods: ```typescript // Common initialization patterns new BigInt64Array(2); // Signed new BigUint64Array([1n, 2n]); // Unsigned ``` ## Example Behavior ```typescript // Signed array allows negative values const signedArr = new BigInt64Array([-100n]); console.log(signedArr[0]); // -100n [1][9] // Unsigned array rejects negatives const unsignedArr = new BigUint64Array(1); unsignedArr[0] = -100n; // Throws RangeError [5][7] ``` ## When to Use - **BigInt64Array**: Financial calculations requiring debt/credit distinctions or systems using signed 64-bit IDs (e.g., some databases)[9]. - **BigUint64Array**: Cryptographic hashes, IPv6 addresses, or hardware registers using unsigned values[4][10]. These arrays enable precise handling of large integers while maintaining platform byte order compatibility[1][5]. For byte-order control or mixed-type operations, use `DataView` instead[1][5]. Citations: - 1: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/BigInt64Array - 2: https://udn.realityripple.com/docs/Web/JavaScript/Reference/Global_Objects/BigInt64Array - 3: https://udn.realityripple.com/docs/Web/JavaScript/Reference/Global_Objects/BigUint64Array/BigUint64Array - 4: https://dev.to/dhrn/day26-typed-arrays-5522 - 5: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/BigUint64Array - 6: https://www.npmjs.com/package/bigint-conversion - 7: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/BigUint64Array/BigUint64Array - 8: https://www.typescriptlang.org/docs/handbook/basic-types.html - 9: https://v8.dev/features/bigint - 10: https://discourse.threejs.org/t/bufferattribute-is-it-possible-to-create-a-bufferattribute-from-a-biguint64array/25093
Verification of Updated TypedArray Definitions
The changes correctly include both
BigUint64ArrayandBigInt64Arrayin theTypedArrayunion and updateTypedArrayWithoutBigIntto exclude them. The web query confirms that the key difference is in their signedness, which aligns with the PR's objective to support 64-bit arrays.
- The updated types are consistent with the TypeScript documentation regarding the handling of large integers.
frontend/javascripts/oxalis/model/accessors/dataset_accessor.ts (2)
197-203: LGTM! The value range determination is correctly delegated.The function now correctly delegates the value range determination to
getSupportedValueRangeForElementClass, which improves maintainability by centralizing this logic.
378-396: LGTM! The element class support is correctly updated.The changes correctly limit uint64 and int64 support to segmentation layers only, with a clear comment about the JavaScript Number type limitation.
Let's verify the JavaScript Number type limitation mentioned in the comment:
✅ Verification successful
🌐 Web query:
What is the maximum safe integer in JavaScript?💡 Result:
The maximum safe integer in JavaScript is **9,007,199,254,740,991**, represented by the `Number.MAX_SAFE_INTEGER` constant[1][2][5]. This value equals \(2^{53} - 1\), derived from JavaScript’s use of 64-bit double-precision floating-point numbers (IEEE 754 standard)[2][5][6]. ### Key Details - **Definition**: A "safe integer" is one that can be: - Exactly represented in IEEE 754 format - Correctly compared with other integers (e.g., `x + 1 ≠ x`)[5][6] - **Limitations**: - Beyond this range (`±9,007,199,254,740,991`), arithmetic operations lose precision: ```javascript console.log(Number.MAX_SAFE_INTEGER + 1 === Number.MAX_SAFE_INTEGER + 2); // true[3][6] ``` - Bitwise/shift operators use 32-bit integers (max safe: \(2^{31} - 1\))[2]. - **Verification**: Use `Number.isSafeInteger()` to check validity: ```javascript console.log(Number.isSafeInteger(9007199254740991)); // true console.log(Number.isSafeInteger(9007199254740992)); // false[4][6] ``` ### Practical Considerations - For integers exceeding this range: - Use `BigInt` type (e.g., `12345678901234567890n`)[3][5]. - Related constants: - `Number.MIN_SAFE_INTEGER`: \(-9,007,199,254,740,991\)[2][4] - `Number.MAX_VALUE`: Largest representable number (~\(1.79 \times 10^{308}\))[2][4]. This limit ensures reliable integer operations in most applications while guiding developers to specialized types like `BigInt` for extreme values[3][5]. Citations: - 1: https://www.w3schools.com/jsref/jsref_max_safe_integer.asp - 2: https://www.tutorialspoint.com/what-is-javascript-s-highest-integer-value-that-a-number-can-go-to-without-losing-precision - 3: https://www.programiz.com/javascript/library/number/max_safe_integer - 4: https://community.latenode.com/t/what-is-the-maximum-integer-javascript-can-accurately-represent/611 - 5: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number/MAX_SAFE_INTEGER?retiredLocale=vi - 6: https://forum.freecodecamp.org/t/explanation-of-safe-integers/322832
Element class support and JS Number type limitation verified.
The maximum safe integer in JavaScript is indeed 2⁵³ – 1 (9007199254740991), which confirms that the adjustment in the code—restricting uint64 and int64 support to segmentation layers only (due to potential precision issues with numbers beyond this threshold)—is correctly implemented.frontend/javascripts/oxalis/model/bucket_data_handling/bucket.ts (3)
28-39: LGTM! The type definitions are correctly updated.The
BucketDataArraytype is correctly updated to includeInt8Array,Int16Array,Int32Array, andBigInt64Array, with a clear comment about adapting the type for new element classes.
79-88: LGTM! The constructor types are correctly updated.The
TypedArrayConstructortype is correctly updated to include the corresponding constructors for the new array types.
614-619: LGTM! The data cloning is correctly handled.The changes correctly clone the data before triggering the
unmergedBucketDataLoadedevent, as the subsequent merge operation is done in-place.Let's verify that the cloning is necessary by checking for any references to this event:
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Description: Search for references to unmergedBucketDataLoaded event # to verify that the cloning is necessary. rg -A 5 "unmergedBucketDataLoaded"Length of output: 1630
Verification Successful: Data cloning is necessary and correctly implemented.
The search output confirms that theunmergedBucketDataLoadedevent is used in a one-time listener to handle the unmerged bucket data, and that the event is triggered with a clone (dataClone). This ensures that the subsequent in-place merge operation ondatadoes not affect the event handler’s data.frontend/javascripts/oxalis/model/sagas/quick_select_heuristic_saga.ts (1)
224-226: LGTM: Extended error handling for signed 64-bit arrays.The addition of
BigInt64Arraycheck aligns with the PR objective of supporting more dtypes while maintaining the restriction that color input layers must not be 64-bit.frontend/javascripts/libs/utils.ts (2)
884-901: LGTM: Improved sign handling in base-256 conversion.The changes correctly handle signed numbers by:
- Capturing the sign before conversion
- Using Math.abs() for consistent handling
- Applying the sign to each element of the result array
921-934: LGTM: Enhanced sign handling for both number and bigint types.The changes correctly handle sign for both types:
- Detects sign for both number and bigint inputs
- Applies sign consistently to both high and low parts
frontend/javascripts/oxalis/geometries/materials/plane_material_factory.ts (3)
232-249: LGTM: Simplified uniform value handling.Changed from Vector4 to simple number values, which is cleaner and more efficient.
955-958: LGTM: Improved active cell ID handling.Correctly handles negative IDs and splits them into high and low parts.
970-971: LGTM: Simplified intensity range assignment.Direct assignment of intensity range values makes the code cleaner and more maintainable.
frontend/javascripts/oxalis/model/sagas/proofread_saga.ts (1)
69-69: LGTM!The removal of the unused
takeimport is a good cleanup, as the code only uses other effects liketakeEvery,takeEveryUnlessBusy, andtakeWithBatchActionSupport.frontend/javascripts/oxalis/view/left-border-tabs/layer_settings_tab.tsx (1)
590-591: LGTM!The changes enhance the histogram's functionality by adding support for fractional data types. The
elementClassis correctly retrieved and used to determine if fractional support is needed based on the data type being "float" or "double".Also applies to: 594-594
frontend/javascripts/oxalis/view/context_menu.tsx (1)
453-453: Improved segment detection to support negative IDs.The change from
> 0to!== 0allows the context menu to handle negative segment IDs, which is essential for supporting the new signed integer data types (int8, int16, int32, int64). This aligns with the PR's objective to enhance dtype support.biome.json (1)
67-67: LGTM: Added linter rule to enforce clean imports.The new
noUnusedImportsrule will help maintain code quality by ensuring all imports are actually used. This is particularly beneficial as new data types are being added to the codebase.package.json (1)
63-63: LGTM: Added semaphore-promise for concurrent resource management.The addition of semaphore-promise will help manage concurrent access to resources, which could be beneficial for handling the new data types.
✅ Verification successful
🌐 Web query:
What is the latest version of semaphore-promise and are there any breaking changes or known issues?💡 Result:
The latest version of `semaphore-promise` is **1.4.2**, released on July 9, 2023[1]. **Key details about recent updates:** - No breaking changes are mentioned in recent changelogs[1][10] - Recent updates focus on TypeScript improvements and debug logging capabilities[1][10] - Added default export support (since v1.3.0) without breaking existing imports[1] - Default constructor now uses 1 semaphore instead of requiring explicit count[1] **Known considerations:** - Requires proper `release()` calls to avoid deadlocks[2][9] - Timeout handling in promise-based workflows might require additional error boundaries (as seen in QUnit integration challenges)[3] - Debugging capabilities were added in v1.4.0 to help identify semaphore imbalances[1][10] The library maintains backward compatibility while adding features like TypeScript support and debug logging over its last several updates[1][10]. No critical issues specific to the package itself are documented in the sources provided - most potential pitfalls relate to general semaphore usage patterns rather than implementation flaws[3][9]. Citations: - 1: https://www.npmjs.com/package/semaphore-promise - 2: https://www.npmjs.com/package/async-mutex/v/0.2.2 - 3: https://github.com/qunitjs/qunit/issues/1375 - 4: https://github.com/semaphoreui/semaphore/releases - 5: https://www.npmjs.com/package/@chriscdn/promise-semaphore - 6: https://github.com/chriscdn/promise-semaphore - 7: https://jackpordi.com/posts/locks-in-js-because-why-not - 8: https://techdocs.f5.com/kb/en-us/products/big-ip_ltm/releasenotes/related/relnote-supplement-bigip-17-1-1-4.html - 9: https://github.com/samccone/promise-semaphore - 10: https://github.com/Granjow/semaphore-promise
Semaphore-promise version 1.4.2 verified and approved.
The latest version on npm is indeed 1.4.2, and recent changelogs confirm no breaking changes. The addition looks good as long as the team remains mindful to correctly handle semaphore releases to avoid potential deadlocks.
webknossos-datastore/app/com/scalableminds/webknossos/datastore/services/AdHocMeshService.scala
Outdated
Show resolved
Hide resolved
| dragIcon.addEventListener("mousedown", (e) => { | ||
| isDragging = true; | ||
| offsetX = e.clientX - outerContainer.offsetLeft; | ||
| offsetY = e.clientY - outerContainer.offsetTop; | ||
| dragIcon.style.cursor = "grabbing"; | ||
| }); | ||
|
|
||
| window.addEventListener("mousemove", (e) => { | ||
| if (!isDragging) return; | ||
|
|
||
| outerContainer.style.left = `${e.clientX - offsetX}px`; | ||
| outerContainer.style.top = `${e.clientY - offsetY}px`; | ||
| }); | ||
|
|
||
| window.addEventListener("mouseup", () => { | ||
| if (isDragging) { | ||
| isDragging = false; | ||
| dragIcon.style.cursor = "grab"; | ||
| } | ||
| }); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Ensure event listeners are properly removed to prevent memory leaks
When adding event listeners to window and other objects, it's important to remove them when they're no longer needed. Consider removing these listeners when the shader editor is closed or destroyed to prevent potential memory leaks.
| window.materials[identifier][shaderType] = input.ace?.getValue() || input.value; | ||
| window.materials[identifier].needsUpdate = true; |
There was a problem hiding this comment.
Add null checks for 'input.ace' to prevent runtime errors
Accessing input.ace without ensuring it is initialized can lead to runtime errors if Ace Editor fails to load or initialize. Adding null checks or defaulting to a safe operation can improve the robustness of the code.
Apply this diff to add null checks:
// Within the overrideShader function
-function overrideShader() {
- window.materials[identifier][shaderType] = input.ace?.getValue() || input.value;
+function overrideShader() {
+ const shaderCode = input.ace ? input.ace.getValue() : input.value;
+ window.materials[identifier][shaderType] = shaderCode;
// Within the loadAce().then(...) block
loadAce().then((transform) => {
transform(input);
+ if (input.ace) {
input.ace.commands.addCommand({
name: "Update Shader",
bindKey: {
win: "Ctrl-Enter",
mac: "Command-Enter",
},
exec: overrideShader,
});
+ }
});Ensure that input.ace is properly initialized before using it in the code.
Also applies to: 193-203
frontend/javascripts/oxalis/model/bucket_data_handling/data_rendering_logic.tsx
Show resolved
Hide resolved
frontend/javascripts/oxalis/model/bucket_data_handling/data_rendering_logic.tsx
Show resolved
Hide resolved
frontend/javascripts/oxalis/model/bucket_data_handling/data_rendering_logic.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
webknossos-datastore/app/com/scalableminds/webknossos/datastore/services/DataConverter.scala (1)
50-60: LGTM! Consider adding documentation for the new parameter.The changes to
toMaybeUnsignedimprove type handling by supporting both signed and unsigned conversions. The implementation is correct and type-safe.Consider adding ScalaDoc to document the
isSignedparameter's purpose and impact:+ /** + * Converts numeric arrays to unsigned types if needed. + * @param data The input array to convert + * @param isSigned If true, preserves the original signed values; if false, converts to unsigned + * @return The converted array + */ def toMaybeUnsigned(webknossos-datastore/app/com/scalableminds/webknossos/datastore/services/FindDataService.scala (1)
168-182: Simplify histogram index adjustments for signed types.The current implementation uses different approaches for adjusting histogram indices across signed types. Consider unifying the approach.
Apply this diff to unify the index adjustments:
case byteData: Array[Byte] => { - byteData.foreach(el => counts(el.toInt + 128) += 1) + byteData.foreach(el => counts((el + 128).toInt) += 1) extrema = (-128, 128) } case shortData: Array[Short] => { - shortData.foreach(el => counts((el / 256.toShort + 128).toInt) += 1) + shortData.foreach(el => counts((el / 256 + 128).toInt) += 1) } case intData: Array[Int] => - intData.foreach(el => counts((el / 16777216 + 128).toInt) += 1) + intData.foreach(el => counts((el / 16777216 + 128).toInt) += 1)webknossos-datastore/app/com/scalableminds/webknossos/datastore/models/datasource/DataLayer.scala (1)
65-78: LGTM! Consider adding documentation for the new method.The implementation correctly determines signedness for all element classes using pattern matching.
Consider adding ScalaDoc to document the method's purpose and return value:
+ /** + * Determines whether an element class represents signed values. + * @param elementClass The element class to check + * @return true if the element class is signed (float, double, or signed integers), + * false if unsigned + */ def isSigned(elementClass: ElementClass.Value): Boolean =
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
webknossos-datastore/app/com/scalableminds/webknossos/datastore/models/datasource/DataLayer.scala(1 hunks)webknossos-datastore/app/com/scalableminds/webknossos/datastore/services/DataConverter.scala(1 hunks)webknossos-datastore/app/com/scalableminds/webknossos/datastore/services/FindDataService.scala(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: circleci_build
🔇 Additional comments (1)
webknossos-datastore/app/com/scalableminds/webknossos/datastore/services/FindDataService.scala (1)
148-154:⚠️ Potential issueFix off-by-one error in extrema calculation for signed types.
The current extrema calculation for signed types has an off-by-one error. For n-bit signed integers, the range should be [-2^(n-1), 2^(n-1)-1].
Apply this diff to fix the extrema calculation:
var extrema: (Double, Double) = ( if (ElementClass.isSigned(elementClass)) - (-math.pow(2, 8 * bytesPerElement - 1), math.pow(2, 8 * bytesPerElement - 1) - 1) + (-math.pow(2, 8 * bytesPerElement - 1), math.pow(2, 8 * bytesPerElement - 1) - 1) else - (0, math.pow(2, 8 * bytesPerElement) - 1) + (0, math.pow(2, 8 * bytesPerElement) - 1) )Likely invalid or redundant comment.
This PR adds rendering support for uint32 color layers, int8, int16, int32 for color + segmentation, and int64 for segmentation layers.
Histogram and thumbnail support were only done partially. The rest can happen in another PR.
URL of deployed dev instance (used for testing):
Steps to test:
TODOs:
Issues:
(Please delete unneeded items, merge only when none are left open)