Conversation
juj
force-pushed
the
webgl2_part_2
branch
2 times, most recently
from
119faca to
c260605
Compare
src/library_gl.js
Outdated
There was a problem hiding this comment.
I want to move away the methods on GL, because they are not subject to dead code elimination. I'll have a followup to this pull request that moves more functions away from GL, especially those getters that might not necessarily even be called by some applications. That feels much cleaner to me since it straightforwardly uses the __deps architecture. The PR #3298 is similar in the idea (which still needs to be resolved somehow, but I don't yet know what would be the best way)
There was a problem hiding this comment.
I feel it would be cleaner to do it with entire objects, like a GL2 object with WebGL2 stuff, or if that isn't specific enough, then some sub-objects.
If even that isn't enough, individual methods are ok, but in that case it might be nicer to have them not begin with underscores, since we use that for compiled code (and js library code implementing a C interface).
There was a problem hiding this comment.
I added the underscore so that it gets expanded to a double underscore and a lowercase letter, i.e. __w..., because that conforms to the set of names reserved to the implementation in ISO C. See http://crasseux.com/books/ctutorial/Reserved-words-in-C.html :
"all identifiers regardless of use that begin with either two underscores or an underscore followed by a capital letter are reserved names. This is so that the library and header files can define functions, variables, and macros for internal purposes without risk of conflict with names in user programs."
Even though objects would be cleaner in some sense, I don't think they are technically as good, since it is quite easy to add these functions as global functions to get the __deps benefits. I think we should favor that whenever sensible. I recall you mentioned that closure would be able to DCE inside the objects, but since very few projects use closure and it's not favored any more for general use, we should not assume that. I am not looking to nuke the whole GL object, but just those getter functions that might not be needed by some programs. It does not feel like a big loss in cleanliness, but it can shave off a considerable number of lines practically for free?
There was a problem hiding this comment.
I feel like the ISO C standard is not what we want here, since these aren't C methods, nor even JS methods implementing C methods. It's just a normal JS method, so
function webGLValidateMapBufferTarget(arg) {
..
}
is the most JavaScript-ey.
There was a problem hiding this comment.
The idea ws to cater for C code that is being compiled. If user code happened to have a global function void webGLValidateMapBufferTarget(), that would get mangled to _webGLValidateMapBufferTarget and conflict with the JS side function. That's why I added the extra underscore. But sure, I can rename this, but then perhaps use emscriptenWebGLValidateMapBufferTarget or similar?
There was a problem hiding this comment.
I don't follow, why would it conflict if this method has no underscore, and the user code has a global function with that name, which would end up with an underscore? Not having an underscore is protection against such collisions?
Yes, emscripten prefix sounds good.
There was a problem hiding this comment.
Ok, updated the pull request to use emscripten prefix instead of underscore _. How does that look?
…tIndexed() to avoid code repetition.
…ntical to GL.get().
…- never write out to null pointer, but issue a GL error instead.
…ug in FULL_ES3 for later fix.
…on GL.getVertexAttrib() function.
…n implementation.
…by bound target, when it needs to be tracked by object name instead. Move GL.validateBufferTarget inside FULL_ES3 for dead code elimination.
|
Thanks, lgtm. |
2 participants
Not yet to merge, just marking this down. This shows currently all WebGL2_part_1 commits as well, I'll refresh this once #3757 is merged.