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

#4866 V texture coordinate inversion on OBJ loadModel #5017

Merged
merged 1 commit into from
Jan 29, 2021

Conversation

myselfhimself
Copy link
Contributor

Testing this as WebGL requires a flipped/inverted V texture coordinate

this is untested for now, unsure it resolves something

Testing this as WebGL requires a flipped/inverted V texture coordinate
@welcome
Copy link

welcome bot commented Jan 28, 2021

🎉 Thanks for opening this pull request! Please check out our contributing guidelines if you haven't already. And be sure to add yourself to the list of contributors on the readme page!

@myselfhimself
Copy link
Contributor Author

Tested, see #4866 (comment)
This copies Processing's UV's V dimension inverting behaviour on OBJ model loading. It seems this inverting had been forgotten in the p5.js implementation so far.

@myselfhimself
Copy link
Contributor Author

This could mergeable with some note to be added in a changelog and maybe a note on UV interpretation in the documentation...
Flipping the V dimension (because Y is flipped in WebGL or so), still does not solve all OBJ UV mapping issues, with sometimes images being flipped horizontally and/or vertically without plainly understanding why... If strict following for p5js of the Processing behaviour and API were not a must, maybe some helper flags or functions could allow developers to flip those coordinates easily. Eg. on loadModel(, , flipU boolean, flipV boolean)..

@stalgiag
Copy link
Contributor

@myselfhimself Thank you so much! I really like the idea of adding flipU and flipV. Perhaps they could be overload parameters in loadModel? We don't have any limitations as far as following Processing and I think this could be useful for quick fixes without needing to reexport model files. That can be discussed in another issue if you would like.

@all-contributors please add @myselfhimself for code

@stalgiag stalgiag merged commit c2c8992 into processing:main Jan 29, 2021
@stalgiag stalgiag mentioned this pull request Jan 29, 2021
13 tasks
@allcontributors
Copy link
Contributor

@stalgiag

I've put up a pull request to add @myselfhimself! 🎉

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.

2 participants