Page MenuHomeWildfire Games

Make GLES mode work on Mesa
ClosedPublic

Authored by linkmauve on Nov 30 2019, 3:20 PM.

Details

Summary

This fixes various GLES 2.0 issues that prevented 0ad from building on Mesa, and from running on Intel’s Iris driver.

Test Plan
  • Gather as many GLES 2.0-compatible computers.
  • Build 0ad for them.
  • Run 0ad and check it works fine.

Event Timeline

linkmauve created this revision.Nov 30 2019, 3:20 PM
Owners added a subscriber: Restricted Owners Package.Nov 30 2019, 3:20 PM

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/661/display/redirect

Successful build - Chance fights ever on the side of the prudent.

Linter detected issues:
Executing section Source...

source/lib/external_libraries/glext_funcs.h
|   1| /*·Copyright·(C)·2013·Wildfire·Games.
|    | [NORMAL] LicenseYearBear:
|    | License should have "2019" year instead of "2013"
Executing section JS...
Executing section cli...

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/1177/display/redirect

JoshuaJB accepted this revision.Dec 1 2019, 11:36 PM
JoshuaJB added a subscriber: JoshuaJB.

Thanks you so much for taking a look at this! Strong GL ES support has been on my wishlist for a long time. If the minimap still doesn't support GL ES (it didn't last I looked), I have a patch from years that could be salvaged rather than starting from scratch.

source/ps/GameSetup/HWDetect.cpp
715

What exactly is there TODO about this?

This revision is now accepted and ready to land.Dec 1 2019, 11:36 PM

The minimap looks correct atm, the entire game looks like it only needs some polish before it looks identical to the GL version, on Intel. :)

linkmauve added inline comments.Dec 14 2019, 3:34 PM
source/ps/GameSetup/HWDetect.cpp
715

It’s because this code currently assumes that if the X11 video driver is enabled in SDL2, the user is using X11 and can query GLX functions.

There are two cases where this breaks:

  • When SDL2 is using EGL as the GL context creation API on X11, which is common on embedded platforms which may not even support GLX.
  • When the user is using Wayland as the SDL2 video backend, despite X11 being also enabled, in this case using GLX would likely lead to a runtime crash or build failure.

For now I disabled it when CONFIG2_GLES is enabled because SDL2 defaults to EGL in this case, but it would be useful to also support it on desktop GL.

linkmauve added inline comments.Dec 14 2019, 3:39 PM
source/lib/external_libraries/glext_funcs.h
134

This one also became core in OpenGL ES 3.2, in addition to this extension for 2.0+, so we may want to discover it at some point.

vladislavbelov added inline comments.
source/renderer/Renderer.cpp
438

That was missed in rP22610.

source/renderer/SkyManager.cpp
129

Do we actually need alpha here?

Stan added a subscriber: Stan.Dec 21 2019, 12:17 PM
Stan added inline comments.
source/renderer/SkyManager.cpp
129

We do not. Skybox textures shouldn't be transparent. Else I guess it would cause visual artifacts.

The only case where they might need to be is if there are multiple (Ex mountains and stuff) over sky. That's currently not possible.

linkmauve added inline comments.Dec 21 2019, 2:54 PM
source/renderer/SkyManager.cpp
129

But all the files currently have an alpha channel, so unless we do a costly CPU-side conversion from RGBA8888 to RGB888 (for the driver to then do it in the other direction most likely, to get pixels aligned to 32-bit boundaries) we can’t avoid using RGBA here.

Another solution would be to convert all textures from RGBA to RGB but they are already compressed so that’d lose even more quality.

Only need to figure why it crashes for @vladislavbelov and @Freagarach

source/renderer/SkyManager.cpp
129

I guess it's fine then.

linkmauve updated this revision to Diff 10702.Dec 21 2019, 4:00 PM

Change how preferglsl is forced to true.

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/833/display/redirect

Successful build - Chance fights ever on the side of the prudent.

Linter detected issues:
Executing section Source...

source/lib/external_libraries/glext_funcs.h
|   1| /*·Copyright·(C)·2013·Wildfire·Games.
|    | [NORMAL] LicenseYearBear:
|    | License should have "2019" year instead of "2013"
Executing section JS...
Executing section cli...

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/1349/display/redirect

Stan added a comment.Dec 28 2019, 2:46 PM

First attempt

In file included from /usr/include/GL/gl.h:2055,
                 from /usr/include/GL/glx.h:32,
                 from ../../../source/ps/GameSetup/HWDetect.cpp:48:
/usr/include/GL/glext.h:778:21: error: conflicting declaration of C function ‘void glShaderSource(GLuint, GLsizei, const GLchar* const*, const GLint*)’
 GLAPI void APIENTRY glShaderSource (GLuint shader, GLsizei count, const GLchar *const*string, const GLint *length);
                     ^~~~~~~~~~~~~~
In file included from ../../../source/lib/external_libraries/opengl.h:40,
                 from ../../../source/lib/ogl.h:30,
                 from ../../../source/ps/GameSetup/HWDetect.cpp:22:
/opt/vc/include/GLES2/gl2.h:600:37: note: previous declaration ‘void glShaderSource(GLuint, GLsizei, const GLchar**, const GLint*)’
 GL_APICALL void         GL_APIENTRY glShaderSource (GLuint shader, GLsizei count, const GLchar** string, const GLint* length);
                                     ^~~~~~~~~~~~~~
make[1]: *** [engine.make:324: obj/engine_Release/HWDetect.o] Error 1
make[1]: *** Attente des tâches non terminées....
../../../source/ps/GameSetup/GameSetup.cpp:298:2: warning: #warning TODO: implement cursors for GLES [-Wcpp]
 #warning TODO: implement cursors for GLES
  ^~~~~~~
../../../source/ps/GameSetup/GameSetup.cpp:320:2: warning: #warning TODO: implement cursors for GLES [-Wcpp]
 #warning TODO: implement cursors for GLES
  ^~~~~~~
make: *** [Makefile:111: engine] Error 2
Stan added a comment.EditedDec 28 2019, 3:56 PM

Wait the error makes no sense cause you have a fix for it... (The one in HWDetect.cpp)

Stan requested changes to this revision.EditedDec 28 2019, 4:40 PM

I assume it worked for you cause you are in wayland.

source/ps/GameSetup/HWDetect.cpp
47

#if defined(SDL_VIDEO_DRIVER_X11) && !CONFIG2_GLES

This revision now requires changes to proceed.Dec 28 2019, 4:40 PM
Stan added a comment.EditedDec 30 2019, 1:13 PM

According to the guys on #dri-devel and https://gitlab.freedesktop.org/mesa/mesa/blob/master/src/gallium/drivers/vc4/vc4_formats.c#L73 The raspberry pi does not support S3TC... (In GLES2.0) mode So while this still needs the change above, I cannot test it further...

Stan added a comment.Jan 3 2020, 1:39 PM

Works on RPI4 removing libraspberrypi-dev (Which introduce headers not supporting DXT5 and DXT3) but it's not related. Only thing missing is my fix.

linkmauve updated this revision to Diff 11054.Jan 16 2020, 7:14 PM

Fix a missing #if guard for systems which don’t have a GL/glx.h header.

Stan accepted this revision.Jan 16 2020, 7:16 PM

Works for me on RPI4 with the mesa lib.

This revision is now accepted and ready to land.Jan 16 2020, 7:16 PM

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/1064/display/redirect

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/macos-differential/160/display/redirect

The rest of the patch looks good for me.

source/ps/GameSetup/Config.cpp
55 ↗(On Diff #11054)

Why not in hwdetect.js?

109 ↗(On Diff #11054)

It's not correct since we read/write this config value in multiple places.

source/ps/GameSetup/HWDetect.cpp
715

I think it should be the same comment as above.

Successful build - Chance fights ever on the side of the prudent.

Linter detected issues:
Executing section Source...

source/ps/GameSetup/Config.cpp
|   1| /*·Copyright·(C)·2019·Wildfire·Games.
|    | [NORMAL] LicenseYearBear:
|    | License should have "2020" year instead of "2019"

source/renderer/SkyManager.cpp
|   1| /*·Copyright·(C)·2019·Wildfire·Games.
|    | [NORMAL] LicenseYearBear:
|    | License should have "2020" year instead of "2019"

source/gui/ObjectTypes/CChart.cpp
|   1| /*·Copyright·(C)·2019·Wildfire·Games.
|    | [NORMAL] LicenseYearBear:
|    | License should have "2020" year instead of "2019"

source/lib/external_libraries/glext_funcs.h
|   1| /*·Copyright·(C)·2013·Wildfire·Games.
|    | [NORMAL] LicenseYearBear:
|    | License should have "2020" year instead of "2013"

source/ps/GameSetup/HWDetect.cpp
|   1| /*·Copyright·(C)·2019·Wildfire·Games.
|    | [NORMAL] LicenseYearBear:
|    | License should have "2020" year instead of "2019"
Executing section JS...
Executing section cli...

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/1582/display/redirect

linkmauve updated this revision to Diff 11061.Jan 17 2020, 9:37 AM

Fix Vladislav’s comments.

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/macos-differential/164/display/redirect

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/1068/display/redirect

Successful build - Chance fights ever on the side of the prudent.

Linter detected issues:
Executing section Source...

source/renderer/SkyManager.cpp
|   1| /*·Copyright·(C)·2019·Wildfire·Games.
|    | [NORMAL] LicenseYearBear:
|    | License should have "2020" year instead of "2019"

source/gui/ObjectTypes/CChart.cpp
|   1| /*·Copyright·(C)·2019·Wildfire·Games.
|    | [NORMAL] LicenseYearBear:
|    | License should have "2020" year instead of "2019"

source/lib/external_libraries/glext_funcs.h
|   1| /*·Copyright·(C)·2013·Wildfire·Games.
|    | [NORMAL] LicenseYearBear:
|    | License should have "2020" year instead of "2013"

source/ps/GameSetup/HWDetect.cpp
|   1| /*·Copyright·(C)·2019·Wildfire·Games.
|    | [NORMAL] LicenseYearBear:
|    | License should have "2020" year instead of "2019"
Executing section JS...
|    | [NORMAL] ESLintBear (spaced-comment):
|    | Expected space or tab after '//' in comment.
|----|    | /zpool0/trunk/binaries/data/mods/mod/hwdetect/hwdetect.js
|    |++++| /zpool0/trunk/binaries/data/mods/mod/hwdetect/hwdetect.js
| 340| 340| 
| 341| 341| global.RunHardwareDetection = function(settings)
| 342| 342| {
| 343|    |-	//print(JSON.stringify(settings, null, 1)+"\n");
|    | 343|+	// print(JSON.stringify(settings, null, 1)+"\n");
| 344| 344| 
| 345| 345| 	var output = RunDetection(settings);
| 346| 346| 
|    | [NORMAL] ESLintBear (spaced-comment):
|    | Expected space or tab after '//' in comment.
|----|    | /zpool0/trunk/binaries/data/mods/mod/hwdetect/hwdetect.js
|    |++++| /zpool0/trunk/binaries/data/mods/mod/hwdetect/hwdetect.js
| 344| 344| 
| 345| 345| 	var output = RunDetection(settings);
| 346| 346| 
| 347|    |-	//print(JSON.stringify(output, null, 1)+"\n");
|    | 347|+	// print(JSON.stringify(output, null, 1)+"\n");
| 348| 348| 
| 349| 349| 	for (var i = 0; i < output.warnings.length; ++i)
| 350| 350| 		warn(output.warnings[i]);

binaries/data/mods/mod/hwdetect/hwdetect.js
| 179| »   var·disable_audio·=·undefined;
|    | [NORMAL] ESLintBear (no-undef-init):
|    | It's not necessary to initialize 'disable_audio' to undefined.

binaries/data/mods/mod/hwdetect/hwdetect.js
| 180| »   var·disable_s3tc·=·undefined;
|    | [NORMAL] ESLintBear (no-undef-init):
|    | It's not necessary to initialize 'disable_s3tc' to undefined.

binaries/data/mods/mod/hwdetect/hwdetect.js
| 181| »   var·disable_shadows·=·undefined;
|    | [NORMAL] ESLintBear (no-undef-init):
|    | It's not necessary to initialize 'disable_shadows' to undefined.

binaries/data/mods/mod/hwdetect/hwdetect.js
| 182| »   var·disable_shadowpcf·=·undefined;
|    | [NORMAL] ESLintBear (no-undef-init):
|    | It's not necessary to initialize 'disable_shadowpcf' to undefined.

binaries/data/mods/mod/hwdetect/hwdetect.js
| 183| »   var·disable_allwater·=·undefined;
|    | [NORMAL] ESLintBear (no-undef-init):
|    | It's not necessary to initialize 'disable_allwater' to undefined.

binaries/data/mods/mod/hwdetect/hwdetect.js
| 184| »   var·disable_fancywater·=·undefined;
|    | [NORMAL] ESLintBear (no-undef-init):
|    | It's not necessary to initialize 'disable_fancywater' to undefined.

binaries/data/mods/mod/hwdetect/hwdetect.js
| 185| »   var·enable_glsl·=·undefined;
|    | [NORMAL] ESLintBear (no-undef-init):
|    | It's not necessary to initialize 'enable_glsl' to undefined.

binaries/data/mods/mod/hwdetect/hwdetect.js
| 186| »   var·enable_postproc·=·undefined;
|    | [NORMAL] ESLintBear (no-undef-init):
|    | It's not necessary to initialize 'enable_postproc' to undefined.

binaries/data/mods/mod/hwdetect/hwdetect.js
| 187| »   var·enable_smoothlos·=·undefined;
|    | [NORMAL] ESLintBear (no-undef-init):
|    | It's not necessary to initialize 'enable_smoothlos' to undefined.

binaries/data/mods/mod/hwdetect/hwdetect.js
| 188| »   var·override_renderpath·=·undefined;
|    | [NORMAL] ESLintBear (no-undef-init):
|    | It's not necessary to initialize 'override_renderpath' to undefined.

binaries/data/mods/mod/hwdetect/hwdetect.js
| 222| »   //·Enable·GLSL·on·OpenGL ES 2.0+,·which·doesn’t·support·the·fixed
|    | [NORMAL] ESLintBear (no-irregular-whitespace):
|    | Irregular whitespace not allowed.

binaries/data/mods/mod/hwdetect/hwdetect.js
| 222| »   //·Enable·GLSL·on·OpenGL ES 2.0+,·which·doesn’t·support·the·fixed
|    | [NORMAL] ESLintBear (no-irregular-whitespace):
|    | Irregular whitespace not allowed.

binaries/data/mods/mod/hwdetect/hwdetect.js
| 179| »   var·disable_audio·=·undefined;
|    | [NORMAL] JSHintBear:
|    | It's not necessary to initialize 'disable_audio' to 'undefined'.

binaries/data/mods/mod/hwdetect/hwdetect.js
| 180| »   var·disable_s3tc·=·undefined;
|    | [NORMAL] JSHintBear:
|    | It's not necessary to initialize 'disable_s3tc' to 'undefined'.

binaries/data/mods/mod/hwdetect/hwdetect.js
| 181| »   var·disable_shadows·=·undefined;
|    | [NORMAL] JSHintBear:
|    | It's not necessary to initialize 'disable_shadows' to 'undefined'.

binaries/data/mods/mod/hwdetect/hwdetect.js
| 182| »   var·disable_shadowpcf·=·undefined;
|    | [NORMAL] JSHintBear:
|    | It's not necessary to initialize 'disable_shadowpcf' to 'undefined'.

binaries/data/mods/mod/hwdetect/hwdetect.js
| 183| »   var·disable_allwater·=·undefined;
|    | [NORMAL] JSHintBear:
|    | It's not necessary to initialize 'disable_allwater' to 'undefined'.

binaries/data/mods/mod/hwdetect/hwdetect.js
| 184| »   var·disable_fancywater·=·undefined;
|    | [NORMAL] JSHintBear:
|    | It's not necessary to initialize 'disable_fancywater' to 'undefined'.

binaries/data/mods/mod/hwdetect/hwdetect.js
| 185| »   var·enable_glsl·=·undefined;
|    | [NORMAL] JSHintBear:
|    | It's not necessary to initialize 'enable_glsl' to 'undefined'.

binaries/data/mods/mod/hwdetect/hwdetect.js
| 186| »   var·enable_postproc·=·undefined;
|    | [NORMAL] JSHintBear:
|    | It's not necessary to initialize 'enable_postproc' to 'undefined'.

binaries/data/mods/mod/hwdetect/hwdetect.js
| 187| »   var·enable_smoothlos·=·undefined;
|    | [NORMAL] JSHintBear:
|    | It's not necessary to initialize 'enable_smoothlos' to 'undefined'.

binaries/data/mods/mod/hwdetect/hwdetect.js
| 188| »   var·override_renderpath·=·undefined;
|    | [NORMAL] JSHintBear:
|    | It's not necessary to initialize 'override_renderpath' to 'undefined'.

binaries/data/mods/mod/hwdetect/hwdetect.js
| 222| »   //·Enable·GLSL·on·OpenGL ES 2.0+,·which·doesn’t·support·the·fixed
|    | [NORMAL] JSHintBear:
|    | This line contains non-breaking spaces: http://jshint.com/docs/options/#nonbsp
Executing section cli...

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/1586/display/redirect

vladislavbelov accepted this revision.Jan 17 2020, 9:55 PM

I've reviewed and tested the patch. It looks ok for me. I'm going to commit it soon (fixed years and GL types by myself).