Skip to content
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

Rework GLES support for portability #826

Merged
merged 15 commits into from
Jan 8, 2019
Merged

Conversation

dos1
Copy link
Contributor

@dos1 dos1 commented Sep 21, 2017

Note: This pull request is mostly for review purposes, probably shouldn't be merged in just yet.

Recently I've started looking into the possibilities of porting Allegro (and, in turn, my game engine) to various platforms. I've found @ptitSeb fork with OpenPandora and ODroid support, @allefant's patches for Emscripten and also started playing with Nokia N900 support by myself. SDL backend allowed me to experiment with X11/EGL without actually implementing it in Allegro itself just yet, so I could focus on the GLES stuff alone.

What I found was a huge mess of #ifdefs. If I wanted to consolidate all the patches I found into one codebase, I would end up with lines like:
#if defined ALLEGRO_IPHONE || defined ALLEGRO_ANDROID || defined ALLEGRO_RASPBERRYPI || defined ALLEGRO_PANDORA || defined __EMSCRIPTEN__
so before I went further, I decided to clean it all up and make parts that are actually GLES specific (as opposed to device specific) check for GLES instead of any particular device.

In turn I've found out some other issues. For instance, on some platforms linking both libGLESv1_CM and libGLESv2 into the same binary breaks GLES2. Also, it wasn't actually easily possible to request a GLES build on a device that wasn't Android-based, iOS-based or Raspberry Pi. In turn, I've reworked the build system a bit.

When it comes to the code, I've tested it so far on GNU/Linux on X11/GLX/GL, SDL/GLX/GL, SDL/EGL/GLES and Android. It still needs to be tested on other platforms - I should be able to do it for macOS and Raspberry Pi, but I can't for iOS, so that would need some external testing. I think it should be fairly solid, aside of iOS-case, where I have no idea, so I might have broken something there.

Compiling with GLES3 is broken right now with this patch, I'll look into it later. Compiling with GLES1 was already broken (and impossible without modifying CMakeFiles.txt), but I've seen some patches floating somethere that should fix it, just haven't looked into them yet.

When it comes to the build system, I'm not sure whether the way I've taken is the best one. Especially I don't like that WANT_GLES2 requires WANT_GLES, which alone means GLES 1.1, and together means GLES 2.0, which are completely different builds (GLES3 could stay that way though). I'd be thankful for any comments on how to tackle it.

@beoran
Copy link
Contributor

beoran commented Sep 22, 2017

I am glad to hear you are taking on this cleanup, because we will need opengl es for Wayland support on Linux. Still, keep in mind that most platform specific hacks are probably there for a reason...

@dos1
Copy link
Contributor Author

dos1 commented Sep 23, 2017

Yeah, I tried not to touch platform specific hacks and just replace the ifdefs only if they looked GLES specific instead of device specific. After all, I made it by trying to compile Allegro on regular Linux with GLES and no GL. Still, I might have made some mistakes, especially with iPhone support, so any help is appreciated.

However, it shouldn't really be needed for Wayland support. With Wayland you need to use EGL to get the context, but you can obtain regular GL context that way too. In the future I'd love to see Allegro supporting X11/EGL with GLES (and Wayland with both GL and GLES), especially as there are more and more GNU/Linux-based GLES capable platforms without X11, GLX or both, but that's a different story :)

@dos1 dos1 mentioned this pull request Sep 25, 2017
@beoran
Copy link
Contributor

beoran commented Sep 27, 2017

Now, on Linux /Raspberry, the graphics driver is statically compiled in. I think in the long run we should move to dynamic graphic drivers, like on Windows, where you can use configuration settings or display flags to select the driver.

But for this PR, I think it seems OK the way it is for now.

@dos1
Copy link
Contributor Author

dos1 commented Sep 28, 2017

Yeah, that would be the best, especially for Android, where now you need to compile with GLES3 support to get multisampled render buffers support and by doing that you're making the build incompatible with devices with just GLES2.

Anyway, this patch is definitely incomplete, as I've just tried to build it on Raspberry Pi with new, open drivers (so without ALLEGRO_RASPBERRYPI) and while it works with OpenGL, it doesn't with GLES2 - first it fails on lack of GLES1 support in open drivers, then it errors out on some symbol redefinitions from GL headers included by GLX headers. So basically, it's still a huge mess :P

@dos1
Copy link
Contributor Author

dos1 commented Oct 14, 2017

Rebased and reworked even more. Still WIP though.

I think I'm kinda happy with the build system now. GL_BUILD_TYPE cmake variable defines which kind of OpenGL API we target - it can be "auto", "gl", "gles1" or "gles2+". Rationale: until Allegro switches to 100% dynamic dispatch for GL libs, it cannot link to multiple GL APIs at the same time, because of overlapping symbols. We were lucky that Allegro worked on most Android phones so far, as what it was doing was pretty much undefined behavior (and it actually breaks some GLES platforms, like Nokia N900).

"auto" tries to automatically determine what APIs are available, tries and fallbacks them in this order: "gl", "gles2+", "gles1" (on Android, iOS and Raspberry Pi with Broadcom drivers it skips "gl").
If the user specifies API manually, no fallback is made.
Default value of GL_BUILD_TYPE is "auto".

I have left WANT_GLES3 variable, which works now only when GL_BUILD_TYPE is set to "gles2+". GLES3 is backwards compatible with GLES2, and GLES3 is only used to provide multisampling on Android. There is no reason for making the Allegro build non-backwards compatible, so the static dispatch of GLES3 function should be replaced with dynamic dispatch and runtime feature testing - so there will be no need for GLES3 specific build at all. I haven't worked on it yet though.

Thanks to this second commit I've finally managed to run Allegro with GLES 2.0 context on PC! \o/

Status as for right now ("works" means just "I've successfully launched some game with it" :P):

  • X11/GL: works, tests pass
  • X11/GLES1: does not build[1]
  • X11/GLES2+: works, test_driver segfaults (will check it later)
  • SDL/GL: works, tests pass
  • SDL/GLES1: does not build[1]
  • SDL/GLES2+: does not build[2]
  • Android: untested yet, will do later
  • Raspberry Pi: untested yet, will do later
  • iOS: untested, won't do later, please help :(

macOS and Windows: don't care for now. I won't work on GLES support for those platforms (is there any need for it there anyway?), so "nothing broke" is the only metric I'll check at the end :D

TODO:

  • use libOpenGL+libGLX instead of libGL when possible
  • fail configuring when libGLX is not found on X11/GLES builds
  • X11/EGL support (as opposed to X11/GLX)

[1] Allegro uses static dispatch for some extension functions, that Mesa has stopped exporting in 2014: https://cgit.freedesktop.org/mesa/mesa/commit/?id=1a59f9a131318e1239b47b9ea4fe7c84f461cf37 It might still work on some mobile proprietary implementations, but it should be switched to dynamic dispatch anyway.
[2] Some header mumbo-jumbo, will look at it later.

@dos1
Copy link
Contributor Author

dos1 commented Oct 15, 2017

test_driver segfaults with GLES because it tries to lock the bitmap with ALLEGRO_PIXEL_FORMAT_RGBA_8888 format, which is apparently not supported? (glReadPixels for format RGBA_8888 failed (GL_INVALID_OPERATION)). Changing the al_lock_bitmap call to use ALLEGRO_PIXEL_FORMAT_ABGR_8888_LE fixes it, but I guess it needs to be actually fixed in Allegro's locking/converting code, as GLES's glReadPixels does not support converting to variety of formats like its GL counterpart does.

All tests pass \o/ except most of the S3TC ones and "test texture rw f32 ABGR_F32". Could somebody confirm that it's actually related to GLES capabilities and not some bug? (seems like it needs OES_texture_float extension and it doesn't actually guarantee any support for rendering into such texture with FBOs)

@beoran
Copy link
Contributor

beoran commented Oct 15, 2017 via email

@dos1
Copy link
Contributor Author

dos1 commented Oct 15, 2017

I mean, in GLES. It might be reasonable for them to fail, but definitely not for the reason you citied, as they all pass with desktop GL on the same PC :)

But it seems like floating point textures are not available in GLES, and OES_texture_float extension provides only some partial support, so it is possible that this test is simply meant to fail on GLES. Haven't checked S3TC yet, but it might be similar.

@dos1
Copy link
Contributor Author

dos1 commented Oct 17, 2017

Did some fixes and more tests, current status:

  • X11/GL: works, tests pass
  • X11/GLES1: does not build
  • X11/GLES2+[1]: works, test_driver needs format change
  • SDL/GL: works, tests pass
  • SDL/GLES1: does not build
  • SDL/GLES2+: builds, but does not work yet
  • Raspberry Pi/GLES2: works, now on Raspbian Stretch as well (master doesn't :D)
  • Android: untested yet, will do later
  • iOS: untested, won't do later, please help :(

[1] Requires CMake 3.10's FindOpenGL to correctly pick up GLVND libraries; can be passed manually as well though.

@ptitSeb
Copy link

ptitSeb commented Oct 17, 2017

GLES Hardware usually doesn't support S3TC texture compression, nor Float Texture. So it seems normal the test fails.

@beoran
Copy link
Contributor

beoran commented Oct 17, 2017

I also don't have an iOS device, but please keep up the good work.
Also, I read this:
https://stackoverflow.com/questions/3326641/opengl-without-x-org-in-linux

I'd love to see this work with EGL, also on the Linux desktop. This would simplify a future Wayland port.

@SiegeLord
Copy link
Member

This is shaping up to be super impressive work! Don't worry about iOS/OSX, I can help with that (once I find time :D).

@dos1
Copy link
Contributor Author

dos1 commented Oct 18, 2017

Thanks :D I've just tested Android (with GLES2) and seems to work fine \o/

So what's left:

  • misbehaving SDL/GLES2 build to investigate
  • fixing GLES1 builds (and potentially refactoring GLES3 at the same time)
  • fixing test_driver's locking format incompatibility

And that actually would be it for this PR. Then it will be time for a new one with X11/EGL :)

@dos1
Copy link
Contributor Author

dos1 commented Oct 19, 2017

Fixed both SDL/GLES2 build (turned out to be mismatch of SDL's renderer and used GL context) and the issue with test_driver (some GLES locking functions lacked format conversion). So only GLES1 is missing now \o/

@dos1
Copy link
Contributor Author

dos1 commented Jan 19, 2018

I'm not sure what to do with this PR now. Seems like the only thing missing is fixing extension calls in GLES 1.1 - they need to be dynamically dispatched at runtime, because conforming implementations should not export symbols for extension to link-time lookup (and Mesa's does not). However, I've tried looking at Allegro's GL extension handling and... I'm not exactly sure how to utilize it properly when it comes to GLES without making a huge mess, and TBH, I don't have so much time to get myself deep into this code, and I suspect it would need to be significantly reworked anyway (or replaced by existing solution, like libepoxy) for dynamic dispatch support for OpenGL on Linux in general (so GL and GLES could be supported by the same build).

Anyway, GLES 1.1 was already broken before this PR. Now it's very close to get unbroken, so if anyone familiar with all this GL dispatch code in Allegro want to jump in and fix it, that would be nice :) Otherwise, GLES 1.1 stays broken for now and I would consider this PR done (sans review/testing, of course).

@SiegeLord
Copy link
Member

The plan is to prioritize this after 5.2.4. Sorry for not following up on this yet!

@beoran
Copy link
Contributor

beoran commented Jan 25, 2018 via email

@SiegeLord
Copy link
Member

Hey @dos1, are you still interested in pursuing this? I've got time to invest into getting this working. I've read through it and it looks pretty good. One thing that would make it better on the surface would be to intruduce a ALLEGRO_OPENGL_FIXED_FUNCTION or something, to be used whenever you have ifndef GLES2.

I am not sure what to do with GLES1, is that something we need to support? That's beyond the scope of this change, however, as you say.

@dos1
Copy link
Contributor Author

dos1 commented May 26, 2018

Yup! As the amount of spare time allows, of course, but I'd still want to have this finished up.

Personally I'm not really interested in any fixed function support, so I wouldn't mind seeing it go away, let alone just GLES1 - I just wanted to not break anything with this refactor and potentially maybe even fix some stuff. GLES1, at least on Mesa, turned out to need some more changes to unbreak, so I simply gave up for now :)

IMO there are two sensible options for long term regarding GLES1:

  • remove all fixed function codepaths, cleaning up a lot of obsolete code; will probably break some games though, so I'm not sure if this would be acceptable at all
  • rework GL linking to be dynamic, adding ability to support both GL and GLES with one binary just like SDL does, which, as an added bonus, should also fix GLES1 for free

...or both :D

Second option will bring other advantages as well, so if we keep fixed function stuff around, we might keep GLES1 as well, because why not. I'm interested in running Allegro on Nokia N900, it's not hard for me to imagine there's someone out there who would like it to run on something pre-GLES2 ¯\(ツ)

@dos1
Copy link
Contributor Author

dos1 commented Jun 6, 2018

Just a note: Newer versions of cmake have improved GLVND detection in OpenGL packages. Something potentially worth looking into here.

CMake Warning (dev) at /usr/share/cmake-3.11/Modules/FindOpenGL.cmake:270 (message):
  Policy CMP0072 is not set: FindOpenGL prefers GLVND by default when
  available.  Run "cmake --help-policy CMP0072" for policy details.  Use the
  cmake_policy command to set the policy and suppress this warning.

  FindOpenGL found both a legacy GL library:

    OPENGL_gl_LIBRARY: /usr/lib/libGL.so

  and GLVND libraries for OpenGL and GLX:

    OPENGL_opengl_LIBRARY: /usr/lib/libOpenGL.so
    OPENGL_glx_LIBRARY: /usr/lib/libGLX.so

  OpenGL_GL_PREFERENCE has not been set to "GLVND" or "LEGACY", so for
  compatibility with CMake 3.10 and below the legacy GL library will be used.

@dos1
Copy link
Contributor Author

dos1 commented Oct 26, 2018

I'm successfully using this branch now with SDL2 backend on X11/EGL-only Nokia N900 \o/ Now I'm likely going to use it for the Librem 5, so I'd like to try to push this thing a bit forward.

So, there's a ALLEGRO_OPENGL_FIXED_FUNCTION suggestion and the GLVND thing in CMake to look at. Anything else?

@SiegeLord
Copy link
Member

Yep, just those two things on your end. On our end, we'll test things out on iOS and at least some Android devices.

@dos1
Copy link
Contributor Author

dos1 commented Oct 27, 2018

Note to self: make sure the CMake configuration process properly bails off when the GL deps aren't found.

@dos1
Copy link
Contributor Author

dos1 commented Oct 30, 2018

There's one more thing - the GLES2/X11 build doesn't compile under 32-bit platforms, as it includes both GL and GLES2 headers, which doesn't work on on 32-bit x86 nor arm (interestingly, it works fine on both amd64 and aarch64). GL headers get included via GLX ones. I'll have to look around if this can actually be fixed without using EGL instead of GLX there :x

Regular GL build should be unaffected though.

@dos1
Copy link
Contributor Author

dos1 commented Nov 20, 2018

OK, I've updated and rebased the branch. I looked around and it seems there's no way to support GLES via GLX on Mesa with 32-bit platforms, so I think it's all done :)

@SiegeLord
Copy link
Member

I'm now focusing on getting this merged. I've started by testing this on Android, and noticed that many examples ceased to output anything. The examples that ceased to work were those that were not using the programmable pipeline (e.g. ex_draw_bitmap). Is it understandable how that works? Were we using some weird poorly supported drawing path, and this change fixes it by disabling it (now requiring the examples to use the programmable pipeline to work?). I'm probably fine with that, but just want to make sure I understand what happened.

Also, along the same lines, I notice in xdisplay.c you're setting the programmable pipeline flag when ALLEGRO_CFG_OPENGLES2 is defined. How does that work? Is this what you meant when you said at some point that Allegro needs make the GL vs GLES choice at compile time? I.e. we can't compile with GL and GLES at the same time?

If that's the case, should we be setting the PROGRAMMABLE_PIPELINE on Android/iOS too?

Meanwhile, I'll test things on OSX/iOS as well.

@dos1
Copy link
Contributor Author

dos1 commented Dec 9, 2018

Ah, right. My engine always sets PROGRAMMABLE_PIPELINE, so it worked for me on Android :)

Basically, desktop GL still contains backwards compatibility with old fixed function pipeline, while GLES2+ does not. Furthermore, with most drivers out there, we cannot link to GLES1 (fixed only) and GLES2 (pipeline only) at the same time, so the only way to get both supported by a single binary would be to use dlopen & dlsym, which Allegro doesn't do right now (I've seen some old code under an always false ifdefs for it though, but probably not for Android anyway). Therefore, a GLES2+ build (which is what you always get on Android as GLES1 is broken right now) has to always use the programmable pipeline, so yes, it should be set on Android just like what I've added to xdisplay as well. Let me fix that.

BTW. There's no GLES on Windows and macOS without jumping through some hops, am I right?

@SiegeLord
Copy link
Member

So you're saying that before this chance, we're using GLES1 for Android somehow? I'm just confused how the examples currently work on Android without using programmable pipeline.

And sure, I wouldn't worry about GLES for Windows and macOS.

@dos1
Copy link
Contributor Author

dos1 commented Dec 9, 2018

Nah, I've just generalized some code in this rework and now it expects this flag to be correctly set on GLES, which wasn't the case before.

Pushed a fix.

@SiegeLord
Copy link
Member

I tried it out on iOS (on a simulator), and had to make a few changes to get things to compile:

SiegeLord@8987c80

First, it appeared that some of the OES APIs are actually present without the suffix in GLES2, I suppose that makes sense. Then, glFramebufferTexture2DMultisampleEXT doesn't appear to exist on iOS at all. The old code explicitly excluded iOS there, so I restore that condition.

For CMake, for iOS GLES is inside a framework, so there's no include dir to be set, a minor change. I also disabled even looking for GLES, so as to remove the confusing GLES2 not found messages. I tested things a little bit, and it appeared to work.

@SiegeLord
Copy link
Member

SiegeLord commented Dec 17, 2018

Worked a little more on the Android issue: it turns out we were just setting the ALLEGRO_PROGRAMMABLE_PIPELINE too late, if we set it earlier then programs that don't set ALLEGRO_PROGRAMMABLE_PIPELINE will work okay on Android (but will set that flag correctly under the hood).

I'll want to check over other instances of this, in case it matters there as well.

I'm collecting these changes in this branch: https://github.com/SiegeLord/allegro5/tree/fixes_for_dos1

@SiegeLord
Copy link
Member

Alright, I tested this some more on Raspberry Pi, and while I couldn't get gles2+ working, you said you did so it's likely some misconfiguration on my part. The toolchain.cmake approach still compiles fine, and that's all that I've ever seen, so it sounds good to me.

Elias tested his game against this and it appeared to work as well, so I think I'm happy to merge this and we'll work out the bugs as they come. Thanks a lot @dos1, sorry it took such a long time but I truly appreciate your hard work on this.

@SiegeLord SiegeLord merged commit ac7f859 into liballeg:master Jan 8, 2019
@dos1
Copy link
Contributor Author

dos1 commented Jan 8, 2019

Yay! :)

I'm going to play with Raspberry Pi in the next few days, so I'll recheck everything. Keep in mind that regular GLES2+ on Raspberry Pi needs enabling Mesa drivers - you can't make a build for Pi that works on both drivers yet... I'd eventually want to see Allegro using dynamic linking and having multiple system backends in one binary for other reasons as well (like Wayland), so it's definitely worth looking into in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants