Skip to content

Please add ANGLE_instanced_arrays support to GLES2 wrapper #2510

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

Closed
floooh opened this issue Jul 9, 2014 · 12 comments · Fixed by #3471
Closed

Please add ANGLE_instanced_arrays support to GLES2 wrapper #2510

floooh opened this issue Jul 9, 2014 · 12 comments · Fixed by #3471
Assignees
Labels

Comments

@floooh
Copy link
Collaborator

floooh commented Jul 9, 2014

The GLES2 wrapper seems to be missing support for ANGLE_instanced_arrays:

  • the GLES2 headers are missing the following prototypes:
    glVertexAttribDivisorANGLE()
    glDrawElementsInstancedANGLE()
    glDrawArraysInstancedANGLE()
  • library_gl.js only seems to have support for the instancing functions in the desktop OpenGL wrapper

I tried copying the latest GL headers from here (http://www.khronos.org/registry/gles/api/GLES2/) to emscripten/system/include/GLES2, this worked for compilation, but the linker failed to find the ANGLE-postfixed GLES2 functions (since they are missing in library_gl.js).

Since WebGL is closer to GLES2 then Desktop GL I'd like to continue using the GLES2 headers and emulation wrapper for the emscripten port, which would require support for ANGLE_instanced_arrays though.

Thanks!
-Floh.

@kripken
Copy link
Member

kripken commented Jul 10, 2014

cc @juj

Sounds good, can you perhaps make a testcase for this?

@floooh
Copy link
Collaborator Author

floooh commented Jul 10, 2014

Yes, I'll try to derive a testcase from one of the existing tests, might take until the weekend until I get around this though.

Also, ANGLE_instanced_arrays isn't implemented in FF on Windows right now, which is strange because it works on OSX, and also on Chrome/Windows. There's a fairly old ticket here: https://bugzilla.mozilla.org/show_bug.cgi?id=843673

Would be good to poke the right people about this ;)

@kripken
Copy link
Member

kripken commented Jul 11, 2014

I see @jdashg assigned it to himself today, so the right people have certainly been poked ;)

@kdashg
Copy link
Contributor

kdashg commented Jul 11, 2014

The patch is ready to commit, so it should be in nightly by the weekend.
On Jul 10, 2014 5:44 PM, "Alon Zakai" [email protected] wrote:

I see @jdashg https://github.com/jdashg assigned it to himself today,
so the right people have certainly been poked ;)


Reply to this email directly or view it on GitHub
#2510 (comment).

@juj
Copy link
Collaborator

juj commented Jul 11, 2014

I can help here, or @floooh , did you already have a test in the works?

@floooh
Copy link
Collaborator Author

floooh commented Jul 11, 2014

Hi @juj I didn't get around to write a test yet, I was planning to cobble something together on the weekend, so if you're more familiar writing emscripten GL tests you're welcome to take over :)

I was thinking that a general instanced-arrays test would make sense, which can be switched between Core-GL, ARB_instanced_arrays and ANGLE_instanced_arrays with a define (I think the extension is in Core-GL since 3.1 or 3.2). The implementation would be the same, only the method names for glVertexAttribDivisor(), glDrawElementsInstanced() and glDrawArraysInstanced() would be different. I'm not sure whether other extensions or Core-GL has more DrawXXXInstanced() variants (I think ANGLE/ARB_instanced_arrays only adds those 3 methods).

@floooh
Copy link
Collaborator Author

floooh commented Jul 11, 2014

On further thought, I think handling ARB_instanced_arrays is not necessary, since the emscripten desktop GL wrapper already implements the Core-GL calls.

There's also another problem in the current Desktop-GL implementation (at least in my demo here: http://floooh.github.io/oryol/Instancing.html) with a runtime error because closure seems to remove some code (more details here: https://groups.google.com/forum/#!topic/emscripten-discuss/Lcm7RzIs7UY), you'll probably also stumble across this when running your test and use closure in the linker stage.

@AbigailBuccaneer
Copy link

Surely the GLES2 wrapper should expose EXT_instanced_arrays - the GLES2 extension - rather than ANGLE_instanced_arrays or ARB_instanced_arrays?

@floooh
Copy link
Collaborator Author

floooh commented Jul 12, 2014

The GLES extension registry lists both EXT_instanced_arrays and ANGLE_instanced_arrays (and a third NV_instanced_arrays). I think the reason that there is a separate ANGLE extension is that this has slight under-the-hood limitations because it needs to be compatible with D3D9. So the right way would be to expose both EXT_instanced_arrays and ANGLE_instanced_arrays I think.

I think we can skip the NV extension because this was very likely the blueprint for the EXT extension.

The official GLES2 extension header (http://www.khronos.org/registry/gles/api/GLES2/gl2ext.h) has all 3 instanced_arrays extensions.

@AbigailBuccaneer
Copy link

Ah, I didn't realise that ANGLE_instanced_arrays existed as a GLES extension as well as a WebGL extension. My bad. I guess if there are subtle differences or limitations then only the ANGLE extension should be exposed.

From a quick scan through, there are very minor differences: the ANGLE extension contains an extra limitation, as you mentioned:

INVALID_OPERATION is generated by DrawArraysInstancedANGLE or DrawElementsInstancedANGLE if there is not at least one enabled vertex attribute array that has a divisor of zero and is bound to an active generic attribute value in the program used for the draw command."

@stale
Copy link

stale bot commented Aug 31, 2019

This issue has been automatically marked as stale because there has been no activity in the past 2 years. It will be closed automatically if no further activity occurs in the next 7 days. Feel free to re-open at any time if this issue is still relevant.

@stale stale bot added the wontfix label Aug 31, 2019
@floooh
Copy link
Collaborator Author

floooh commented Aug 31, 2019

FWIW I think this is properly fixed for quite a while now. All the required ANGLE extension prototypes are in the GLES2/gl2ext.h header.

IMHO the ticket can be closed as fixed.

@stale stale bot removed the wontfix label Aug 31, 2019
@kripken kripken closed this as completed Sep 1, 2019
tlively added a commit to tlively/emscripten that referenced this issue Dec 11, 2019
This test unnecessarily checked for the presence of an explicit type
in a function definition in a .wast. The upcoming PR emscripten-core#2510 removes
this type from wast output, so this test needed to be updated first to
keep the rollers happy.
kripken pushed a commit that referenced this issue Dec 12, 2019
This test unnecessarily checked for the presence of an explicit type
in a function definition in a .wast. The upcoming PR #2510 removes
this type from wast output, so this test needed to be updated first to
keep the rollers happy.
belraquib pushed a commit to belraquib/emscripten that referenced this issue Dec 23, 2020
…-core#10011)

This test unnecessarily checked for the presence of an explicit type
in a function definition in a .wast. The upcoming PR emscripten-core#2510 removes
this type from wast output, so this test needed to be updated first to
keep the rollers happy.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants