From d2ae13b03f21e3ab9545d50d7403ec52004bfbfc Mon Sep 17 00:00:00 2001 From: Alireza Date: Mon, 6 Oct 2025 09:39:56 -0400 Subject: [PATCH] fix: viewport ready event firing logic (#5462) Co-authored-by: Cursor Agent Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- BUG_FIX_SUMMARY.md | 92 ------------------- .../contextProviders/ViewportGridProvider.tsx | 13 ++- 2 files changed, 11 insertions(+), 94 deletions(-) delete mode 100644 BUG_FIX_SUMMARY.md diff --git a/BUG_FIX_SUMMARY.md b/BUG_FIX_SUMMARY.md deleted file mode 100644 index bff646768..000000000 --- a/BUG_FIX_SUMMARY.md +++ /dev/null @@ -1,92 +0,0 @@ -# Bug Fix: Duplicate Segmentation Entries When Switching Layouts - -## Issue Description -**Linear Issue:** OHI-2076 -**Title:** [Bug] Duplicate entries for same segmentation when switching between Common and advanced layout - -When switching from a Common layout to an advanced layout (e.g., 3D Four-Up) in the OHIF Viewer, duplicate entries for the same segmentation appear in the Segmentation Panel. - -## Root Cause -The issue was in the `useViewportSegmentations` hook located at: -``` -extensions/cornerstone/src/hooks/useViewportSegmentations.ts -``` - -When a viewport contains multiple representations of the same segmentation (e.g., both a LABELMAP and SURFACE representation), the hook was creating separate entries for each representation, even though they belong to the same segmentation. This resulted in duplicate entries in the Segmentation Panel. - -The problem occurred because: -1. When switching layouts, new viewports are created -2. The synchronizer may add the same segmentation with different representation types to these viewports -3. The `getSegmentationRepresentations(viewportId)` call returns ALL representations for that viewport -4. Each representation was mapped to a separate entry, creating duplicates - -## Solution -Added deduplication logic in the `useViewportSegmentations` hook to ensure only one entry per unique `segmentationId` is shown in the panel, regardless of how many representation types exist. - -### Code Changes - -**File:** `extensions/cornerstone/src/hooks/useViewportSegmentations.ts` - -**Before:** -```typescript -const representations = segmentationService.getSegmentationRepresentations(viewportId); - -const newSegmentationsWithRepresentations = representations.map(representation => { - const segmentation = segmentationService.getSegmentation(representation.segmentationId); - const mappedSegmentation = mapSegmentationToDisplay(segmentation, customizationService); - return { - representation, - segmentation: mappedSegmentation, - }; -}); -``` - -**After:** -```typescript -const representations = segmentationService.getSegmentationRepresentations(viewportId); - -// Deduplicate representations by segmentationId to prevent showing -// the same segmentation multiple times in the panel when it has -// multiple representation types (e.g., labelmap and surface) -const uniqueSegmentationMap = new Map(); -representations.forEach(representation => { - if (!uniqueSegmentationMap.has(representation.segmentationId)) { - uniqueSegmentationMap.set(representation.segmentationId, representation); - } -}); - -const newSegmentationsWithRepresentations = Array.from(uniqueSegmentationMap.values()).map( - representation => { - const segmentation = segmentationService.getSegmentation(representation.segmentationId); - const mappedSegmentation = mapSegmentationToDisplay(segmentation, customizationService); - return { - representation, - segmentation: mappedSegmentation, - }; - } -); -``` - -## How It Works -1. After retrieving all representations for a viewport, we create a `Map` keyed by `segmentationId` -2. We iterate through all representations and only add the first occurrence of each unique `segmentationId` -3. This ensures that even if a segmentation has multiple representation types (labelmap, surface, etc.), only one entry appears in the panel -4. The fix is applied at the data source level, ensuring consistency across all panel views - -## Testing -To verify the fix: -1. Launch OHIF Viewer with segmentation data (e.g., https://viewer-dev.ohif.org/segmentation?StudyInstanceUIDs=1.3.6.1.4.1.14519.5.2.1.1706.8374.643249677828306008300337414785) -2. Drag and drop a segmentation into the Common layout viewport -3. Switch to an advanced layout (e.g., 3D Four-Up) -4. Verify that only one entry per segmentation appears in the Segmentation Panel (no duplicates) - -## Impact -- **Scope:** Minimal - only affects the display logic in the Segmentation Panel -- **Breaking Changes:** None -- **Performance:** Negligible - adds a simple Map-based deduplication step -- **User Experience:** Significantly improved - eliminates confusion from duplicate entries - -## Related Files -- `/workspace/extensions/cornerstone/src/hooks/useViewportSegmentations.ts` (modified) -- `/workspace/extensions/cornerstone/src/hooks/useActiveViewportSegmentationRepresentations.ts` (uses the fixed hook) -- `/workspace/extensions/cornerstone/src/panels/PanelSegmentation.tsx` (consumes the data) diff --git a/platform/ui-next/src/contextProviders/ViewportGridProvider.tsx b/platform/ui-next/src/contextProviders/ViewportGridProvider.tsx index 1a9d5fb59..e88007c4e 100644 --- a/platform/ui-next/src/contextProviders/ViewportGridProvider.tsx +++ b/platform/ui-next/src/contextProviders/ViewportGridProvider.tsx @@ -404,8 +404,17 @@ export function ViewportGridProvider({ children, service }: ViewportGridProvider const getGridViewportsReady = useCallback(() => { const { viewports } = viewportGridState; - const readyViewports = Array.from(viewports.values()).filter(viewport => viewport.isReady); - return readyViewports.length === viewports.size; + // Filter viewports that have display sets (i.e., have content to display) + const viewportsWithContent = Array.from(viewports.values()).filter( + viewport => viewport.displaySetInstanceUIDs?.length > 0 + ); + // If there are no viewports with content, return false + if (viewportsWithContent.length === 0) { + return false; + } + // Check if all viewports with content are ready + const readyViewports = viewportsWithContent.filter(viewport => viewport.isReady); + return readyViewports.length === viewportsWithContent.length; }, [viewportGridState]); const setLayout = useCallback(