-
Notifications
You must be signed in to change notification settings - Fork 667
Reduce differences between the OpenGL 3 and OpenGL ES 2 demos #873
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
base: master
Are you sure you want to change the base?
Conversation
RobLoach
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good to have some consistancy.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I find some of those changes a bit confusing, but I suspect you wanted to keep the code the same, in all files, at all costs. Sadly, because we have to deal with so many demos, the review process is really hard, so apologies if I missed something obvious.
Overall, I think that merging this without feature detection implemented doesn't seem very useful. Right now, most of those additions is just a dead code.
However, just like I mentioned in #777 (comment) , I do agree that we need this.
| glGetProgramiv(dev->prog, GL_LINK_STATUS, &status); | ||
| assert(status == GL_TRUE); | ||
|
|
||
| /* TODO: Detect at runtime */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this unfinished? Are you planning to implement this in the force push?
(same for other files)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the current consensus regarding PR #777? Should I wait for that to be merged, or should this one be merged first and that one rebased on top? Or should the existing loader code from x11_opengl3 be adapted for the other demos instead of using GLAD?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the current consensus regarding PR #777? Should I wait for that to be merged
I don't believe you will get #777 merged any time soon. I would ignore it for now.
Or should the existing loader code from x11_opengl3 be adapted for the other demos instead of using GLAD?
I think this will be a way to go in the future. Right now, we have way too many duplicate code in the demos, so we need to deal with this problem before doing anything else.
@RobLoach (tactical ping)
The differences between the OpenGL 3 and OpenGL ES 2 demos are:
glMapBufferRangeis in OpenGL 3.0 or OpenGL ES 3.0 and could be used as an alternative, butglMapBufferis only available in OpenGL ES with extensions.