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

WebGLRenderer: added getPhysicalSize() #11374

Merged
merged 2 commits into from
Jun 21, 2017
Merged

WebGLRenderer: added getPhysicalSize() #11374

merged 2 commits into from
Jun 21, 2017

Conversation

WestLangley
Copy link
Collaborator

Actually, this method could be renamed to getDrawingBufferSize(), as opposed to using the physical/logical nomenclature.

Also, I updated the docs. Those changes may be up for discussion, too.

This PR was posted in response to #10238 (comment).

@Mugen87
Copy link
Collaborator

Mugen87 commented May 26, 2017

I prefer .getDrawingBufferSize(). In this way, it's more clear what kind of data the method actually returns. At least for me 😊 . Docs are okay 👍 .

@WestLangley
Copy link
Collaborator Author

getDrawingBufferSize() is fine, but how does one describe what getSize() returns, then?

Does the renderer's "size" have any meaning that is useful?

@Mugen87
Copy link
Collaborator

Mugen87 commented May 28, 2017

Um, good question 🤔 ! Let me think about this 😊

@pailhead
Copy link
Contributor

pailhead commented Jun 6, 2017

What's the terminology used when dealing with css?

@Mugen87
Copy link
Collaborator

Mugen87 commented Jun 6, 2017

In the context of CSS and the devicePixelRatio, correct names would be:

.getPhysicalSize () and .getLogicalSize () instead of
.getDrawingBufferSize() and .getSize()

see https://stackoverflow.com/questions/8785643/what-exactly-is-device-pixel-ratio

But i'm not sure if this terminology is appropriate for a 3D library.

@pailhead
Copy link
Contributor

pailhead commented Jun 6, 2017

It's a 3d library that exclusively renders to an element that already has a nomenclature in place. Makes sense for this to be as close to the terminology used in canvas/dom as possible?

@mrdoob
Copy link
Owner

mrdoob commented Jun 20, 2017

.getDrawingBufferSize() kind of sound better to me too.

@@ -415,6 +415,15 @@ function WebGLRenderer( parameters ) {

};

this.getPhysicalSize = function () {

return {
Copy link
Owner

Choose a reason for hiding this comment

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

How about:

return {
    width: _width * _pixelRatio,
    height: _height * _pixelRatio
};

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

OK.

@WestLangley
Copy link
Collaborator Author

.getDrawingBufferSize() kind of sound better to me too.

Done!

@mrdoob mrdoob merged commit f373dbd into mrdoob:dev Jun 21, 2017
@mrdoob
Copy link
Owner

mrdoob commented Jun 21, 2017

Many thanks!

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.

4 participants