KhronosGroup / OpenGL-Registry

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

Change type of 'severities' argument to enum in GetDebugMessageLogAMD #528

Closed NogginBops closed 2 years ago

NogginBops commented 2 years ago

Fixes #516

Not sure if I should add this change to the change log of the extension text. I also noticed now that the text later refers to GetDebugMessageLogAMD with the new signature, so the extension was inconsistent to begin with.

I'm also adding an extra commit where I change instances of enum to be GLenum, I can easily revert this if desired.

oddhack commented 2 years ago

(edit) Removed my previous comment. I think the enum -> GLenum change should be reverted before accepting this. The general style is to omit prefixes (although of course there are some extension specs that don't do this), and having a mix of prefixed and non-prefixed types in the same prototype is worse than either.

NogginBops commented 2 years ago

@oddhack reverted the enum to GLenum commit.

NogginBops commented 2 years ago

I can't figure out from your message if I should revert the changelog change too, but I'm guessing that there is something more as you said I should revert more than one thing. But I'm unsure of what I'm supposed to remove.

oddhack commented 2 years ago

I can't figure out from your message if I should revert the changelog change too, but I'm guessing that there is something more as you said I should revert more than one thing. But I'm unsure of what I'm supposed to remove.

LGTM. I should not have used "revert" in the first sentence, all I meant was I had previously signed off and then replaced that comment after realizing the GL prefix needed to be dropped.

oddhack commented 2 years ago

@pdaniell-nv I think this is OK per discussion on #516? Also, if @rpatlega is a WG member from AMD, please let me know so I can add them to the Github team and @-ping them.

pdaniell-nv commented 2 years ago

@pdaniell-nv I think this is OK per discussion on #516? Also, if @rpatlega is a WG member from AMD, please let me know so I can add them to the Github team and @-ping them.

Yes, this looks good to me. @rpatlega is an OpenGL WG member and often dials in. If he is happy this this change we should accept and merge.

rpatlega commented 2 years ago

From AMD side, we are good this for merge.

oddhack commented 2 years ago

From AMD side, we are good this for merge.

Thanks. I'll take that as Signed Off given @pdaniell-nv's comment. BTW I sent you an invite to the github team associated with the OpenGL repos.