-
Notifications
You must be signed in to change notification settings - Fork 44
Add clusteringPixelSizeThreshold + memory monitoring #363
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
Conversation
|
Here's the code health analysis summary for commits Analysis Summary
|
|
@fedorov I have this solution for items 1 and 2 of #330 (comment). Please let me know if its better now. |
|
Now the image doesn't load at all. I deployed using the
|
|
Then we broke the production with another regression... It was working as of Nov 21, 2025, as documented in the screenshot in #330 (comment). |
If you try this curl it returns: * Request completely sent off
< HTTP/2 200
< content-type: multipart/related; boundary=19b0c81d44214ba1e34ef05ad1c6cec5a344b04b1e085b59b4499caddbf0; transfer-syntax=1.2.840.10008.1.2.4.80; type="image/jls"
< vary: Origin
< vary: X-Origin
< vary: Referer
< x-xss-protection: 0
< x-frame-options: SAMEORIGIN
< x-content-type-options: nosniff
< alt-svc: h3=":443"; ma=2592000,h3-29=":443"; ma=2592000
< alt-svc: h3=":443"; ma=2592000,h3-29=":443"; ma=2592000
< strict-transport-security: max-age=31536001; includeSubDomains; preload
< x-cloud-trace-context: 0fe719e72d005ec594972f978f92aaa3;o=1
< date: Mon, 12 Jan 2026 17:14:29 GMT
< server: Google Frontend
< content-length: 378
< via: 1.1 google
<
--19b0c81d44214ba1e34ef05ad1c6cec5a344b04b1e085b59b4499caddbf0
Content-Type: image/jls; transfer-syntax=1.2.840.10008.1.2.4.80
[{
"error": {
"code": 400,
"message": "transcoding frame: generic::invalid_argument: Component 1 out of 3 components does not have the expected width. Component Width: 128, Expected Columns: 256",
"status": "INVALID_ARGUMENT"
}
}
* Connection #0 to host proxy.imaging.datacommons.cancer.gov left intactNo problems using dcm4chee... Let me know if you get the same results! |
|
@igoroctaviano I deployed with the
|
|
Binary SEGs don't load either - neither in prod, nor in GHA deploy... |
|
Igor, the update of accept headers seems to work great! But I don't see the memory monitoring footer you mentioned. |
|
As clarified by @igoroctaviano, this requires new config flag: Looks nice and helpful!
@igoroctaviano let's get those merged! |
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.
Pull request overview
Adds configurable annotation clustering controls and introduces an optional, app-wide memory monitoring footer (plus a small enhancement to the DICOM Tag Browser to better align the selected series with the active viewport).
Changes:
- Add clustering enable/disable toggle and
clusteringPixelSizeThresholdplumbing into viewer construction and SlideViewer UI/state. - Add memory monitoring service + footer UI, gated behind
enableMemoryMonitoringapp config. - Improve DICOM Tag Browser series selection behavior (pass series UID from current route; avoid in-place sorting).
Reviewed changes
Copilot reviewed 13 out of 17 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| yarn.lock | Updates lock entry for pinned dicomweb-client version resolution. |
| package.json | Pins dicomweb-client to 0.10.3 (no range). |
| src/types/performance.d.ts | Adds TS declarations for experimental memory APIs. |
| src/services/MemoryMonitor.ts | Introduces singleton memory monitoring with modern + Chrome fallback APIs. |
| src/components/MemoryFooter.tsx | Adds footer UI to display memory usage and emit warnings. |
| src/AppConfig.d.ts | Adds enableMemoryMonitoring configuration flag. |
| src/App.tsx | Conditionally renders MemoryFooter across routes when enabled. |
| src/components/SlideViewer/utils/viewerUtils.ts | Passes annotationOptions.clusteringPixelSizeThreshold into DMV viewer construction. |
| src/components/SlideViewer/types.ts | Extends SlideViewerState with clustering threshold + enabled flag. |
| src/components/SlideViewer.tsx | Adds clustering toggle/threshold UI + handlers; adds shouldComponentUpdate. |
| src/components/Header.tsx | Extracts seriesInstanceUID from route and passes it to Tag Browser. |
| src/components/DicomTagBrowser/DicomTagBrowser.tsx | Accepts seriesInstanceUID, avoids in-place sort, and auto-selects matching series. |
| src/App.light.less | Adds multiline menu item CSS helper class. |
| src/App.dark.less | Adds multiline menu item CSS helper class. |
| docs/MEMORY_MONITORING.md | Adds detailed documentation for memory monitoring feature/configuration. |
| README.md | Documents memory monitoring feature and config flag. |
| .gitignore | Ignores .cursorrules. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@fedorov updated, do you need to run copilot again? |
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.
Pull request overview
Copilot reviewed 13 out of 17 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@igoroctaviano I hope the new comments are helpful, but if they are not - I definitely trust you more than Copilot! Please use your judgement. |
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.
Pull request overview
Copilot reviewed 13 out of 17 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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.
Pull request overview
Copilot reviewed 13 out of 17 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@fedorov addressed the CR issues. |
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.
Pull request overview
Copilot reviewed 13 out of 17 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|







Clustering Configuration and Memory Monitoring
Summary
This PR introduces several enhancements to annotation clustering and processing, plus adds comprehensive memory monitoring capabilities:
Changes
1. Clustering Pixel Size Threshold (Initial Feature)
Added
clusteringPixelSizeThresholdto component statenumber0.001mm (1 micrometer)SlideViewerStateinterfaceAdded threshold input UI
Updated viewer construction
clusteringPixelSizeThresholdto viewer viaannotationOptionsconstructViewersutility functionAdded
handleClusteringPixelSizeThresholdChangehandler2. UI Toggle for Clustering (Latest Feature)
Added clustering toggle switch in the Annotation Groups menu
SwitchcomponentisClusteringEnabled: true)Conditional display of threshold input
State Management
isClusteringEnabledto component state (type:boolean, default:true)shouldComponentUpdateto include clustering state changesViewer Integration
clusteringPixelSizeThresholdis passed asundefinedto the viewerhandleClusteringTogglehandler with error handling3. Memory Monitoring
Created MemoryMonitor service (
src/services/MemoryMonitor.ts)performance.measureUserAgentSpecificMemory()) with cross-origin isolationperformance.memory) when modern API unavailableCreated MemoryFooter component (
src/components/MemoryFooter.tsx)Added type declarations (
src/types/performance.d.ts)Performanceinterface for experimental APIsWindowinterface for cross-origin isolation flagIntegrated into App layout
MemoryFootercomponent to all routes (worklist, study viewer, projects, logout)Documentation
docs/MEMORY_MONITORING.mdwith comprehensive technical documentationREADME.mdwith memory monitoring feature description4. Style Updates
Code Quality
/** */).cursorrulesguidelinesUser Experience
Testing
Threshold can be set and updated successfully
Toggle can be switched on/off successfully
Threshold input appears/disappears based on toggle state
Viewer correctly applies clustering settings when toggled
No unnecessary annotation loading when toggling clustering
Works correctly when toggled before annotations are loaded
Memory monitoring displays correctly in footer
Memory warnings appear at appropriate thresholds
Monitoring works with both modern and fallback APIs
Footer hides gracefully when APIs unavailable
Bulk annotations always cluster #330 (comment)
DMV PR: Add clusteringPixelSizeThreshold option dicom-microscopy-viewer#222
Other issues