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

Redefine layer bounds to be standard OpenGL texture coords. #156

Closed
toji opened this issue Dec 1, 2016 · 3 comments
Closed

Redefine layer bounds to be standard OpenGL texture coords. #156

toji opened this issue Dec 1, 2016 · 3 comments
Milestone

Comments

@toji
Copy link
Member

toji commented Dec 1, 2016

In 1.1 the layer leftBounds and rightBounds are defined as a kind of strange combination of a UV and a width and height, defined in UV space. It also uses the order [left, top, width, height], which implies a flipped UV space from OpenGL. I've brought this up before and the consensus was generally "Don't break it if we don't have to", but since then I've been bitten by this system a few time and I think that Chrome's implementation has it "upside down" as a result. :P

I suggest that for clarity in 1.2 we redefine the layer bounds to explicitly be OpenGL UVs with the order [left, bottom, right, top]. (In OpenGL's UV space positive Y is up, so the bottom left corner is the origin.) Additionally we should define that bounds where left > right or bottom > top are invalid.

This means that instead of the default bounds being:

{
leftBounds: [0.0, 0.0, 0.5, 1.0],
rightBounds: [0.5, 0.0, 0.5, 1.0]
}

They become:

{
leftBounds: [0.0, 0.0, 0.5, 1.0],
rightBounds: [0.5, 0.0, 1.0, 1.0]
}

This feels more consistent with the rest of the platform to me (sticks to WebGL's conventions) and more predictable for developers. Any complaints?

@toji toji modified the milestone: 1.2 Dec 2, 2016
toji added a commit that referenced this issue Jan 7, 2017
Attempts to address #142, #107, #166, #156, and #125. Feedback welcome!
@mkeblx
Copy link
Contributor

mkeblx commented Jan 30, 2017

This seems quite unobjectionable (and none expressed), and minor. I don't see a reason for not redefining & closing, as we don't care about backwards compat too much and easily polyfillable.

@toji
Copy link
Member Author

toji commented Feb 3, 2017

Pushed this change as d6219a8. We're likely redefining how layers work soon, so it's somewhat moot, but I wanted to close the ticket down and ensure than change carried forward.

@toji toji closed this as completed Feb 3, 2017
@RafaelCintron
Copy link

LGTM after the fact.

@cwilso cwilso modified the milestones: Spec-Complete for 1.0, 1.0 Apr 30, 2019
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

No branches or pull requests

4 participants