greggman commented 10 years ago

I'm just curious but have you considered making CSS based sizing the default? If you did less code would have to change for various use cases

For example many samples do stuff like this

camera = new THREE.PerspectiveCamera( 
    50, window.innerWidth / window.innerHeight, 1, 1000 );

And or they set

renderer.setSize( window.innerWidth, window.innerHeight );

But if instead you based the size of the canvas off its clientWidth and clientHeight then it would just always work. The user can make container and set it's CSS to any size. For example

<style>
html, body {
   width: 100%;
   height: 100%;
   margin: 0px;
   padding: 0px;
   overflow: hidden;  // make sure no scrollbar
}
#c {
   width: 100%;
   height: 100%;
}
</style>
<body>
  <canvas id="c"></canvas>
</body>

And it just works. If they change the canvas to be some specific size like

#c {
  width: 400px;
  height: 300px;
}

Then it still just works.

If they put the canvas inside some other containers (say they are making an editor and there's a side tab) it all just works. Whereas the way three is now you always have to go muck with the code for a different use cases.

You'd have to stop mucking with style.width and style.height inside three but as it is it seems like three is fighting the web platform instead of embracing it.

Thoughts?

To be clear see these samples. They all use exactly the same JavaScript (in fact they're all pointing to the same JavaScript file). Because they use CSS and clientWidth, clientHeight nothing needs to change

Full size window with user created canvas http://greggman.com/downloads/examples/three-by-css/three-by-css-01.html

80% size with user created canvas (size the window) http://greggman.com/downloads/examples/three-by-css/three-by-css-02.html

Inline for article with user created canvas http://greggman.com/downloads/examples/three-by-css/three-by-css-03.html

Full size window using container div where three.js makes the canvas http://greggman.com/downloads/examples/three-by-css/three-by-css-by-container-01.html

80% size with using container div where three.js makes the canvas (size the window) http://greggman.com/downloads/examples/three-by-css/three-by-css-by-container-02.html

Inline for article using container span where three.js makes the canvas http://greggman.com/downloads/examples/three-by-css/three-by-css-by-container-03.html

No container div, uses the body, three.js creates the canvas http://greggman.com/downloads/examples/three-by-css/three-by-css-body-only.html

This one is same as inline but uses box-sizing: box-border; which mean even though the container is set to 400px/300px the border is subtracted from the inside so the canvas needs to be 390/290 and again it just works because that's what clientWidth and clientHeight are. http://greggman.com/downloads/examples/three-by-css/three-by-css-by-container-04.html

mrdoob commented 10 years ago

Hmmm, as far as I'm aware the renderer should already getting the size from the passed canvas? If not, that's a bug :)

As per the camera... how about making a method for that?

camera.setAspectFromRenderer( renderer );

WestLangley commented 10 years ago

// [requestAnimationFrame] should always go at the bottom. That way you can actually debug.
// With it at the top the problem is even if your code causes an exeption
// you've already asked the browser to queue a new event so the code will
// keep executing

@mrdoob comment?

mrdoob commented 10 years ago

// [requestAnimationFrame] should always go at the bottom. That way you can actually debug.
// With it at the top the problem is even if your code causes an exeption
// you've already asked the browser to queue a new event so the code will
// keep executing

@mrdoob comment?

Heh! Interesting. If I remember correctly, the reason I put it on the top is because @greggman himself once suggested somewhere to have it on top to avoid ms delays consumed by executing the code. Happy to change the pattern.

greggman commented 10 years ago

As for where to put RaF, I probably did say that :-P

As for the rest though, nearly all the three examples use window.innerWidth/window.innerHeight which means when someone wants to use them as a base they have code to edit if their use case is different. If all the samples instead used canvas.clientWidth, canvas.clientHeight for both setting the the size of the canvas and for setting the aspect ratio they wouldn't have to edit any code to repurpose the samples.

Also, the default for three.js is that when you call setRendererSize it changes the css size. It seems like it should never set the css size, no? The 7 samples I posted are trying to demonstrate that given a certain default the same code works for 7 use cases with no changes whereas nearly all of the examples require changes for any use case other than "fill the window"

For the specific case of setting the aspect, yea, a method sounds good. I'd consider changing all the examples to use it though (yea, I know, work :P) Just passing on the idea.

mrdoob commented 10 years ago

Also, the default for three.js is that when you call setRendererSize it changes the css size. It seems like it should never set the css size, no?

That's because three.js takes care of devicePixelRatio for you. In your use cases the user would need to take care of it, no?

greggman commented 10 years ago

no, devicePixelRatio works just fine in all those samples.

On Tue, Jun 10, 2014 at 6:31 AM, Mr.doob notifications@github.com wrote:

Also, the default for three.js is that when you call setRendererSize it changes the css size. It seems like it should never set the css size, no?

That's because three.js takes care of devicePixelRatio for you. In your use cases the user would need to take care of it, no?

— Reply to this email directly or view it on GitHub https://github.com/mrdoob/three.js/issues/4903#issuecomment-45546888.

greggman commented 10 years ago

In fact even better, with clientWidth and clientHeight used every where appropriate, if you explicitly set the size of the canvas everything else still just works.

In other words if I remove the code that adjusts the size of the canvas the rest of the code that uses clientWidth and clientHeight still works.

Here's the same 3 examples with the canvas set to an explicitly small size so to make it clear I'm explicitly setting the canvas size and yet the aspect ratio is still correct. Even as I scale the window the aspect ratio stays correct without changing the size of the canvas.

http://greggman.com/downloads/examples/three-by-css/three-by-css-explicit-canvas-size-01.html

http://greggman.com/downloads/examples/three-by-css/three-by-css-explicit-canvas-size-02.html

http://greggman.com/downloads/examples/three-by-css/three-by-css-explicit-canvas-size-03.html

Could also make 3 samples that use THREE to set the canvas size but still let CSS pick the display size (like all these samples do)

On Tue, Jun 10, 2014 at 7:15 AM, Gregg Tavares github@greggman.com wrote:

no, devicePixelRatio works just fine in all those samples.

On Tue, Jun 10, 2014 at 6:31 AM, Mr.doob notifications@github.com wrote:

Also, the default for three.js is that when you call setRendererSize it changes the css size. It seems like it should never set the css size, no?

That's because three.js takes care of devicePixelRatio for you. In your use cases the user would need to take care of it, no?

— Reply to this email directly or view it on GitHub https://github.com/mrdoob/three.js/issues/4903#issuecomment-45546888.

greggman commented 10 years ago

Here's ones that explicitly set the size by calling renderer.setSize but still use clientWidth clientHeight where appropriate

http://greggman.com/downloads/examples/three-by-css/three-by-css-by-container-explicit-canvas-size-01.html

http://greggman.com/downloads/examples/three-by-css/three-by-css-by-container-explicit-canvas-size-02.html

http://greggman.com/downloads/examples/three-by-css/three-by-css-by-container-explicit-canvas-size-03.html

http://greggman.com/downloads/examples/three-by-css/three-by-css-by-container-explicit-canvas-size-04.html

On Tue, Jun 10, 2014 at 6:31 AM, Mr.doob notifications@github.com wrote:

Also, the default for three.js is that when you call setRendererSize it changes the css size. It seems like it should never set the css size, no?

That's because three.js takes care of devicePixelRatio for you. In your use cases the user would need to take care of it, no?

— Reply to this email directly or view it on GitHub https://github.com/mrdoob/three.js/issues/4903#issuecomment-45546888.

WestLangley commented 10 years ago

Also see @greggman's article: http://games.greggman.com/game/webgl-anti-patterns/

mrdoob commented 8 years ago

So, you are proposing:

Passing a canvas reference to the WebGLRenderer and automatically adjust to it's clientWidth and clientHeight.
Updating the camera's projection matrix at render time if clientWidth or clientHeight changes.
Making the user add window.devicePixelRatio to Mesh*Material.wireframeLinewidth, Line*Material.linewidth and PointsMaterial.size.

In order to transition to that, how about we add a property to the renderer by now?

var canvas = document.getElementById( 'viewport' );
var renderer = new THREE.WebGLRenderer( { canvas: canvas } );
renderer.autoResize = true;

Or maybe we can assume we're in auto resize mode when the user passes a canvas in the constructor?

mrdoob commented 8 years ago

What happens when we serialise the data (like when we save in the editor)? Say that we serialise PointsMaterial.size and the user's computer window.devicePixelRatio is 2. Should we do PointMaterial.size / window.devicePixelRatio at serialising time?

greggman commented 8 years ago

That's a good point. One problem there are several use cases.

Case #1: The user wants PointsMaterial.size to be the smallest point possible.

In other words they want gl_PointSize = 1.0 and they want the smallest point the GPU can render so they don't want to multiply device pixel ratio. This might be less true for points? It's certainly common for lines.

Case #2: The user wants PointsMaterial.size to be device independent but not view dependent.

In this case it's generally gl_PointSize = desiredSize * devicePixelRatio;.

Case #3: The user wants PointsMaterial.size to be device independent and view dependent.

In other words they want the points to get smaller based on their distance from the camera. This one probably also includes devicePixelRatio. Unfortunately it's actually kind of a bogus case because see case #4

Case #4: The user wants PointsMaterial.size to be device independent and view dependent and match the unit size.

4 is different than #3 in that it means you want the points to be a certain size relative to normal triangle based geometry. This is arguably what most traditional 3d scenes would want. If cube is 1x1x1 unit and the desired point size = 1 then a point displayed at the same world location is the same size as the cube.

This is something like gl_PointSize = desiredSize * resolutionFactor * view;

Note there is no devicePixelRatio here. If the display is 4000 pixels across and a 1 unit cube 12 units in front of the camera ends up being rendered 50x50 pixels then gl_PointSize would end up being 50.

Maybe this is how you really want PointMaterial to work most of the time? That way regardless of the size of the user's display points will be the same size relative to other things in the scene.

WestLangley commented 8 years ago

If cube is 1x1x1 unit and the desired point size = 1 then a point displayed at the same world location is the same size as the cube.

We currently do that for Sprite and PlaneGeometry (#3894).

greggman commented 8 years ago

There's also arguably Case #5: The user wants unit sized points but not view dependent. In other words

 gl_PointSize = desiredSize * halfResolutionOfRenderTarget;

So a 1 unit point takes the same space as a 1 unit quad. Basically the same as #4 but without a view.

In any case it seems like PointsMaterial should support #4 if it doesn't already. That would make the users's scene show up the same on all devices. If you put some points around a cube they'll stay the same relative size across devices and there's no reference to devicePixelRatio.

WestLangley commented 8 years ago

The approach by @greggman appears to be working nicely, but I am having trouble getting points to be sized the same on devices with different DPRs -- both when attenuated, and when not.

Lines and wireframes look good.

For reference, here is my testbed: http://jsfiddle.net/LbLfu76e/

greggman commented 8 years ago

As for setting the aspect when creating the camera you could have it check if there's only 3 arguments. Or for that matter no arguments, 1, 3, 4,

 0 args = 45, auto, 0.1, 1000
 1 args = arg0, auto, 0.1, 1000
 3 args = arg0, auto, arg1, arg2
 4 args = arg0, arg1, arg2, arg3

auto meaning auto could either be 1 or it could set the camera's auto set aspect ratio flag in which case calling render.scene(...camera) would set the aspect to whatever the current render target is? With ES6 you can name the args.

You could also make the camera constructor take an object

 new THREE.PerspectiveCamera({ fov: 45, zNear: 1, zFar: 1000});

And check in the constructor if the first argument is an object.

I guess auto adjust on the camera also has issues. Ideally you want to set the aspect to canvas.clientWidth / canvas.clientHeight but if you're rendering to render target you don't know the aspect ratio. You can assume it's width / height but you don't know for sure. I suppose in the rare case where it's not the user can not set auto on the camera.

WestLangley commented 8 years ago

if you're rendering to render target you don't know the aspect ratio. You can assume it's width / height but you don't know for sure.

@greggman What, in your view, is the use case in which width / height would not be correct?

Actually, we have been doing it wrong. The camera's aspect should be the same as the viewport's aspect -- not the canvas asspect -- unless, of course, the viewport is the same size as the canvas.

greggman commented 8 years ago

The camera aspect and the viewport are unrelated.

  <canvas width="100" height="200" style="width: 400px; height: 300px"></canvas>

This will give you a canvas that is 100x200 pixels (thin and tall) but displayed 400x300 pixels (wide and short).

To render to the entire thing (not have your vertices clipped) you need to set the viewport to 100x200 which is gl.viewport(0, 0, gl.canvas.width, gl.canvas.height); or more correctly gl.viewport(0, 0, gl.drawingBufferWidth, gl.drawingBufferHeight);

To render to the correct aspect ratio it's aspect = 400/300 or aspect = canvas.clientWidth / canvas.clientHeight.

You can see examples here.

WestLangley commented 8 years ago

@greggman You are constructing an unusual example. I am not aware of any three.js example where the user wants the canvas stretched. (Rescaled for performance reasons -- maybe -- but only if the aspect is unchanged.)

Consider a three.js example in which the viewport is a (proper) subset of the canvas. For simplicity, assume DPR is 1.

renderer.setSize( 800, 1200 ); // tall and thin
renderer.setViewport( 200, 500, 600, 400 ); // short and wide
var viewportAspect = 600 / 400;
camera = new THREE.PerspectiveCamera( 40, viewportAspect, 1, 1000 ); // correct

To prevent distortion of the rendering, the camera aspect must match the viewport aspect. This is always the case.

So, continuing with your suggestion of using CSS-based sizing, the resize() callback would look like this:

function onWindowResize() {

    var canvas = document.getElementById( 'canvas' );

    // get the display size in CSS pixels -- also see canvas.getClientBoundingRect()
    var width = canvas.clientWidth;
    var height = canvas.clientHeight;

    // reset the size of the drawing buffer
    renderer.setSize( width * dpr, height * dpr, false ); // pass false here! // <=== scale by dpr

    // reset the viewport (because .setSize() changes it)
    var viewportWidth = 600 * dpr;
    var viewportHeight = 400 * dpr
    renderer.setViewport( 20 * dpr, 20 * dpr, viewportWidth, viewportHeight );

    // reset the camera
    var aspect = viewportWidth / viewportHeight;

    // orthographic
    camera.left = 0.5 * frustumSize * aspect;
    camera.right = - 0.5 * frustumSize * aspect;
    camera.top = 0.5 * frustumSize;
    camera.bottom = - 0.5 * frustumSize;

    // perpsective
    camera.aspect = aspect;

    camera.updateProjectionMatrix();

    // render
    render(); // if no animation loop

}

greggman commented 8 years ago

These are not three.js but I disagree that having the canvas's display size not match it's backbuffer size is not normal. Most of the samples by me on this page have a fixed size canvas regardless of the size it's displayed.

Maybe I'm missing it but it feels objectively true that most people want the aspect ratio to end up drawing things where 1 unit in X = 1 unit in Y. That that happens to be true if the canvas's display size is the same as it's backbuffer size is irrelevant. You can do the correct math and it will always be correct regardless of what the whether the CSS is makes them match or not. Why fight that?

What do gain from not following that?

In fact, given the fact that asking for a certain size is not guaranteed in WebGL if you don't do that you'll get the wrong result. Three.js saw this on the 5k imac. You'd ask for some giant canvas, you'd get back some smaller canvas than you asked for that was not the aspect ratio you asked for. Setting the viewport to drawingBufferWidth, drawingBufferHeight and setting the aspect to clientWidht / clientHeight will continue to give you an image that appears correct. Not doing it will give you a distorted image.

WestLangley commented 8 years ago

With all due respect, it appears to me you misunderstood my previous post. I will restate it.

The display size can be different from the drawing buffer size due to CSS style settings, but the drawing buffer aspect and CSS aspect should be identical.
The viewport can be a subset (sub-region) of the drawing buffer.
The three.js camera's aspect must match the aspect of the viewport -- not the aspect of the drawing buffer, and not the ratio clientWidth / clientHeight.
I provided code to implement it.

greggman commented 8 years ago

Sorry West, you're right. I probably mis-read your example. In your example I assume you want to set the viewport to 600x400 at 20x20?

This is a complicated area. Let's say your on an iMac5K.

You do this

var width = canvas.clientWidth;       // 2560
var height = canvas.clientHeight;     // 1440

// reset the size of the drawing buffer
renderer.setSize( width * dpr, height * dpr, false ); // pass false here! // <=== scale by dpr

you passed 5120, 2880 but you actually get 4096x2880 In other words

console.log(canvas.drawingBufferWidth, canvas.drawingBufferHeight);

prints

4096 2880

So now the question is how to do set viewport and the aspect. Let's assume the user wants to split the canvas 2x1, so 50% on each size. Currently they'd likely pass in a viewport width of 1280. If you really want to make it 50% then it's

  viewportX = desiredViewportX * gl.drawingBufferWidth / canvas.width;
  viewportY = desiredViewportY * gl.drawingBufferHeight / canvas.height;
  viewportWidth = desiredViewportWidth * gl.drawingBufferWidth / canvas.width;
  viewportHeight = desiredViewprotHeight * gl.drawingBufferHeigth / canvas.height;

  gl.viewport(viewportX, viewportY, viewportWidth, viewportHeight);

The aspect is still a separate issue because it's still being displayed the size requested. The correct aspect in this case seems like it would be

 var visualWidth = viewportWidth * canvas.clientWidth / gl.drawingBufferWidth;
 var visualHeight = viewportHeight * canvas.clientHeight / gl.drawingBufferHeight;

  camera.aspect = visualWidth / visualHeight;

That example fits this one where arguably the desire is to split the screen at 50% across and then split the right half at 50% down. So, if you have a canvas that is being displayed where the canvas's drawingBuffer size doesn't match its display size then you'd need to do the math as above.

greggman commented 8 years ago

I thought I'd revisit this again. I hope this is a simpler and more concrete example. I also hope I'm not forgetting anything above.

I'm not sure this is still a goal but at one point I thought someone said a goal of three.js was to be as simple as possible.

These changes would remove a few lines of JavaScript from nearly every sample but add a few lines of CSS. It would also make the remain code more flexible/portable.

See this diff

So, in particular

What do you think about setting the camera's aspect ratio automagically (see example above)?

To stay backward compatible I added

renderer.setAutoSize( true )

With that set then when you call

renderer.render( scene, camera )

The camera's aspect is set to

camera.aspect = _canvas.clientWidth / _canvas.clientHeight;

Could also make it handle render targets as well I think

That would remove at least one step. The line camera.aspect = .... in the resize function in nearly every sample would disappear

You could also change THREE.PerspectiveCameras signature so it's both

THREE.PerspectiveCamera(fov, aspect, zNear, zFar)

and

THREE.PerspectiveCamera(fov, zNear, zFar)

Which I did in the sample because once the camera is automatic there's no reason to even care what the aspect is for most projects so no need to pass it in anywhere.

Is that not a win?

I could do the same for and Orthographic camera and make it default to the width and height of the canvas if autoSize is true.

What about auto-resizing the canvas to match its display size?

Again, to be backward compatible only if you call renderer.setAutoSize( true );

That would remove several more lines of code from most projects. No need to call renderer.setSize period, no need to set up a window resize handler, no need for the window resize handler code. (again see example)

Thoughts?

mrdoob commented 8 years ago

I like it! But I think the multiple constructor parameters is an issue.

I would even go with THREE.PerspectiveCamera2, THREE.AutoPerspectiveCamera, ...

greggman commented 8 years ago

If those are the only choices I'd pick THREE.PerspectiveCamera2.

I'm hoping that if you approve this I can refactor all the examples to be this style which would basically make THREE.PerspectiveCamera deprecated though.

I'm curious why it's bothersome. It's a common JavaScript pattern. The most common pattern is the last argument is a callback and all previous arguments are optional. gl.texImage2D is another with a 6 argument and a 9 argument version.

Another idea would be to make the first parameter to THREE.PerspectiveCamera a number or object. So you could do this

var camera = new THREE.PerspectiveCamera({ fov: 60, near: 0.1, far: 300 });

Thoughts?

trusktr commented 6 years ago

@greggman CSS-based sizing is default in infamous. You can see it, for example, by resizing the browser window on this pen, the buttons and layout remain the same size (the default perspective camera is adjusted by infamous so that a size of 1 in Three.js corresponds to 1 CSS px). The buttons themselves are <button> DOM elements, while the shadows and lighting are WebGL (Three.js).

The way that I'm doing this (implementation) doesn't work well for complicated scenes with deep trees, because the deeper the tree, the more floating point error it introduces which will start to mis-align WebGL and DOM content. In that example it's fairly simple; minimal depth to the scene graph.

I'm thinking of how to improve this by extending Three.js core to take over the matrix math (TODO).

(WebGL in Infamous started with your webglfundamentals.org, btw. Thanks! 😃 )

greggman commented 5 years ago

It's been 5 years since this topic came up. Now that you've had experience with VR support were the system chooses the aspect and size for you as well as your ModelViewer where you let CSS dictate the size of viewer maybe you have some new thoughts on this?

greggman commented 3 years ago

I'm going to bring this up again because I noticed not only are all the samples broken ala (#19136) but they are all arguably wrong in other ways as well related to this issue. That wrongness keeps spreading all over the net as people copy the examples. Wouldn't it be nice to fix this for new users? (I'm happy to volunteer if we can get some agreement on the solution)

Most of the samples call render.setPixelRatio at initialization time but the device pixel ratio changes depending on the zoom level so really it should be called in resize as well. But, even that is problematic.

I'd like to try again to propose changing how three.js deals with this to make the code simpler for users and I'd like to suggest again getting rid of setPixelRatio. In fact it's not actually compatible with a working correct solution.

If you want to draw in device pixels, the only way to do it reliably in all cases is to use ResizeObserver and devicePixelContentBoxSize which will let the browser tell you exactly what size something was made. It returns the size in device pixels so there is no need or even room for devicePixelRatio or setPixelRatio.

Of course if you don't want to go all the way to device pixels then you're choosing some amount of scaling but if you do want device pixels, multiplying various browser values by devicePixelRatio will not get you the correct size.

I'd like to propose that most of the samples could be changed to this

Terse CSS version (can separate out body, html, and #c if you want)

html, body, #c {
  margin: 0;
  width: 100%;
  height: 100%;
  display: block;
}

...
<body>
  <canvas id="c"></canvas>
   ...
</body>
...

const canvas = document.querySelector( "#c" );
const renderer = new THREE.WebGLRenderer( { canvas } );

THREE.ResizeUtils.setResizeHandler( canvas, resize, { useDevicePixels: true } );

function resize( width, height ) {

  renderer.setSize( width, height );
  camera.aspect = canvas.clientWidth / canvas.clientHeight
  camera.updateProjectionMatrix();

}

Note, looking up the canvas instead of looking up the container is different than it's been but there is 1 less element and the 1 less code concept. To be concrete the current way there are 2 elements, a container and a canvas, and there are 3 steps (look up the container, look up the domElement, append the domElement to the container). The suggested way above is simplified cause there is only 1 element (the canvas) and 2 steps (looking up the canvas, passing it to the renderer). Less steps and less objects = simpler and isn't simple a goal of three.js?

As for the functions

There would be more setPixelRatio and it would print a deprecated warning. As stated above it doesnt' actually work. Also I've gone over why I think it's problematic in years old posts above. To restate, you often need to know the size of the canvas's drawing buffer but with setPixelRatio you don't really know, instead you guess here and there in the API whether or not you should pass in or use a CSS pixel size or a device pixel based size for picking, for render targets and effects passes, for screen space gl_FragCoord shaders, etc. Removing setPixelRatio removes that guessing. There is only 1 size, the size you asked for.
renderer.setSize would lose it's 3rd parameter and stop setting the CSS width and height on the a canvas. That is part of the source of the current issues. You can't know what size the browser will make the canvas ahead of time. Instead you have to let the browser choose a size and then ask it what it did. This is part of what's happening with #19136. Even though the current examples have the #19136 issue (and get an unwanted scrollbar as a result) to give a more concrete example, imagine you have a 999 device pixel wide window and dpr of 2.0 and you ask for two width:50% canvases/elements side by side. One of those 2 elements will have to be 499 device pixels, and the other 500 but there is no way to calculate which, you have to ask the browser what it did.
Adding THREE.ResizeUtils.setResizeHandler would allow three.js to more seamlessly handle the size for most users. (1) It's a step for removing the current issue of VR being different than non VR because three.js can pass the correct size to the handler. (2) It also means three can handle resizing in a central place for most users vs the current method where every example has its own resize code (3) It can handle more that just window resizing. Using ResizeObserver means for example the editor that has a canvas that changes size by dragging the divider between the toolbar and the 3d view just works using the setResizeHandler. Using the resize event it doesn't work because dragging the slider does not change size of the window.

Here is an example of an implementation for THREE.ResizeUtils.setResizeHandler

/**
  * @parameter {HTMLCanvasElement} canvas
  * @parameter {function(width, height)} handler that gets pass width and height
  * @parameter {object} options
  *      useDevicePixels: true/false  default false
  *      minDevicePixelRatio: default unset.
  *      maxDevicePixelRatio: default unset.
  * /
THREE.ResizeUtils.setResizeHandler = function( canvas, handler, options = {} ) {

  const useDevicePixels = options.useDevicePixels !== undefined ? options.useDevicePixels : false;
  const minDevicePixelRatio = options.minDevicePixelRatio;
  const maxDevicePixelRatio = options.maxDevicePixelRatio;

  function applyOptions( dimension ) {

    const effectiveDPR = useDevicePixels ? window.devicePixelRatio : 1;
    const effectiveMinDPR = minDevicePixelRatio !== undefined ? minDevicePixelRatio : 0;
    const effectiveMaxDPR = maxDevicePixelRatio !== undefined ? maxDevicePixelRatio : effectiveDPR;

    const dpr = Math.max( effectiveMinDPR, Math.min( effectiveMaxDPR, effectiveDPR ));
    return dimension * dpr / effectiveDPR;

  }

  function onResize( entries ) {

    for ( const entry of entries ) {

      let width;
      let height;
      let dpr = useDevicePixels ? window.devicePixelRatio : 1;

      if (useDevicePixels && entry.devicePixelContentBoxSize) {

        // NOTE: Only this path gives the correct answer
        // The other paths are an imperfect fallback
        // for browsers that don't provide anyway to do this

        width = entry.devicePixelContentBoxSize[ 0 ].inlineSize;
        height = entry.devicePixelContentBoxSize[ 0 ].blockSize;

        dpr = 1; // it's already in width and height

      } else if (entry.contentBoxSize) {

        if (entry.contentBoxSize[0]) {

          width = entry.contentBoxSize[ 0 ].inlineSize;
          height = entry.contentBoxSize[ 0 ].blockSize;

        } else {

          // legacy
          width = entry.contentBoxSize.inlineSize;
          height = entry.contentBoxSize.blockSize;

        }

      } else {

        // legacy
        width = entry.contentRect.width;
        height = entry.contentRect.height;

      }

      const displayWidth = applyOptions( width * dpr );
      const displayHeight = applyOptions( height * dpr );

      handler( displayWidth, displayHeight );

    }

  }

  const resizeObserver = new ResizeObserver( onResize );
  resizeObserver.observe( canvas, { box: "content-box" } );

}

Note that even though it says above that devicePixelContentBoxSize is the only correct answer it happens that the fallbacks appear to work in all current browsers for full window 100% width, 100% height canvases (although in Safari only on zoom = 100%). In other words it would work for all the examples that are fullscreen. It will get the wrong answer on browsers that don't support devicePixelContentBoxSize if those samples are in an iframe (like here) but there is no way to get the correct answer for those browser so it's no worse than current solutions.

Here's a working example

And a version not in an IDE. Since this is full window it should work in Firefox and Safari

To test you can generally use Ctrl+-, Ctrl++ (windows) or Command+-, Command++ (mac) to change the zoom level (vs having to go find machines with different DPRs)

Here's a non-full window example, it will only work in all cases in Chrome/Edge until Firefox and Safari implement the needed API but there is no solution for those browsers _{(well, short of trying to reverse engineer their layout algorithms and checking if Safari A, if Firefox B, ...)}

Hoping for some positive feedback @mrdoob 😀🤞

mrdoob commented 3 years ago

Gregg!

A few month ago I researched this topic and realised why it was hard for me to understand.

Turns out MacOS devices do not have fractional DPRs, either 1.0 or 2.0. I realised this when I got a Chromebook Go which is 1.25 DPR (and makes image-rendering: pixellated look horrible btw ☹️).

On top of that, when resizing windows MacOS, the OS skips fractional sizes. Because of that, I never got scrollbars because the innerWidth and innerHeight multiplied by devicePixelRatio always gave perfect numbers.

This breaks on Windows and ChromeOS because the OS can resize a windows to a fractional number... AND innerWidth and innerHeight are integers, so there is no way to get the actual size of the buffer.

You can read a more here: https://github.com/w3c/csswg-drafts/issues/5260#issuecomment-726044922

I talked about this with some people at Chrome but they seem to have a hard time to empathize with the problem. On the WebGL side, Ken also suggested devicePixelContentBoxSize, if only it had better browser support...

Having said all that... Yes, this needs to be resolved somehow.

Does ResizeObserver trigger synchronously? Or do we have to wait for the next frame?

API design wise, this ties nicely with something that has been bothering me.

For example:

const renderer = new THREE.WebGLRenderer();
renderer.setSize( 100, 100 );
renderer.setAnimationLoop( animation );
document.body.appendChild( renderer.domElement );

That last line is ugly. I would like to replace it with something like this:

const renderer = new THREE.WebGLRenderer();
renderer.setSize( 100, 100 );
renderer.setAnimationLoop( animation );
renderer.setDOMParent( document.body );

Makes the code much prettier and easier to read. (Suggestions for better names than setDOMParent very much welcome, I just made that up.)

But that would give us a chance to do what you're proposing, let the container guide the sizing. So we would end up with just this code:

const renderer = new THREE.WebGLRenderer();
renderer.setAnimationLoop( animation );
renderer.setDOMParent( document.body );

Not sure how to get rid of the aspect ratio parameter from the camera APIs without breaking anything though.

PS: We recently starting setting display:block by default to the canvas element: https://github.com/mrdoob/three.js/pull/20616

greggman commented 3 years ago

Turns out MacOS devices do not have fractional DPRs, either 1.0 or 2.0. I realised this when I got a Chromebook Go which is 1.25 DPR (and makes image-rendering: pixellated look horrible btw ☹️).

If you zoom in the browser (Cmd++, Cmd+-) you get fractional DRPs on MacOS. My eyes are going bad so I zoom alot more than I used to 😭

see here

Some thoughts on setDOMParent.

It's normal web dev to style elements like this

<style>
   .someClass {
      ...
   }
   #someId {
      ...
   }
</style>
<body>
    <canvas style="/* some local style */ border: 1px solid black;"></canvas>
    <canvas class="someClass"></canvas>
    <canvas id="someId"></canvas>
</body>

setDOMParent seems to fight that. I can style the container (making it more complicated, why did I need an extra container)? I can not use setDOMParent and then I can style the canvas in the normal way (so why have setDOMParent)? I can let the renderer create the canvas and then in JavaScript apply style, id, className but that still is less common so why have 2 ways?

It's normal to look up elements in JavaScript. A dev will easily learn how to look up an element. It's required for doing almost anything in the browser with JavaScript. So is it really helping the user to have three create the canvas vs just having them look it up? Basically they have to learn some non-standard way to do something vs just doing what they do everywhere else.

Does ResizeObserver trigger synchronously? Or do we have to wait for the next frame?

ResizeObserver is async. It has to be because only the compositor knows what size something will actually be made. See my previous comment about how only the compositor knows which of 2 elements will be made 499 device pixels and the other 500 device pixels if they are both set to 50% side by side on a 999 device pixel window.

mrdoob commented 3 years ago

If you zoom in the browser (Cmd++, Cmd+-) you get fractional DRPs on MacOS. My eyes are going bad so I zoom alot more than I used to 😭

I started using reading glasses this year... 😭

Hmm... Safari doesn't do that 🤔

setDOMParent seems to fight that. I can style the container (making it more complicated, why did I need an extra container)? I can not use setDOMParent and then I can style the canvas in the normal way (so why have setDOMParent)? I can let the renderer create the canvas and then in JavaScript apply style, id, className but that still is less common so why have 2 ways?

I think we discussed this already. CSS and browser engines are frustratingly hard to understand. Add browser engines discrepancies on top of it and also regressions. Every complexity I can hide I will.

We could add html, body { height: 100% } in examples/main.css and then add <canvas id="canvas" style="width:100%; height:100%"> in the examples, although I can already see users being confused that the canvas doesn't fill the screen vertically, because they're no aware of html, body { height: 100% }.

Does ResizeObserver trigger synchronously? Or do we have to wait for the next frame?

ResizeObserver is async. It has to be because only the compositor knows what size something will actually be made.

How would a project that just renders a single frame be? How would this code look like instead?

const renderer = new THREE.WebGLRenderer();
renderer.setPixelRatio( window.devicePixelRatio );
renderer.setSize( window.innerWidth, window.innerHeight );
document.body.appendChild( renderer.domElement );

renderer.render( scene, camera );

greggman commented 3 years ago

Every complexity I can hide I will.

Okay then, how about making the the Renderer construct do this

class WebGLRenderer {

  constructor( options ) {

    const canvas = document.createElement( 'canvas' );

    if ( ! options.dontDoAutomaticSetup ) {

      document.documentElement.style.height = '100%';

      if ( document.body ) {

        document.body.style.height = '100%';
        document.body.style.margin = '0';
        document.body.appendChild(canvas);

      } else {

        // there was no <body>
        document.documentElement.appendChild(canvas);

     }

      canvas.style.display = 'block';
      canvas.style.width = '100%';
      canvas.style.height = '100%';

    }

 }

}

Now you need no HTML or CSS whatsoever an HTML file with nothing but a <script> will work.

Make the camera default to auto computed aspect including changing its constructor to only have 3 values (fov, near, far). Almost none of the samples use anything but the display size of the canvas for aspect (or the display size of a rendertarget) but of which three already knows. So you'd be hiding a bunch of complexity.

Have three auto set the size of the canvas to its display size. Get rid of renderer.setSize (change the name since it's use would be the exception not the rule) and add in the minDPR and maxDPR settings as options to the constructor and you can handle that for them too. No needing to call setPixelRatio. So much complexity related to both sizing and DPR would be hidden.

Doing this would achieve your goal of

Every complexity I can hide I will.

your smallest example becomes this

import * as THREE from 'path/to/three.module.js';

const renderer = new THREE.WebGLRenderer();

renderer.render( scene, camera );

mattrossman commented 3 years ago

Whoa, this thread goes back a long way 😅

I'm just checking in to say that I would really appreciate an auto resize mode. In react-three-fiber, the <Canvas /> automatically fills the available space of the surrounding element, and I've never found a use case that this doesn't work for. I don't think they're doing anything too complicated in the logic for that, could we just follow the same recipe?

mrdoob commented 3 years ago

I still think this is my favourite approach:

const renderer = new THREE.WebGLRenderer();
renderer.setAnimationLoop( animation );
renderer.setDOMParent( document.body ); // Kind of like renderer.setContainer() too

The renderer would then "listen" the parent for changes and resize the canvas accordingly.

In this mode maybe the renderer could also ignore camera.aspect and compute it automatically accordingly too.

But we will end up with a useless property in the PerspectiveCamera constructor...

const camera = new THREE.PerspectiveCamera( 50 /*fov*/, 1 /*aspect*/, 1 /*near*/, 10 /*far*/ );

Or maybe it could be...

const camera = new THREE.PerspectiveCamera( 50, 'auto', 1, 10 );
const camera = new THREE.PerspectiveCamera( 50, null, 1, 10 );
...

🤔

Dmarcoux111 commented 2 years ago

One fun thing I ran into..... I was working on this split renderer with one side perspective, then the other orthographic and ray traced, and the images were always slightly off and I had no idea why. ( image 1 ). Turns out I wasn't taking the device pixel ratio into account, and naively sending window.innerWidth / 2 as my width. This caused an error when normalizing my coordinates because I was doing

vec2 uv = 2.0 * vec2( gl_FragCoord.x / ( resolution.x ), gl_FragCoord.y / resolution.y ) - 1.0;

When I used the suggestion I found here ( thanks guys )...

resolution: { type: 'v2', value: new THREE.Vector2( rightRenderer.domElement.width, rightRenderer.domElement.height ) },

Everything scales correctly and I get image 2

greggman commented 2 years ago

Bringing this up again because ....

AFAICT, most of the examples in https://threejs.org/examples have some issues with zoom. The normal pattern in most of the examples is

At Init time call renderer.setPixelRatio(devicePixelRatio)
On resize use renderer.setSize(window.innerWidth, window.innerHeight) and set the camera's aspect to window.innerWidth / window.innerHeight

The problem with this pattern is devicePixelRatio changes based on the zoom level, at least in Chrome and Firefox.

To show the issue

Go to https://threejs.org/examples/webgl_animation_keyframes.html
Press Ctrl/Cmd+Minus until the zoom level is 25%

you might see 2 issues here:
- the demo will now be rendering some enormously large canvas. On my 2019 MBP with the window maximized I get 6972x3924 which is then x2 because when I started the example dpr was 2 so it's rendering 13944 x 7869. Chrome is actually limiting that to 7677 x 4321 but the canvas is multisampled and samples is 4 so the page is actually dealing with 15354 x 8642 so it's slow. There's no reason to render 15354 x 8642 when there aren't that many pixels on my screen
- the image may move to the upper right. This is likely because three.js is assuming the size requested worked rather than using gl.drawingBufferWidth and gl.drawingBufferHeight in the appropriate places.
Refresh the page
Press Ctrl/Cmd+Plus until the zoom level is 100%

Notice the resolution is very low. This is because the dpr when the demo started is 0.5 so once we're back to 100% the resolution is a quarter what it would normally be

A simple solution is to call renderer.setPixelRatio in every demo every time renderer.setSize is called but I'd argue given Mr Doob's philosophy that anything that makes developers lives easier is the thing he'll chose, all this complication and leaving resizing as something the user has to deal with would be better solved by doing this in three.js itself based on CSS as suggested above.

mrdoob commented 2 years ago

@greggman Could you do a simple jsfiddle with plain webgl/js (without three.js) showing the code that will always return the pixel perfect values for every case? (browser resize, zoom change, fractional pixel ratios, cross browser, drawingBufferWidth, ...)

As far as I know, the only way of reliably achieving this is with devicePixelContentBox.

greggman commented 2 years ago

There is no code that will return pixel perfect values except devicePixelConentBox and unfortunately devicePixelContentBox is only on Chromium browsers. See this

That's not really the issue in https://github.com/mrdoob/three.js/issues/4903#issuecomment-1042099647

The issues are

handling the fact that devicePixelRatio can change at any time so you can't just set it at init time
using drawingBufferWidth and drawingBufferHeight in the appropriate way (few things I've written do this). If you handle issue 1 then this situation is rare
getting pixel perfect values for canvas size
deciding how much of these issues three.js should solve for the user rather than having the user have to solve it themselves

If three.js does decide to handle this for the user then deciding what that means.

Make it CSS based (three.js never setting any CSS and just asking the browser what size the user made it? My guess is the <model-viewer> does this.
Handling aspect ratio automatically for perspective cameras and possibly some different solution for orthographic cameras?
Handling dpr automatically but maybe letting the user set a min and max (so they can choose perf over resolution or visa versa)

mrdoob commented 2 years ago

There is no code that will return pixel perfect values except devicePixelContentBox and unfortunately devicePixelContentBox is only on Chromium browsers.

However, Safari doesn't do fractional dprs so we should be able to get pixel perfect by just doing element.clientWidth * devicePixelRatio.

So the blocker here is Firefox...

greggman commented 2 years ago

There is no code that will return pixel perfect values except devicePixelContentBox and unfortunately devicePixelContentBox is only on Chromium browsers.

However, Safari doesn't do fractional dprs so we should be able to get pixel perfect by just doing element.clientWidth * devicePixelRatio.

No, because Safari doesn't change it's dpr when the user zooms so if they zoom the dpr doesn't actually tell you how many device pixels per CSS pixel

mrdoob commented 2 years ago

I do not know what the solution for this is 😕

mrdoob / three.js

CSS based size by default? #4903

What do you think about setting the camera's aspect ratio automagically (see example above)?

What about auto-resizing the canvas to match its display size?