Created attachment 309302 [details] [review]
Patch

Does this actually cause an issue for you or is it just a cosmetic fix?

It doesn't cause an issue for me right now (because double is typedef'd to GLdouble in the current gl.h), but it's also more than cosmetic. The opengl types were defined so that we don't have issues with types sizes. double is not guaranteed to be any particular size, so by using GLdouble we can keep portability across compilers even if the particular definition of GLdouble in gl.h changes in the future. We're already using opengl types for everything else, so it's also better to be self-consistent.

That said, Sebastian Dröge noticed that GLclampd was removed in later opengl versions, so I think we should probably be using GLdouble instead of GLclampd.

GLclampd only seems to be removed from GLES, not sure if it was also removed from desktop GL in later versions.


double is defined to be a 64 bit IEEE754 floating point number

It appears that GLclampd was entirely removed (in favor of GLdouble) starting with opengl 4.2:
https://www.opengl.org/sdk/docs/man/html/removedTypes.xhtml

AFAICT, double is not guaranteed to be 64 bit or IEEE754; it just has to be a floating point representation of some kind. In contrast, GLdouble is guaranteed to be 64 bit IEEE754. On most (all? not sure) machines and compilers, they end up being the same, but in principle, they don't have to be.

Opengl reference:
https://www.opengl.org/wiki/OpenGL_Type

C reference (I'm sure we could find the actual standard, but this is quick and handy):
https://en.wikipedia.org/wiki/C_data_types
"Actual properties unspecified (except minimum limits), however on most systems this is the IEEE 754 double-precision binary floating-point format. This format is required by the optional Annex F "IEC 60559 floating-point arithmetic."

"The actual size and behavior of floating-point types also vary by implementation. The only guarantee is that long double is not smaller than double, which is not smaller than float."

https://en.wikipedia.org/wiki/Double-precision_floating-point_format disagrees.  They are both guaranteed to be 64-bits.  That being said, I'm fine with the change to GLdouble for the GL prototype/s.

Thanks; I'll send a patch on Monday to convert double --> GLdouble in the prototypes.

Actually GLdouble is not defined by GLES at all so that will not work either without a typedef for GLES.

I'm a little confused. Looking at glprototypes/opengl.h, it appears that all these functions are defined only for normal opengl anyway. That said, I'm still pretty new to this codebase; am I missing something?

glprototypes/opengl.h:

/* These are the core GL functions which are only available in big
   GL */
GST_GL_EXT_BEGIN (only_in_big_gl,
                  GST_GL_API_OPENGL | GST_GL_API_OPENGL3,
                  1, 0,
                  255, 255, /* not in GLES */
                  "\0",
                  "\0")

[snip]

GST_GL_EXT_FUNCTION (void, ClearDepth,

[snip]

GST_GL_EXT_END ()

All functions are defined for all GL api's GLES or otherwise as the function table is public API/ABI.  Compiling with different symbols in the vfunc table would break applications built against versions of the library targetting a different GL api set.

I understand that part. What confuses me is that these functions are Opengl-specific section, with comments indicating that they're not in GLES. There are other such functions in the section that are Opengl-specific, such as GetTexImage.

If all these functions are going to have NULL vfunc table entries in GLES contexts anyway, why does the prototype (double/GLdouble) matter? Am I misunderstanding the way the contexts are handled?

It matters because GLdouble will not exist with GLES only systems and using it unconditionally will break compilation there.

Yeah, that makes sense. I agree with you then that raw double is the only solution unless we add a typedef for GLES. Any opinion between the two?

Matthew?

I don't really mind which way, staying with raw double means no code has to be changed.  If someone really wants to go to the effort of changing these to GLdouble, then by all means, go ahead.

Let's close it for now then.

If anyone feels like making a patch, please re-open or file a new bug, thanks!