Update canvas size to match with device framebuffer size. #756
Conversation
src/lib/viewport-uniforms.js
Outdated
| modelViewMatrix: new Float32Array(modelViewMatrix), | ||
|
|
||
| // Screen size | ||
| // _Q2# : Verify and remove multiplicaiton with devicePixelRatio (assuming |
There was a problem hiding this comment.
@ibgreen @Pessimistress , is multiplication with devicePixelRation needed here? Who sets these viewport bounds?
There was a problem hiding this comment.
Yes. DeckGL defined viewport from user provided props. The uniform is used by layers to project pixel size to clipspace, which requires the actual canvas size.
src/react/webgl-renderer.js
Outdated
| // for clean logging, only calls gl.viewport if props have changed | ||
| _updateGLViewport() { | ||
| /* | ||
| let {viewport: {x, y, width: w, height: h}} = this.props; |
There was a problem hiding this comment.
@Pessimistress @ibgreen , how are these props set? Can we ignore this and alway use [0, 0, canvas.width, canvas.height] , given canvas width/height matches device framebuffer size? If need to use this.props, is it guranteed the size provided by props updated with devicePixelRatio?
There was a problem hiding this comment.
The size in props is not multiplied by devicePixelRatio. It is the width and height that users pass to the DeckGL component.
src/react/webgl-renderer.js
Outdated
| } | ||
|
|
||
| // Gets current size of canvas in css (logical/window) coordinates | ||
| _getCSSSize(canvas) { |
There was a problem hiding this comment.
All these added methods are copy pasted from luma, we can import from luma once we finalize the fix.
There was a problem hiding this comment.
clientWidth and clientHeight have perf impacts. Since we already know the canvas' width and height styles (set in the render method) we don't need the work around here.
There was a problem hiding this comment.
So this is one of trickiest aspects of the canvas API. Namely that width and height control the size of the drawing buffer, not the DOM element, and are set independently from the style.width, height (which are read back as clientWidth and clientHeight). We can't keep reading width and height since they will be multiplied by DPR, we'll keep multiplying the drawing buffer size. If there is a reflow issue with clientWidth and clientHeight we could use the props.width and props.height here as they are intended to be in logical coordinates.
src/react/webgl-renderer.js
Outdated
| // Lookup the size the browser is displaying the canvas in CSS pixels | ||
| // and compute a size needed to make our drawingbuffer match it in | ||
| // device pixels. | ||
| const cssSize = this._getCSSSize(canvas); |
There was a problem hiding this comment.
Just use this.props.width and this.props.height.
src/react/webgl-renderer.js
Outdated
| // for clean logging, only calls gl.viewport if props have changed | ||
| _updateGLViewport() { | ||
| /* | ||
| let {viewport: {x, y, width: w, height: h}} = this.props; |
There was a problem hiding this comment.
The size in props is not multiplied by devicePixelRatio. It is the width and height that users pass to the DeckGL component.
| if (newBufferSize.width !== canvas.width || newBufferSize.height !== canvas.height) { | ||
| // Make the canvas render buffer the same size as | ||
| canvas.width = newBufferSize.width; | ||
| canvas.height = newBufferSize.height; |
There was a problem hiding this comment.
These are overwriting what's set in the render() method. Should remove the width and height assignment in render to avoid confusion.
|
Remove the |
|
After this change we still have multiple copies of this line in the code: |
1chandu
force-pushed
the
CanvasSize
branch
2 times, most recently
from
20c1771 to
6229ffd
Compare
|
@ibgreen @Pessimistress , created this #760 to track remaining issues. |
src/react/webgl-renderer.js
Outdated
| // device pixels. | ||
| return { | ||
| width: Math.floor(canvas.clientWidth * cssToDevicePixels), | ||
| height: Math.floor(canvas.clientHeight * cssToDevicePixels), |
There was a problem hiding this comment.
Just use this.props.width and this.props.height?
clientWidth and clientHeight causes page reflow. We already know the canvas css size since we set it here in render().
There was a problem hiding this comment.
What is page reflow, is it because clientWidth/clientHeight can be different than props.width/height?
Based on discussion with @ibgreen (also I verified changing window size), the width and height extracted from props and set to style object in render(), will be set to canvas.clientWidth and canvas.clientHeight, so they should identical.
There was a problem hiding this comment.
The browser attempts to recalculate the layout of the whole page when you access certain layout properties. It triggers a lot more action under the hood than just reading the props object.
3 participants
devicePixelRation@ibgreen @Pessimistress : not ready to merge, have some open questions