The current implementation of ARB_get_program_binary is incomplete and currently violates the spec. Specifically these are the major issues: * glGet with GL_NUM_PROGRAM_BINARY_FORMATS is not implemented, it should return a at least 1. * glGet with GL_PROGRAM_BINARY_FORMATS is not implemented, it should return a list with a single binaryFormat. * glGetProgramBinary succeeds with a binary length of zero, and also does not set binaryFormat to a valid value, it should always succeed and set binaryFormat to the value returned by GL_PROGRAM_BINARY_FORMATS. * glProgramBinary always fails with GL_INVALID_OPERATION despite program being valid, it should always succeed if binaryFormat is correct, and return GL_INVALID_ENUM if not. Additionally it should always set GL_LINK_STATUS to GL_FALSE to indicate the program will need to load source anyway. If these are not to be implemented ARB_program_binary and GL 4.1+ support should not be advertised by Mesa.
(In reply to Leith Bade from comment #0) > The current implementation of ARB_get_program_binary is incomplete and > currently violates the spec. > > Specifically these are the major issues: > * glGet with GL_NUM_PROGRAM_BINARY_FORMATS is not implemented, it should > return a at least 1. The ARB_get_program_binary spec says: Get Value Type Get Command Minimum Value Description Section ------------- ---- ----------- ------------- ----------- ------- PROGRAM_BINARY_FORMATS 0* x Z GetIntegerv N/A Enumerated program binary formats 2.14.3 NUM_PROGRAM_BINARY_FORMATS Z GetIntegerv 0 Number of program binary formats 2.14.3 The Khronos conformance tests check for this and behave appropriately. Your code should also check the number of available formats and behave appropriately. Like ARB_texture_compression and ARB_multisample, this is an "ask if you can ask" extension. You first have to ask if the extension is there, then you ask whether you can do anything with it. > * glGet with GL_PROGRAM_BINARY_FORMATS is not implemented, it should return > a list with a single binaryFormat. Given the above, it should return zero formats. > * glGetProgramBinary succeeds with a binary length of zero, and also does > not set binaryFormat to a valid value, it should always succeed and set > binaryFormat to the value returned by GL_PROGRAM_BINARY_FORMATS. Hmm... the spec doesn't really provide any guidance here, and I think that's why it's implemented in this way. Since there is no possible binary format, there are zero bytes of binary data. Nothing in the extension spec or later OpenGL specs give any guidance for an error to be generated in this scenario. There are no binary formats, so there is no value to write to binaryFormat. It's really not clear what the correct behavior is... other than not call this function if there are no binary formats. :) > * glProgramBinary always fails with GL_INVALID_OPERATION despite program > being valid, it should always succeed if binaryFormat is correct, and return > GL_INVALID_ENUM if not. Additionally it should always set GL_LINK_STATUS to > GL_FALSE to indicate the program will need to load source anyway. You are correct. It should set link status to false and generate GL_INVALID_ENUM. > If these are not to be implemented ARB_program_binary and GL 4.1+ support > should not be advertised by Mesa. FWIW, no driver in Mesa advertises a GL version higher than 3.3. Note that this is also part of OpenGL ES 3.0, and at least the i965 driver has passed OpenGL ES 3.0 conformance testing. The behavior of GL_NUM_PROGRAM_BINARY_FORMATS and GL_PROGRAM_BINARY_FORMATS may be annoying, but it is, strictly speaking, correct.
Yeah the spec is a bit ambiguous. It is almost as if they can't see why anyone would implement ARB_get_program_binary without supporting at least one binaryFormat. Of course they then made it core, so IDK. OK I will add a check for GL_NUM_PROGRAM_BINARY_FORMATS on my code. But the GL_INVALID_OPERATION is rather misleading.
I sent some patches to the mesa-dev mailing list: http://lists.freedesktop.org/archives/mesa-dev/2014-December/073210.html http://lists.freedesktop.org/archives/mesa-dev/2014-December/073212.html http://lists.freedesktop.org/archives/mesa-dev/2014-December/073211.html The last one I'm a little iffy about.
Looks good. Are you able to send feedback upstream to the GL ARB about the vagueness around having no support binaryFormats in the spec?
Fixed by the following commits on Mesa master. Once there is some feedback from Khronos on the spec bug, I'll post some updates. commit f591712efeb9a757379d1e89907e2147749aaf6c Author: Ian Romanick <ian.d.romanick@intel.com> Date: Sun Dec 21 12:06:23 2014 -0800 mesa: Always generate GL_INVALID_OPERATION in _mesa_GetProgramBinary There are no binary formats supported, so what are you doing? At least this gives the application developer some feedback about what's going on. The spec gives no guidance about what to do in this scenario. Signed-off-by: Ian Romanick <ian.d.romanick@intel.com> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=87516 Reviewed-by: Kenneth Graunke <kenneth@whitecape.org> Acked-by: Leight Bade <leith@mapbox.com> commit 4fd8b3012371a5795a0d272928266c6237e57466 Author: Ian Romanick <ian.d.romanick@intel.com> Date: Sun Dec 21 12:03:57 2014 -0800 mesa: Ensure that length is set to zero in _mesa_GetProgramBinary v2: Fix assignment of length. Noticed by Julien Cristau. Signed-off-by: Ian Romanick <ian.d.romanick@intel.com> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=87516 Reviewed-by: Kenneth Graunke <kenneth@whitecape.org> Acked-by: Leight Bade <leith@mapbox.com> commit 201b9c181825551559f6d995007de8ff12d1a54c Author: Ian Romanick <ian.d.romanick@intel.com> Date: Sun Dec 21 12:03:09 2014 -0800 mesa: Add missing error checks in _mesa_ProgramBinary Signed-off-by: Ian Romanick <ian.d.romanick@intel.com> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=87516 Reviewed-by: Kenneth Graunke <kenneth@whitecape.org> Acked-by: Leight Bade <leith@mapbox.com>
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.