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

Correctly calculate GLViewport with Framebuffer #6553

Merged
merged 4 commits into from
Jan 13, 2022

Conversation

felixpalmer
Copy link
Collaborator

For #6552

Background

When passing a Framebuffer as a target the GLViewport calculated assumed the width and height to be equal to the GL canvas

Change List

  • When a custom target is passed to the layers pass, use its width&height rather than that of the canvas
  • When the moduleParameters specify a pixel ratio, respect this when calculating the GL viewport

@Pessimistress
Copy link
Collaborator

Pessimistress commented Jan 10, 2022

If I understand this approach correctly, it expects the application to calculate and pass a devicePixelRatio, which is not straightforward, yet it still doesn't account for the case where the framebuffer's aspect ratio does not match the canvas.

The canvas's aspect ratio is never wrong because we base the dimensions of viewport on the canvas's size. The same cannot be said for an external framebuffer.

I think it would be more robust to take the dimensions of the canvas and "stretch to fit" into the dimensions of the external framebuffer.

@coveralls
Copy link

coveralls commented Jan 11, 2022

Coverage Status

Coverage increased (+0.01%) to 80.471% when pulling 57c876a on felix/layer-pass-framebuffer into da1a3ab on master.

@felixpalmer
Copy link
Collaborator Author

This does work for a external framebuffer as the dimensions (and thus aspect) are based on the viewport that is passed in. I have it working with a square framebuffer in #6554

The moduleParameters.devicePixelRatio is optional and is used when rendering into the framebuffer. I need this when rendering to a fixed size off-screen framebuffer as otherwise the y-axis inversion is wrong. If I cannot override this then it becomes difficult to construct a projectionMatrix that I can use when reading the framebuffer later.

I think it is reasonable to expect that the following works

const target = new Framebuffer(gl, {width, height});
const viewport = new WebMercatorViewport({width, height});
deck.deckRenderer.renderLayers({
  target,
  viewports: [viewport],
  moduleParameters: {devicePixelRatio: 1},
  ...
});

_getModuleParameters already accepts the passed in moduleParameters as an override and these then get passed onto the shaders. It just happens to be the case that getGLViewport ignores this and hardcodes the pixel ratio to the cssToDeviceRatio default.

I can see that the stretch-to-fit approach would work for the case of rendering the current canvas into a framebuffer, but if I just want to render a fixed view into a fixed framebuffer why should this be dependent on the canvas pixel ratio & size?

Does this make sense? Perhaps I'm misunderstanding what you mean by "stretch-to-fit"?

Copy link
Collaborator

@Pessimistress Pessimistress left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes it makes sense. Thank you for the clarification.

modules/core/src/passes/layers-pass.js Outdated Show resolved Hide resolved
@felixpalmer felixpalmer merged commit 4763704 into master Jan 13, 2022
@felixpalmer felixpalmer deleted the felix/layer-pass-framebuffer branch January 13, 2022 11:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants