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

should renderer.setSize be responsible for setting the style of the canvas element? #2969

Closed
0xor1 opened this issue Jan 23, 2013 · 7 comments

Comments

@0xor1
Copy link

0xor1 commented Jan 23, 2013

I was using an old version of three.js and since migrating to r55 my canvas now doesn't resize as it should, because when I call

renderer.setSize(width, height);



this.setSize = function ( width, height ) {     

    _canvas.width = width * this.devicePixelRatio;      
    _canvas.height = height * this.devicePixelRatio;        

    _canvas.style.width = width + 'px';     
    _canvas.style.height = height + 'px';       

    this.setViewport( 0, 0, _canvas.width, _canvas.height );    

};

2feae0ee overwrite my css stylesheet settings that are on the canvas for it to be 100% for both width and height. I was just wondering whether the renderer should just be responsible for setting the actual canvas height and width and not be concerned with the canvas.style.height / width.

I have been able to fix this within my application I'm just not sure I'm convinced that the renderer.setSize() method is the best place to be updating the actual canvas style dimensions.

@mrdoob
Copy link
Owner

mrdoob commented Jan 23, 2013

Oh. Well, this code was done for supporting hdpi resolutions automatically. Which is something you don't want to think about in the realm of phones and tablets. But I can see the side effect...

@gero3
Copy link
Contributor

gero3 commented Jan 23, 2013

Maybe an extra argument that when undefined, it sets the canvas style.
When true, it sets the canvas style.
When false, it doesn't set the canvas style

renderer.setSize(width, height, false);

@0xor1
Copy link
Author

0xor1 commented Jan 23, 2013

I'm not completely opposed to the idea.

I prefer to style my canvas dimensions with percentage so it resizes the canvas nice and smoothly, I then make the call to renderer.setSize() based on a timeout so it isn't being continually called as the user might be manually resizing the screen.

I guess I could just take the timeout and let setSize get called every time the window resize event is fired but that seems a bit excessive.

@0xor1
Copy link
Author

0xor1 commented Jan 23, 2013

could go for:

this.setSize = function ( width, height ) {     

    var csw = _canvas.style.width
        , csh = _canvas.style.height
        ;

    _canvas.width = width * this.devicePixelRatio;      
    _canvas.height = height * this.devicePixelRatio;        

    this.setViewport( 0, 0, _canvas.width, _canvas.height );

    if( csw.substring(csw.length-2) === 'px' ) {
        _canvas.style.width = width + 'px';
    }   
    if( csh.substring(csh.length-2) === 'px' ) { 
        _canvas.style.height = height + 'px';
    }          

};

@mrdoob
Copy link
Owner

mrdoob commented Jan 23, 2013

That's way too smart.
@gero3 suggestion is better. Non-smart solutions tend to bring less problems.

@0xor1
Copy link
Author

0xor1 commented Jan 24, 2013

@mrdoob I don't think I would call it smart, but OK.

gero3 added a commit to gero3/three.js that referenced this issue Jan 28, 2013
@mrdoob mrdoob closed this as completed Apr 14, 2013
@Nesandu
Copy link

Nesandu commented Jun 28, 2020

Maybe an extra argument that when undefined, it sets the canvas style.
When true, it sets the canvas style.
When false, it doesn't set the canvas style

renderer.setSize(width, height, false);

thank you

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants