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

WebGL Code cleanup? #5

Open
greggman opened this issue Jul 26, 2017 · 2 comments
Open

WebGL Code cleanup? #5

greggman opened this issue Jul 26, 2017 · 2 comments

Comments

@greggman
Copy link

greggman commented Jul 26, 2017

There's a few, um, less than best practices in the WebGL code. I didn't look at the others

2 examples

  • looking up uniform and attribute locations every frame at render time instead of once at init time

  • assigning properties to WebGLObjects that could be null

There's also questionable issues like forcing 800x600. That might make sense on desktop PCs where you'd be hard pressed to find a display that's only 800x600 but it makes far less sense on a browser that might be viewed in a phone where the phone is trying to emulate 320x568 resolution.

Another is using devicePixelRatio. Does that happen on your c++ version? In other words if you open an 800x600 window are you getting 800x600 pixels or 800x600 * devicePixelRatio

Other random stuff

Setting the size of the canvas directly is kind of an anti-pattern. You should let CSS choose the size on the web.

Vertex Buffer Objects are available pretty much universally on WebGL

http://webglstats.com/webgl/extension/OES_vertex_array_object?platforms=0000dfffcfbfabfd01

In fact it's only IE and Edge that, not the actual hardware so if you want to use them you can either just use them and tell IE and Edge users they're S.O.L. or you can use polyfill that will just fill it in on IE and EDGE.

Would you be interested in a PR that deals with those issues?

@kosua20
Copy link
Owner

kosua20 commented Aug 1, 2017

First of all, thanks a lot for all this feedback!
I must apologize as I've realized that the WebGL version was in a worst state than I remembered...

As a learning experience I've tried to correct some of the aberrations that were present in my version (your remarks and your website were extremely useful, thank you again!). I've committed these changes, but I must admit that I have almost no experience with Javascript so some more general JS anti-patterns might remain in my code.
I have also removed the fixed resolution and the failed/unneeded attempt at handling HiDPI screens (in the C++ version I am using a fixed vertical rendering resolution), and now rely on CSS only.

Concerning VBO and extensions, my initial goal was to write a basic WebGL 1 version without any extensions as a kind of lowest common denominator. Maybe a WebGL 2 version with extensions would be different enough to live in its own webgl2 directory?

@greggman
Copy link
Author

greggman commented Aug 1, 2017

WebGL2 should probably be in another folder if you want to do that and you won't need extensions for vertex array objects. You can also do uniform buffer objects in WebGL2 but they are a little funky because there is no glMapBuffer in WebGL2.

My point with vertex array objects for WebGL1 is they're available on 99% of machines so for all intents and purposes they aren't are really an extension if that makes any sense. I'm only pointing that out if you wanted to try to keep the code similar since the C++ is using VAOs. Of course you can also leave it as is.

Thanks for the cool example. It's great to see so many versions.

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

2 participants