Skip to content
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

How should we size the canvas for best experience and render? #159

Closed
chrisgervang opened this issue Sep 16, 2021 · 5 comments · Fixed by #171
Closed

How should we size the canvas for best experience and render? #159

chrisgervang opened this issue Sep 16, 2021 · 5 comments · Fixed by #171
Assignees
Labels
question Further information is requested

Comments

@chrisgervang
Copy link
Collaborator

chrisgervang commented Sep 16, 2021

I'm trying to create a canvas with these properties:

  1. the framebuffer size is set to the desired video resolution (and can re-scale as export settings change)
  2. the canvas element is scaled to fit in the user interface (and can re-scale as the user resizes the window)
  3. the viewport boundaries in the world don't change when framebuffer resolution changes

So far we've implemented this two ways with different results. Each only satisfy 2 of the 3 requirements.

Solution A: window.devicePixelRatio = factor

  1. ⛔️ - The actual framebuffer size is not always the expected size.
  2. ⛔️ - When container changes size the viewport bounds change.

Through an understanding of deck/mapbox internals we've determined the framebuffer size is set as a function window.devicePixelRatio and canvas css size. If we control these variables, we can indirectly control the framebuffer size. This sometimes works as expected, but also may result in an inaccurate height in the export depending on the container's size. The issue results in images with this resolution discrepancy:

Expected resolution: 3840 x 2160
Actual: 3840 x 216*5*

Here is where we set the devicePixelRatio which will internally be used by deck.gl and mapbox-gl-js to set the framebuffer size. Note: The issue also occurred before we rounded to the nearest even.

this._setDevicePixelRatio(nearestEven(dimension.width / width));

The canvas is styled to fit a container, and the container is set to an explicit size.

const deckStyle = {
width: '100%',
height: '100%'
};
const containerStyle = {
width: `${width}px`,
height: `${height}px`,
position: 'relative'
};

Note: The deck width/height props are set explicitly to the expected dimensions. But these are not used to set framebuffer size.

width={dimension.width}
height={dimension.height}

Current Conclusion

Resolution error is introduced when framebuffer size is derived in deck.gl and mapbox by multiplying their container size by devicePixelRatio. There doesn't appear to be an interface for explicitly setting framebuffer width/height in both libraries.

At some container sizes this solution works as expected, but further investigation is needed to ensure the output is always the expected resolution.

So far we've observed this solution works as expected when:

  • When requirement 2. is relaxed: when the scale factor is 1.0 and the container size matches the resolution.
  • When the browser window is sized to 1920x1080 with chrome dev tools, and the container height is hard coded to 1080px.

Solution B: transform: scale(${factor})

  1. ⛔️ - The boundary of the viewports have changed in these two images where the viewState is fixed and canvas internal size has changed.

If we apply a scale factor to the deck/mapbox canvas as a CSS transform we can fit a canvas of any size into a container of any size. Deck and mapbox were designed to be explicitly sized, so standard methods can be used to get actual framebuffer matching our desired resolution. However, window.devicePixelRatio = 1 is necessary to ensure retina (or other HiDPI) displays do not effect the framebuffer size.

In this example we set window.devicePixelRatio = 1, a random container size, the same view state. We switch between 1080p or 4K resolution. See the framebuffer size in the images as "Canvas Internal Size".

1080p

4K

Current Conclusion

It is very common to resize a video and expect the same boundary given a viewState, so this method is not viable unless there is a way to set the viewState and the bounds of the viewport. Currently we do not have a get/setBounds function for our viewports which works at any perspective.

@chrisgervang chrisgervang added the question Further information is requested label Sep 16, 2021
@chrisgervang
Copy link
Collaborator Author

chrisgervang commented Sep 16, 2021

Alternate idea I haven't implemented:

  • Use an off-screen canvas for rendering.
    • Solution 1 seems to work when Requirement 2 was relaxed. We can relax it since the user won't see it.
    • I expect this has the same viewport boundary issue as Solution 2.
    • Still need a preview. Use Solution 1 if its ok to be inaccurate by a few pixels?? Or Solution 2 w/ viewport addressed.

@chrisgervang
Copy link
Collaborator Author

Digging deeper I've realized that so far neither solution satisfies requirement 3. yet, and I spent some time figuring out why.

“Resolution setting” is user defined setting of the expected image export.
“Container” is canvas.clientHeight/Width
“Rendered resolution” is the actual image export resolution. It’s canvas.height/width
“DPI” is window.devicePixelRatio
“Viewport bounds” is the perspective boundary of the rendered world (would be a new viewport.getBounds().

In Solution 1, it’s observed that the viewport changes when the user resizes the container. It’s also observed that locking the container size results in a fixed viewport at any resolution setting, and this results in the expected rendered resolution because the DPI is changing.

In Solution 2 it’s observed that the viewport changes when resolution setting is changed because DPI is fixed, and the container changes to export the expected rendered resolution. It’s also observed that locking the resolution setting and setting any container size results in a fixed viewport because the rendered resolution is exactly set to the resolution setting, the container is scaled with CSS transform, and DPI is fixed.

So it seems the only way to keep the viewport bounds fixed in the current system is to not change the container size. Next I'm going to investigate if I can change DPI while maintaining a viewport - if I can, I'm interested in understanding why because the frameworks pass that into webgl and I'd like to have more direct control if I can.

@chrisgervang
Copy link
Collaborator Author

change DPI while maintaining a viewport

I'm happy to see changing window.devicePixelRatio can change the resolution without changing viewport, so it seems the canvas.clientWidth/Height is directly related to viewport bounds. Next I'll look at how this works.

I want a better solution than changing window.devicePixelRatio, since it's undefined behavior. Being a global variable and it'll also cause effects on other components relying on the variable.

Low DPI (expecting blurry)

Screen Shot 2021-09-20 at 5 47 37 PM

High DPI

Screen Shot 2021-09-20 at 5 47 23 PM

@chrisgervang
Copy link
Collaborator Author

chrisgervang commented Sep 21, 2021

@ibgreen I've been watching the code that runs when resizing the canvas or changing DPI and I have more (but not all) confidence a shared drawing buffer resolution is controlled by our frameworks rather than mapbox. Deck's AnimationLoop constantly checks for canvas size/DPI changes and calls luma's resizeGLContext.

Luma floors the resolution, which partly explains why I'm seeing slightly unexpected resolutions.

I'd like to have a new feature on resizeGLContext / AnimationLoop that allows me to directly set the inner canvas width/height and ignore DPI. The viewport uses the canvas client width/height to set the viewport bounds - I'll either need to set the canvas to something constant or directly set the viewport size somewhere.

Edit: In the mean time I'm going to hack together a proof that our frameworks have control of the mapbox gl context resolution.

@chrisgervang
Copy link
Collaborator Author

Today I took a slight tangent to see if we could render both deck and mapbox to a non-default framebuffer that we define. While it was easy to do with deck, I don't think it is possible in mapbox right now. Implementation in 26dbd2f.

DeckRender allows the target framebuffer to be overridden,
opts.target = opts.target || Framebuffer.getDefaultFramebuffer(this.gl);

The Painter in mapbox doesn't appear to have it where I expected it,
this.context.bindFramebuffer.set(null);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants