Test build GLES version with travis ci

[ghost]

This shows that the GLES implementation submitted by mupen64plus-ae doesn't build.

@littleguy77 @paulscode @Gillou68310 @krnlyng

[littleguy77]

What did we submit? I integrated the GLES version upstream in April 2013 and this is the first I've heard of any issues. We've obviously been using the GLES codepath without issue since well before then.

[ghost]

@littleguy77 see the travis ci output at https://travis-ci.org/mupen64plus/mupen64plus-video-rice/builds/40458530

It now builds after commit 4ca2c55 but the output still looks broken. The GLESv2 version of Glide64mk2 seems to work

Rice:

Glide64mk2:

[littleguy77]

Ah, I now I understand (at least part of) the problem, thanks to your latest commit. When I integrated the gles2 code, I simply relied on SDL's flag, SDL_VIDEO_OPENGL_ES2. SDL defines that after auto-detecting the platform. I had not intended the developer to force gles with their own flag (USE_GLES), but rather to understand how SDL was forcing it, since the SDL dependency was so pervasive.

Your top picture looks vaguely familiar, like I've seen that issue before. It might be something to do with the EGL setup not being correct. I must admit it's been a very long time since I looked at this, so I might be completely off the mark. But like you've discovered, there's a lot more to forcing the GLES2 path than simply compiling the correct cpp file or linking with the right library. https://github.com/mupen64plus/mupen64plus-video-rice/commit/2742a0ceb5f8d4df00c3384fba7fd972b17f434e

I would say this one is for @krnlyng to track down, since I never tried to get GLES working on non-Android platforms.

[ghost]

I cannot find any EGL related code in Rice. Can you give more hints?

[krnlyng]

All i can say is, that rice worked ootb on my N9, i've never had to worry about the underlying implementation (i only added the USE_GLES flag), so i am probably the wrong person to ask. I am currently learning GLES, so i might be able to help someday. My current phone (jolla) has some serious graphic driver bugs which prevent both rice video and gles2n64 from working: https://www.youtube.com/watch?v=oAKEssTtZWc but i've not seen graphic glitches like the one on your picture.

[littleguy77]

@fayvel By EGL I mean the UI side where the GL context is set up (buffer configuration, color depth, etc.). But like I said it's been such a long time that I could be completely off the mark.

@krnlyng Are you using mupen64plus, or mupen64plus-ae, on the N9? Can you describe your process for building and testing mupen on the N9?

Back when I integrated the code paths, the SDL2 API was pretty unstable with respect to the Android platform. Major interfaces were changing rapidly and the implementation often left something to be desired. Rather than track all the changes, we decided to freeze it at release 2.0.0 and just hand-modify a few problem areas. Rather than defining a custom flag like USE_GLES, we changed some of the last lines in SDL_config_android.h, so that SDL would be fully cognizant of the code path we wanted. See these commits for more info: https://github.com/mupen64plus-ae/mupen64plus-ae/commit/a887b6f92a451cb21a058a9448dd8f29c42789dd https://github.com/mupen64plus-ae/mupen64plus-ae/commit/e175994bfc4b471c993a95749332163c51877da8

[ghost]

@littleguy77 The GL context seems to be set up fine by the core/ui. Otherwise glide64mk2 wouldn't work in GLES2 mode.

[littleguy77]

Good point. Like I said, it's been a long time.

[ghost]

@krnlyng the texture+color problem in you video looks exactly like the one I see here too (especially the orange zelda logo). But I don't have the fancy lines and rotation. The rotation is a known problem in Qualcomm drivers. But the texture/color stuff is a problem in the GLES part of rice.

I am using mesa 10.3.2 on Linux 3.16 with an Ivybridge Intel card

[krnlyng]

@littleguy77 i use mupen64plus as both N9 and Jolla are non-android devices (the Jolla phone has an android compatibility layer for apps but mupen64plus-ae doesn't work with it).

as for building and testing for the N9/Jolla, i build all mupen64plus modules using the makefile in projects/unix/Makefile in a scratchbox environment (http://www.scratchbox.org/) using "SOMEFLAGS... make all" and then upload the binaries to the device via scp. sometimes i even build directly on the device (if i am not at home), then i use "SOMEFLAGS... make all" and "make install". i debug via gdb and testing is done on the device directly (for the jolla i could use the virtualbox image they provide with their SDK but i haven't bothered yet).

@fayvel Okay i thought the texture + color problem was a side effect of the driver bugs, as everything seems to be affected by it. Note: even with my rotation hack like the one i've written for glide64mk2 both gles2n64 and rice-video produce garbage output (according to HdkR from dolphin-emu it's also due to (a side effect of) the rotation bug). I need to check if current master of rice works on my N9, but i don't remember any issues with it.

[ghost]

Ok, please compile the stuff with the sanitizers:

CFLAGS="-fsanitize=undefined -fsanitize=address  -fsanitize=leak -g3" make -C projects/unix all USE_GLES=1 OPTFLAGS="" SDL_CONFIG=sdl2-config V=s
LIBGL_DEBUG=verbose mupen64plus --gfx projects/unix/mupen64plus-video-rice.so ~/zelda.z64

=================================================================
==26688==ERROR: AddressSanitizer: global-buffer-overflow on address 0x7f0f22fcb580 at pc 0x7f0f229a0779 bp 0x7fff5a747110 sp 0x7fff5a747108
READ of size 4 at 0x7f0f22fcb580 thread T0
    #0 0x7f0f229a0778 in COGLExtRender::ApplyTextureFilter() ../../src/OGLExtRender.cpp:293
    #1 0x7f0f229cb94b in CRender::SetTextureFilter(unsigned int) ../../src/Render.cpp:1984
    #2 0x7f0f22a2d3e3 in DLParser_RDPSetOtherMode(Gfx*) ../../src/RSP_Parser.cpp:1077
    #3 0x7f0f22a2b749 in DLParser_Process(OSTask*) ../../src/RSP_Parser.cpp:891
    #4 0x7f0f22b159ec in ProcessDListStep2 ../../src/Video.cpp:285
    #5 0x7f0f22b1a05c in ProcessDList ../../src/Video.cpp:861
    #6 0x7f0f1b9b695d in DoRspCycles (/usr/lib/x86_64-linux-gnu/mupen64plus/mupen64plus-rsp-hle.so+0x795d)
    #7 0x7f0f2601c87b (/usr/lib/x86_64-linux-gnu/libmupen64plus.so.2+0x5787b)
    #8 0x7f0f152b62da (+0x82da)

0x7f0f22fcb580 is located 0 bytes to the right of global variable 'mtex' from '../../src/OGLExtRender.cpp' (0x7f0f22fcb560) of size 32
0x7f0f22fcb580 is located 32 bytes to the left of global variable 'minflag' from '../../src/OGLExtRender.cpp' (0x7f0f22fcb5a0) of size 32
SUMMARY: AddressSanitizer: global-buffer-overflow ../../src/OGLExtRender.cpp:293 COGLExtRender::ApplyTextureFilter()
Shadow bytes around the buggy address:
  0x0fe2645f1660: f9 f9 f9 f9 01 f9 f9 f9 f9 f9 f9 f9 01 f9 f9 f9
  0x0fe2645f1670: f9 f9 f9 f9 04 f9 f9 f9 f9 f9 f9 f9 04 f9 f9 f9
  0x0fe2645f1680: f9 f9 f9 f9 04 f9 f9 f9 f9 f9 f9 f9 00 00 00 00
  0x0fe2645f1690: f9 f9 f9 f9 00 00 00 00 f9 f9 f9 f9 00 00 00 00
  0x0fe2645f16a0: f9 f9 f9 f9 00 00 00 00 f9 f9 f9 f9 00 00 00 00
=>0x0fe2645f16b0:[f9]f9 f9 f9 00 00 00 00 f9 f9 f9 f9 00 00 00 00
  0x0fe2645f16c0: f9 f9 f9 f9 04 f9 f9 f9 f9 f9 f9 f9 04 f9 f9 f9
  0x0fe2645f16d0: f9 f9 f9 f9 04 f9 f9 f9 f9 f9 f9 f9 04 f9 f9 f9
  0x0fe2645f16e0: f9 f9 f9 f9 04 f9 f9 f9 f9 f9 f9 f9 04 f9 f9 f9
  0x0fe2645f16f0: f9 f9 f9 f9 04 f9 f9 f9 f9 f9 f9 f9 04 f9 f9 f9
  0x0fe2645f1700: f9 f9 f9 f9 04 f9 f9 f9 f9 f9 f9 f9 00 00 00 00
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:       fa
  Heap right redzone:      fb
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack partial redzone:   f4
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Contiguous container OOB:fc
  ASan internal:           fe
==26688==ABORTING

This doesn't happen with the GL implementation

[ghost]

Before there is a misunderstanding: the new patch doesn't fix the graphical problems

[krnlyng]

"SDL_CONFIG=sdl2-config" unfortunately on the N9 there is no SDL2 only SDL1.2, also the sanitizers are not available in my build environment (scratchbox), and building rice currently fails: https://bpaste.net/show/db138de2cc25 i might look into this some other time but i've got to study now, but i can assure you, at the time USE_GLES was added it worked fine on my N9 (git bisect maybe?)

[ghost]

@krnlyng I've also tested with the version (2742a0ceb5f8d4df00c3384fba7fd972b17f434e) when you've added USE_GLES and it didn't work.

But it works better with 80cc1e4427d8daf4991a04ccd236366ab32b5e59 + Makefile change to compile $(SRCDIR)/OGLES2FragmentShaders.cpp instead of $(SRCDIR)/OGLFragmentShaders.cpp (so before you've added USE_GLES)

The first commit which breaks it is d9c1b73d7492e0dc32657a080ad8937fb9c618d9 from @littleguy77. I would guess from the content that it breaks when the Rice option "Mipmapping" is set to 1, 2 or 3. Anything else like 0 works.

Anyone an idea what must be done to support Mipmapping with GLES? @Gillou68310 maybe?

[krnlyng]

@fayvel yeah i admit that it doesn't work on my jolla phone, all i was saying is that it worked on the N9

[ghost]

Ok, 4873e8b86c6432d92919e7dddbf29da30aa6e478 should fix the texture problem when mipmapping is enabled in USE_GLES=1 mode

[littleguy77]

This pull request has kind of diverged to a totally separate topic. I have no issue with the Travis script (it's a great addition BTW). But other than as a diagnosis tool, I don't think 4ca2c557ae2e8febab8734522910cc7ea7408ae9 is the right way to address the build problem.

SDL needs to "know" whether to use GLES just as much as video-rice does. So replacing SDL_OPENGL_ES2 with a custom flag can create a potential conflict between SDL and rice. That conflict will only appear when building for certain platforms. In other words, what builds and runs on some platforms will fail on other (possibly obscure) platforms.

I would encourage the interested parties to read the SDL documentation and understand the role of the platform-specific headers in [SDL2]/include/SDL_config_xxxx.h. Work with SDL's flags rather than against them.

[krnlyng]

@fayvel still looks the same on my Jolla phone...

@littleguy77 yes the USE_GLES flag was probably not a good decision, but at the time nobody complained about it, it was simply merged without comment

[ghost]

@littleguy77 yes, you have to also introduce USE_GLES in other parts to generate a clean GLES context. See

SDL_GL_SetAttribute(SDL_GL_CONTEXT_PROFILE_MASK, SDL_GL_CONTEXT_PROFILE_ES);
SDL_GL_SetAttribute(SDL_GL_CONTEXT_MAJOR_VERSION, 2);

But your old version doesn't help at all. It is simply not possible to compile your version against GLES2 when OpenGL would also be available in the specific build of SDL2. Even when the display server or driver cannot handle OpenGL. And this cannot be expressed with your preprocessor "configure" magic. You would have to manually rebuild SDL2 and strip it from the support of other GL profiles and then rebuild rice against it. @krnlyng started with a solution which works better because now you only have to build mupen64plus and not SDL2. But I agree, maybe it is better to make a quick check if GLES2 is supported by SDL2 when using USE_GLES=1.

@krnlyng the rotation and weird blocks should be the same. But the textures should look better

[ghost]

I close this pull request because we hit a dead end and it got off topic

[littleguy77]

@krnlyng I don't fault you for the solution you took. It certainly worked for you and was a clean commit. We just weren't aware of the actual complexity across the platforms at the time.

@fayvel Good inputs, fair enough. This is the first I've heard of issues with the GLES2 codepath, and it may simply be because this is the first time anyone upstream is actually exercising it. I had hoped for this discussion back in April 2013 when I did the integration but I guess we take what we can get, when we get it :P .

I think you're right, with the existing code we would need to modify and recompile SDL2 to get the GLES integration on many platforms. I completely agree that is not a tenable solution and I wouldn't advocate for that. The only reason we even do it in mupen64plus-ae is because that project started when SDL had no android support whatsoever and required a custom fork anyhow (the short-lived 1.3).

I'm all ears for a better solution to the flags, as I've never actually tried building for GLES2 on the desktop. I definitely agree this is an issue that needs to be fixed. I guess the bottom line is that we should take our time and see if SDL has a proscribed approach for dealing with this (certainly they must?). The "right" solution might be as simple as setting some other SDL flags in the makefile.

[littleguy77]

SDL_GL_SetAttribute(SDL_GL_CONTEXT_PROFILE_MASK, SDL_GL_CONTEXT_PROFILE_ES);
SDL_GL_SetAttribute(SDL_GL_CONTEXT_MAJOR_VERSION, 2);

@fayvel Looks like you understand the SDL interplay better than I had assumed, and probably better than me. If we can robustly target GLES using a makefile flag that would indeed be a better solution than the config file stuff (I didn't like the magical-ness of it either). It would also obviate mupen64plus-ae's need to compile a modified SDL, which I'm all for.

[krnlyng]

@fayvel sorry i have to correct, the glitches don't appear on my jolla phone (except the rotation related ones) anymore :)