fix: Align GRID_STATE_CHANGED timing with viewport updates

[sedghi]

Summary

• add pending-viewport queue to viewportGridService so GRID_STATE_CHANGED fires only after every affected viewport finishes viewport data changed
• expose notifyViewportUpdateCompleted and call it from Cornerstone’s viewport service after each data change
  x

Also fixing the viewport grid not properly doing empty viewports
To repro: add more rows and columns in the HP

Before

---

Note

Defers GRID_STATE_CHANGED until all affected viewports complete data updates, adds notifyViewportUpdateCompleted (called by Cornerstone), handles empty panes in HP layouts, syncs provider state updates, and documents the timing change.

• Core: ViewportGridService
  • Add pending-change queue and notifyViewportUpdateCompleted(viewportId); publish GRID_STATE_CHANGED only after all dependent viewports finish viewport data changed.
  • Replace deferred setTimeout emits with immediate events; ACTIVE_VIEWPORT_ID_CHANGED now includes and returns state.
  • New internals: _queueGridStateChanged, _flushPendingGridStateChanges, _getViewportIdsWithDisplaySets.
  • setDisplaySetsForViewport(s) accepts preCallback; set, setLayout, and display-set updates now queue grid-state changes; reset/onModeExit clear pending queue.
• Cornerstone extension
  • Call viewportGridService.notifyViewportUpdateCompleted(viewportId) after VIEWPORT_DATA_CHANGED in setViewportData/updateViewport; early notify and return for empty stack imageIds.
• App UI (ViewportGrid.tsx)
  • findOrCreateViewport returns an empty viewport definition when no HP match (instead of logging and skipping).
• UI-next provider
  • Introduce APPLY_VIEWPORT_GRID_STATE and applyReducerAndCommit to compute-and-commit reducer updates synchronously; guard missing viewport in VIEWPORT_IS_READY.
• Docs
  • Migration guide: explain deferred GRID_STATE_CHANGED timing and requirement to call notifyViewportUpdateCompleted for custom viewports.

^{Written by Cursor Bugbot for commit d2c2cdb. This will update automatically on new commits. Configure here.}

[netlify]

✅ Deploy Preview for ohif-dev canceled.

Name	Link
🔨 Latest commit	d2c2cdb
🔍 Latest deploy log	https://app.netlify.com/projects/ohif-dev/deploys/6938b054b39eb3000846a0b9

[cypress]

Viewers    Run #5817

Run Properties:   Passed #5817  •  d2c2cdbf80: Merge branch 'fix/grid-state-changed' of https://github.com/OHIF/Viewers into fi...

Project	Viewers
Branch Review	fix/grid-state-changed
Run status	Passed #5817
Run duration	02m 23s
Commit	d2c2cdbf80: Merge branch 'fix/grid-state-changed' of https://github.com/OHIF/Viewers into fi...
Committer	Alireza
View all properties for this run ↗︎

---

Test results
Failures	0
Flaky	0
Pending	0
Skipped	0
Passing	37
View all changes introduced in this branch ↗︎

[Copilot]

Pull request overview

This pull request implements a deferred event broadcasting system for viewport grid state changes to address race conditions where listeners received incomplete viewport state. The grid service now queues state change events until all affected viewports complete their data updates, ensuring listeners always see the final, consistent state rather than intermediate updates.

Key Changes:

• Implemented viewport update notification system with queuing mechanism to defer GRID_STATE_CHANGED events until all viewport data updates complete
• Refactored ViewportGridProvider to use synchronous state application pattern with applyReducerAndCommit for immediate state access
• Added preCallback support to setDisplaySetsForViewport methods for pre-update hooks

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 7 comments.

File	Description
platform/core/src/services/ViewportGridService/ViewportGridService.ts	Implements pending state change queue with notifyViewportUpdateCompleted() tracking; adds preCallback parameter support; returns state from service methods
platform/ui-next/src/contextProviders/ViewportGridProvider.tsx	Refactors reducer to use useCallback and implements applyReducerAndCommit pattern for synchronous state updates via ref; replaces all dispatch calls
extensions/cornerstone/src/services/ViewportService/CornerstoneViewportService.ts	Adds notifyViewportUpdateCompleted() calls after viewport data change events to signal completion to grid service
platform/docs/docs/migration-guide/3p11-to-3p12/index.md	Documents timing changes for GRID_STATE_CHANGED event and migration path for custom viewports

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[cursor]

This is the final PR Bugbot will review for you during this billing cycle

Your free Bugbot reviews will reset on January 30

Details

You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.

To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.

[cursor]

Bug: Empty viewport pending IDs block GRID_STATE_CHANGED events

In setDisplaySetsForViewports, the pendingViewportIds on line 245 includes ALL updated viewports via updatedViewports.map(viewport => viewport.viewportId), even those with empty displaySetInstanceUIDs. However, viewports without display sets render EmptyViewport (which never calls notifyViewportUpdateCompleted), and setViewportData returns early without calling notifyViewportUpdateCompleted when imageIds are empty. This causes the pending queue to never clear for such viewports, blocking all subsequent GRID_STATE_CHANGED events. The fix should filter to only include viewports with displaySetInstanceUIDs, similar to how setLayout and set use _getViewportIdsWithDisplaySets.

 

[cursor]

Bug: Promise rejection prevents grid state change notification

The notifyViewportUpdateCompleted call is only placed inside .then() callbacks without any .catch() or .finally() handlers. If displaySetPromise rejects due to an error during viewport data loading, notifyViewportUpdateCompleted will never be called. This causes pendingGridStateChanges to wait indefinitely for that viewport, preventing GRID_STATE_CHANGED from ever being broadcast. Before this change, GRID_STATE_CHANGED would fire regardless of loading success (via setTimeout), but now the pending queue mechanism creates a dependency on successful completion.

Additional Locations (1)

• extensions/cornerstone/src/services/ViewportService/CornerstoneViewportService.ts#L1129-L1136

 

[wayfarer3130]

is published after the state has been updated/is available.

[wayfarer3130]

@sedghi - going to respond on slack about this PR - I have some real questions about the direction that this PR is taking

[wayfarer3130]

There are some good internal changes/fixes, but I think the overall change doesn't agree with the general direction we are trying to go in for OHIF. I think we want to:

• Document the viewport grid state and the zustand/useViewportGrid capabilities that are required
• Document how OHIF handles the transitions between states
• Document what state transition notifications are available - as either the zustand state updates OR the event models
• Create a change that exposes a number of zustand state event methods, integrated with your changes here for how the
  viewports get notified from CS3D about image updates.

For the viewport grid state, what it is trying to enable is:

• Viewport grid component to layout the viewport grid - this component must be able to listen to grid changes without needing to listen to viewport content changes
• Viewport component that listens to what is the current display set(s) in a viewport and initiates changes for recording that, but does NOT concern itself with the grid component
• Viewport internal state that knows about what image is displayed, what the orientation etc is.

I think the viewport grid state also needs to be able to be listened to in order to discover overall load state - but this should be a part of the viewport grid state and listened to via the useViewportGrid with an appropriate method to get the specific state part.
That would allow for things like "activate sync only when viewport grid is stable, and auto-deactivate on any composition changes"

You can do all these things with the new listeners you've added, but it requires multiple ways of doing things, and I think the future is really about using the state management with state selection to choose exactly what updates you get.
That way the ViewportGrid UI component would listen only to viewport grid layout changes.
The individual viewports would listen to the viewport type/component to render in them, and the display set
Ideally the individual viewport would have some way to get "current state" as well as the updated state in a way that works with CS3D state. OR, CS3D should also use zustand internally to hold the individual state.

I'm thinking something like a listener that wants to do something on the state having settled and be displaying the updated display set uids using:
const gridStatus = useViewportGrid( (state) => state.gridStatus)

Then, grid status could be used to show a spinny icon cursor or the like.

Individual viewports might render something like:

const viewportDisplaySets = useViewportGrid( (state) => state.getDisplaySetsForViewport(viewportId));
const viewportState = useViewportGrid(state => getViewportState(viewportId))
const setViewportState = useViewportGrid(state => state.setViewportState(viewportId))
... start loading
setViewportState('completed', loadedViewportDisplaySets)

when the viewport finishes loading.

Then the overall viewport state is defined internally to the store by the individual viewport states.
You might need to use both an id and a display set key to figure out the load state, as you don't want to setViewportState to completed on the wrong set of display sets

[wayfarer3130]

Would it be possible to create a state transition diagram showing the state information and what causes those state transitions, along with who is listening to what part of the state? It probably needs a few related diagrams, for updates to the display set/initial state as well as for the overall grid state. I think having that would allow us to talk about how the state access methods would work.