-
Notifications
You must be signed in to change notification settings - Fork 29
Calculate surface area of segments #8928
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from 33 commits
Commits
Show all changes
40 commits
Select commit
Hold shift + click to select a range
6ac57c6
WIP calculate surface area of segments
fm3 61c71a2
fix triangle offset
fm3 710d52e
Merge branch 'master' into mesh-surface-area
fm3 40a8ce4
fix vertex index offset
fm3 00b458a
Merge branch 'master' into mesh-surface-area
fm3 dbd3335
move to segmentStatistics routes
fm3 18e2409
add meshFileName, lod and seedPosition to getSegmentSurfaceArea reque…
fm3 7d56a1a
integrate first version for surface area into front end
philippotto 6dfe14c
also add to segment statistics modal
philippotto cc95bbe
Fix RPC fetching voxel size for annotation
fm3 d55e5a2
Merge branch 'master' of github.com:scalableminds/webknossos into mes…
philippotto bad0855
only show segment stats when explicitly triggered
philippotto 2ea2456
integrate surface area into stats modal and csv export
philippotto ae62338
remove seed pos
philippotto ee138b3
Merge branch 'master' into mesh-surface-area
fm3 9940941
fix voxelSize rpc; remove lod param; check meshfile mapping matches
fm3 3172698
add token to request, remove unused imports
fm3 9855846
keep menu open when load stats is clicked
philippotto 0e6f1ea
remove lod
philippotto 9af5488
fix type error
philippotto 82400f7
sort imports
philippotto b65aa32
Merge branch 'master' into mesh-surface-area
fm3 7d22add
Merge branch 'master' into mesh-surface-area
fm3 c61dbca
changelog
fm3 e9ec82e
migrate new function getSegmentSurfaceArea to use new LayerSourceInfo…
MichaelBuessemeyer 90bcfef
remove now-unused route implementation
fm3 fa7ef22
remove empty line
MichaelBuessemeyer 00d68aa
deduplicate react key
MichaelBuessemeyer d1cb2bc
Merge branch 'mesh-surface-area' of github.com:scalableminds/webknoss…
MichaelBuessemeyer a02d99b
improve dataset.notFound messages
fm3 299c8e3
fix oversize-segment stats table
MichaelBuessemeyer 10cfcdd
Merge branch 'mesh-surface-area' of github.com:scalableminds/webknoss…
fm3 f39564b
Merge branch 'master' into mesh-surface-area
fm3 7b44303
fix after merge
fm3 1cb016a
add seed position parameter for segment surface area request
MichaelBuessemeyer 2c684cd
fix typo & reuse calculated mapping name
MichaelBuessemeyer 89ed12c
undo adding segment seed position to surface calculation route
MichaelBuessemeyer b566e1c
remove comment about seed position for FE surface area route as it is…
MichaelBuessemeyer a8d15ef
remove seed position from request body type
fm3 c6aa9f2
Merge branch 'mesh-surface-area' of github.com:scalableminds/webknoss…
fm3 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -879,6 +879,41 @@ export function getSegmentVolumes( | |
| ); | ||
| } | ||
|
|
||
| type SegmentStatisticsParametersMeshBased = { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd rename this to |
||
| mag: Vector3; | ||
| segmentIds: number[]; | ||
| mappingName?: string | null; | ||
| additionalCoordinates?: AdditionalCoordinate[] | null; | ||
| meshFileName?: string | null; | ||
| }; | ||
|
|
||
| export function getSegmentSurfaceArea( | ||
| layerSourceInfo: LayerSourceInfo, | ||
| mag: Vector3, | ||
| meshFileName: string | undefined | null, | ||
| segmentIds: Array<number>, | ||
| additionalCoordinates: AdditionalCoordinate[] | undefined | null, | ||
| mappingName: string | null | undefined, | ||
| ): Promise<number[]> { | ||
| const requestUrl = getDataOrTracingStoreUrl(layerSourceInfo); | ||
| return doWithToken((token) => { | ||
| const data: SegmentStatisticsParametersMeshBased = { | ||
| mag, | ||
| segmentIds, | ||
| mappingName, | ||
| additionalCoordinates, | ||
| meshFileName, | ||
| }; | ||
| return Request.sendJSONReceiveJSON( | ||
| `${requestUrl}/segmentStatistics/surfaceArea?token=${token}`, | ||
| { | ||
| data, | ||
| method: "POST", | ||
| }, | ||
| ); | ||
| }); | ||
| } | ||
|
|
||
| export function getSegmentBoundingBoxes( | ||
| layerSourceInfo: LayerSourceInfo, | ||
| mag: Vector3, | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This route is only used to retrieve the voxelsize If am not incorrect. Maybe we could instead implement a
getVoxelSizeor so instead of sending all the whole datasource to the tracingstore?Moreover, maybe the access checking via a DB context and a passed accesstoken in the request could be better. Just as coderabbit is indicating.
Although, keeping the global access context and only returning the voxel size should be fine as this does not leak any crucial imo.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it’s fine. The tracingstore needs to be authenticated anyway, and it doesn’t give any of this info to the user.