Skip to content

Conversation

@WestLangley
Copy link
Collaborator

Fixes #12210.

When specifying the viewport and the scissor box, (x, y) is the coordinate of the lower left corner of the rectangular region.

https://developer.mozilla.org/en-US/docs/Web/API/WebGLRenderingContext/viewport

@mrdoob mrdoob added this to the r93 milestone Apr 18, 2018
@mrdoob mrdoob modified the milestones: r93, r94 May 22, 2018
@mrdoob mrdoob modified the milestones: r94, r95 Jun 27, 2018
@mrdoob mrdoob modified the milestones: r95, r96 Jul 31, 2018
@mrdoob mrdoob modified the milestones: r96, r97 Aug 29, 2018
@mrdoob mrdoob modified the milestones: r97, r98 Sep 25, 2018
@WestLangley
Copy link
Collaborator Author

WestLangley commented Oct 3, 2018

Now, .setViewport() follows the webGL convention, so the viewport transform, which converts from NDC space to window space, is correct when applied to NDC coordinates.

Also, .setViewport() now follows the same convention as WebGLRenderTarget.viewport.set().


Aside: .getCurrentViewport() should correctly return the viewport previously set by .setViewport(). If it doesn't, that is a separate issue, unrelated to this PR.

@Mugen87
Copy link
Collaborator

Mugen87 commented Oct 3, 2018

Yeah, I also think it's better to stick to the WebGL conventions.

@mrdoob mrdoob modified the milestones: r98, r99 Oct 31, 2018
@mrdoob mrdoob modified the milestones: r99, r100 Nov 29, 2018
@WestLangley
Copy link
Collaborator Author

I have other PRs in the pipeline that depend on this correction.

/ping @Mugen87 I don't know what else I can do.

@Mugen87
Copy link
Collaborator

Mugen87 commented Dec 7, 2018

I'm fine with this change 😇

@mrdoob mrdoob modified the milestones: r100, r101 Dec 28, 2018
@mrdoob mrdoob modified the milestones: r101, r102 Jan 31, 2019
@WestLangley WestLangley changed the title WebGLRenderer: Revert y swap to setViewport() and setScissor() Make WebGLRenderer.setViewport(), setScissor() relative to lower-left corner Feb 11, 2019
@WestLangley WestLangley merged commit 280a1f6 into mrdoob:dev Feb 13, 2019
@mrdoob
Copy link
Owner

mrdoob commented Feb 13, 2019

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.

3 participants