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
Use the WebGL 1.0 specific constant for the half float format. #8242
Conversation
@@ -159,6 +159,7 @@ | |||
#if defined GL_ES_VERSION_2_0 | |||
#undef GL_ARRAY_BUFFER_ARB | |||
#undef GL_ELEMENT_ARRAY_BUFFER_ARB | |||
#define GL_HALF_FLOAT_OES 0x8D61 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In fact and from my view, for WebGL we should use header files from ES 2.0/3.0, because it is based on it. But someone decided to use files from desktop GL. That's strange. And there is no GL_HALF_FLOAT_OES in desktop GL. So I added this constant manually as #define.
PS If the desktop include above is replaced by include gl2ext.h, for example, the webgl renderer stops working and pulls a huge amount of patching and testing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you should put this into graphics_opengl_defines.h and follow the style of the other defines
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay 🙌🏻 Done.
I made a build of the engine myself and tested half float texture on WebGL 1.0 device - works great. The only thing for WebGL 1.0 is that half float texture must be POT. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this requires POT textures, we should probably output an error if someone is trying to use a wrong combination, otherwise we might just get a black screen and no info on why it is happening
@@ -159,6 +159,7 @@ | |||
#if defined GL_ES_VERSION_2_0 | |||
#undef GL_ARRAY_BUFFER_ARB | |||
#undef GL_ELEMENT_ARRAY_BUFFER_ARB | |||
#define GL_HALF_FLOAT_OES 0x8D61 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you should put this into graphics_opengl_defines.h and follow the style of the other defines
Ok 🙌🏻 I think it should be a new issue and a new PR (like "The engine should warn that the texture should be POT"). |
Looks to me like everything is ok now and you can do the merge. |
WebGL 1.0 supports loading half float textures. But it requires the use of HALF_FLOAT_OES constant, which has a different value from the HALF_FLOAT constant from WebGL 2.0 / OpenGL ES 3.0. This fix enables to use half float texture format on WebGL 1.0 devices.
Fixes #8241
PR checklist