feat(canvas): canvas packing - #2174
Conversation
f40b525 to
650ba56
Compare
| const VIEWPORT_MIN_SIZE = 2; | ||
|
|
||
| // Maximum size of the offscreen canvas | ||
| const MAX_WIDTH = 16384; |
There was a problem hiding this comment.
rename to max offscreen canvas width
|
|
||
| // Maximum size of the offscreen canvas | ||
| const MAX_WIDTH = 16384; | ||
| const MAX_HEIGHT = 16384; |
|
|
||
| // Create offscreen canvas and set it immediately | ||
| const offscreenCanvas = document.createElement('canvas'); | ||
| offscreenCanvas.width = MAX_WIDTH; |
There was a problem hiding this comment.
as discussed, let's detect the device isMobile and cap it at 4096 by 4096 based on this link https://jhildenbiddle.github.io/canvas-size/#/?id=test-results
| this.offscreenMultiRenderWindow = | ||
| vtkOffscreenMultiRenderWindow.newInstance(); | ||
| this.offScreenCanvasContainer = document.createElement('div'); | ||
| this.offScreenCanvasContainer.style.width = `${MAX_WIDTH}px`; |
There was a problem hiding this comment.
This immediately consumes the full size - it seems like a bad idea on limited devices. What about tesselating the current total screen area as a height/width? In theory that allows for any viewport displayed on screen non-overlapping, and won't consume more memory than the current screen size.
|
|
||
| // Create offscreen canvas and set it immediately | ||
| const offscreenCanvas = document.createElement('canvas'); | ||
| offscreenCanvas.width = MAX_WIDTH; |
There was a problem hiding this comment.
How many of these are we likely to have? If tehre are a lot, it will consume a huge amount of memory.
|
Please update the description - it isn't clear whether this resolves the very large monitor issue with very wide screens, or whether it just packs things into a single offscreen as a step towards getting multiple offscreen, nor does it really explain exactly what the packing method is in terms of height/width. I know it references it, but a diagram showing it, included in the final documentation would be quite helpful. |
There was a problem hiding this comment.
I'd really like to see some jest unit tests go over various packing situations, describing the expected results as part of the test. I can read the code, but having a unit test would give me a lot more confidence about this.
| } | ||
|
|
||
| /** | ||
| * Packs rectangles using a shelf packing algorithm. |
There was a problem hiding this comment.
An optimization that helps a lot for a large window with a ton of small windows (a very common occurance) is to apply the shelf packing recursively when the window height decreases, that is
packRectangles on a tall 1024x2048 window with 512x512 windows will first pack the 1024x2048 as the left/top most window, and then will recursively pack the 512x512 windows in the right half of that first window so that an area of 2048x2048 can be filled with 8 small windows and 1 tall window.
There was a problem hiding this comment.
If you start with a window that is the width/height of the tallest window, and progressively expand:
width if width< max_width
then expanding height accordingly. That can result in an entire shelf almost empty, but only if the previous shelf was
already filled. There are more efficient algorithms for expanding, but that one is pretty good, and most of the ones I can immediately think of are variations
|
|
||
| // @ts-expect-error | ||
| offScreenCanvasContainer.width = offScreenCanvasWidth; | ||
| offScreenCanvasContainer.width = MAX_WIDTH; |
There was a problem hiding this comment.
If you set this after checking the packing you can significantly reduce memory usage - otherwise I suspect you will crash many browsers.
|
@sedghi - does it really help to avoid multiple offscreen viewports, or is it just as easy to have one per viewport? If it works to have lots of offscreen viewports, then the easiest solution is to just allocation one per viewport and render to it directly, without worrying about any shelf algorithms. |
The WebGL context limit is 16, you can't have more viewports than that. |
theres some text here that describes the limitations, also this PR is not final yet, there's issues on linux and windows, and we are having difficuilty figuring out what the actual canvas width/height limit is |
IbrahimCSAE
changed the title
IbrahimCSAE
changed the title
3 participants
No description provided.