KhronosGroup / OpenGL-Registry

OpenGL, OpenGL ES, and OpenGL ES-SC API and Extension Registry
678 stars 274 forks source link

Add enum groups for ANGLE use cases. #538

Closed null77 closed 1 year ago

null77 commented 2 years ago

This includes a few GLES 1.x enumes and a few others that we use in ANGLE.

See http://anglebug.com/6461 for context.

@pdaniell-nv @oddhack PTAL

SunSerega commented 2 years ago

Changes to these enums look right:

Changes to these are reasonable on their own, but shouldn't they also apply to alias enums?

And the reason for the change to GL_TEXTURE_CUBE_MAP_OES I could not understand, even with the repo which you linked for context.

null77 commented 2 years ago

@SunSerega thanks for the feedback.

Changes to these are reasonable on their own, but shouldn't they also apply to alias enums?

Done. I had omitted these because the style of the file seemed to be only modify what you need, but happy to do this little extra bit.

And the reason for the change to GL_TEXTURE_CUBE_MAP_OES I could not understand, even with the repo which you linked for context.

This one is special in GLES 1 because the parameter to the texture specification function is "coord". You can see by searching gl.xml for TextureCoordName.

SunSerega commented 2 years ago

Done

Missed GL_MAX_CUBE_MAP_TEXTURE_SIZE_OES.

(P.S. please don't force push, it's harder to compare locally...)

seemed to be only modify what you need

I think in this case, the reason is indifference to groups. These minor inconsistencies are mostly just annoying. But also it's not necessarily valid to mush together groups of all enums differing only in vendor postfix. What if the enum is defined only for a specific platform, and only for that platform is there a case where this enum is acceptable to some function? That might actually be the case with:

This one is special in GLES 1 because the parameter to the texture specification function is "coord"

I did previously (not thoroughly) check the TextureCoordName definition and uses and didn't find a case where GL_TEXTURE_CUBE_MAP_OES is accepted as coord.

Though now that I look closely at OES_texture_cube_map - it sais TEXTURE_GEN_STR_OES should be part of the TextureCoordName.

And GL_TEXTURE_CUBE_MAP_OES should only be in TextureTarget (which it already is).

null77 commented 2 years ago

(P.S. please don't force push, it's harder to compare locally...)

Noted. Is there a process to automatically merge all the WIP patches before landing? Having a small change be a stack of 11 patches is quite a mess.

Though now that I look closely at OES_texture_cube_map - it sais TEXTURE_GEN_STR_OES should be part of the TextureCoordName.

My bad - I was looking at the wrong text in the ext, somehow associating enums with the wrong usage paragraph (below instead of above). Fixed now.

oddhack commented 2 years ago

Noted. Is there a process to automatically merge all the WIP patches before landing?

I use the squash and merge option when accepting MRs, so I think yes, if I understand the question.

NogginBops commented 2 years ago

Looks good to me

pdaniell-nv commented 2 years ago

@oddhack this is approved to merge.

null77 commented 2 years ago

@oddhack friendly ping - is this ready to merge?