fix(rendering): Introduce SequentialRenderingEngine to resolve canvas size limitation, performance degradation on high res monitors and enhance multi-monitor support - #2181
Conversation
1fd25cf to
cd9a724
Compare
sedghi
left a comment
There was a problem hiding this comment.
Great job! I'll review it, but I'm going to request changes to prevent accidental merging. This should really be a separate mode in RE, set during cscore.init.
|
Can we get the adapter/tool changes reverted, unless specifically related? |
sedghi
left a comment
There was a problem hiding this comment.
I went through all 158 examples, and they're working well. It looks like only the orientation marker is broken, which I assume you're already addressing. This is a really great pull request—one of the best we've had in recent years!
|
Wow that's seems really amazing, congratulations ! |
There was a problem hiding this comment.
I made this one configurable with rows and columns, going to 12x22. There is a significant pause about 1/3 of the way through - not that I'm concerned, just noticing it.
wayfarer3130
left a comment
There was a problem hiding this comment.
I added a rows and columns parameter, and then tested with rows=16 and columns=32
There is a really long lag about 1/3 of the way through, then the remaining windows appear immediately after the lag. I'd suggest squashing this (I can do this), but would be nice to see if there is anything immediately obvious.
|
As discussed in slack, we can improve this later. I'm gonna merge |
Overview
This PR introduces a new
SequentialRenderingEngineto fundamentally solve the issue of viewport cropping, misalignment, and terrible performance on high-resolution displays or in layouts with many viewports.The Problem: Offscreen Canvas Overflow & Performance Degradation Due to Ofscreen Canvas Growth
Cornerstone3D has historically used a single, large offscreen canvas for all viewports. This canvas grows horizontally as more viewports are added. This strategy is efficient until the total width of all viewports exceeds the browser's maximum canvas size (e.g., 16,384px in Chrome).
When this limit is breached, the offscreen canvas is silently cropped. Since the engine copies pixel data from fixed locations on the offscreen canvas to the on-screen viewports, this cropping results in severe visual artifacts, including misaligned and completely blank viewports. This issue is easily reproducible on high-DPI monitors or by manually increasing
window.devicePixelRatio. The peformance also heavly degrades as the limit is approached.Visual Example of the Bug:
Diagram of Current RenderingEngine:
The Journey to a Solution
Before arriving at the final solution, several alternative approaches were extensively researched and implemented. Each had significant drawbacks that made them unsuitable:
VolumeViewportsmust share the same offscreen canvas. OnlyStackViewportssaw a partial benefit.The Solution:
SequentialRenderingEngineThis PR introduces a new, robust rendering strategy: the
SequentialRenderingEngine.Instead of a single, massive offscreen canvas that grows with each new viewport, this engine uses a fundamentally different approach. It renders each viewport individually to a viewport sized offscreen canvas, copies the result to the corresponding on-screen canvas, and then proceeds to the next viewport, reusing the same offscreen canvas.
This elegant solution completely bypasses the previous pain points:
Visual Example of the Fix:
Diagram of Sequential RenderingEngine:
Implementation Details
This PR includes the following key changes:
RenderingEngineRefactor: The originalRenderingEnginereturns an instance of eitherStandardRenderingEngineorSequentialRenderingEngine, based on a property that can be configured in the init function, the property isrenderingEngineMode, which is a string that can be eitherstandardornextBaseRenderingEngine: A new abstract base class has been created to define a common rendering engine interface.StandardRenderingEngine: The originalRenderingEnginehas been moved to the classStandardRenderingEnginewhich extendsBaseRenderingEngineSequentialRenderingEngine: The new engine is implemented as a separate class, also extendingBaseRenderingEngine.worldToCanvasandcanvasToWorldmethods on bothStackViewportandVolumeViewporthave been updated to support either rendering engine.Timeline
Usage
The new Sequential Rendering Engine is now the default in Cornerstone3D. If you wish to continue using the old rendering engine, you can override this behavior by explicitly setting the renderingEngineMode to 'standard'.
To use the old rendering engine:
Demos and Previews
OHIF
Video: https://streamable.com/41h2xy, You can see me experience a context loss in the video, when I try to go back from one up to the grid, due to how huge the offscreen is.
OHIF after: https://gorgeous-sable-7bbec5.netlify.app/viewer?StudyInstanceUIDs=1.3.12.2.1107.5.1.4.60175.30000008042114404745300000010&hangingProtocolId=@ohif/mnGrid
Cornerstone
Reference of previous attempts and useful resources
PRs
Packing
Canvas size limits per browser
WebGL context limits
This is different per OS and browser
Thanks
Thanks to @wayfarer3130 @sedghi for their help and insights that helped me brainstorm a solution. I wouldn't have been able to resolve this without them.