mupen64plus / mupen64plus-video-rice

Video plugin for the Mupen64Plus v2.0 project, using OpenGL. This plugin is based on the RiceVideoLinux plugin from earlier versions of Mupen64Plus.
31 stars 40 forks source link

[WIP] Color Combiner Refactoring #31

Closed Narann closed 8 years ago

Narann commented 9 years ago

And here we are.

Here is the "new" Color Combiner. A lot of old fixed pipeline gl calls have been remove. A lot of classes supporting various OpenGL version on have been removed and the main class OGLColorCombiner is now OpenGL 2.1/OpenGLES 2 and rely on GLSL.

This commit make Rice incompatible with < OpenGL2.1. OpenGLES 2 use the same code path than the OpenGL one. OpenGLES 2 still need some fixes but it compile and run on OGLES 2 hardware.

From my various regression tests we should not have lost anything.

I've update config file update process so you should not lost anything regarding your config.

There is still some work to do. OGLFragmentShader and OGLESFragmentShader files are still here but should be removed safely. Demuxer is sooooo integrated deep into Rice code that I'm not able to remove yet but I'm not too far. Only few calls in the deep of Rice still need it.

Feel free to discuss, compile, test, etc...

littleguy77 commented 9 years ago

Do you plan to rebase this pull request once it's tested and ready? That would

Narann commented 9 years ago

I wasn't aware about git rebase actually, I just discover it today. Do you mean this.

Any help is welcome: What you suggest is to:

Is that right?

littleguy77 commented 9 years ago

Yes that is what I meant. I think it's fine to keep working this branch until you're confident in the final product. Then when you feel good and ready, go back and do several rounds of interactive rebasing to get a clean set of logically distinct commits. Split, reorder, squash, repeat. Then submit a new PR. I catch all sorts of little things in the cleanup process and consider it essential. I'm not the world's greatest coder but if you look at my commits in mupen64plus-ae you will hopefully see what I mean.

Narann commented 9 years ago

I think it's fine to keep working this branch until you're confident in the final product.

I will maybe look kinda silly but: "I am..."

I'm not currently working on the Combiner anymore. I consider it ready (as I said on the mailing list few month ago). It work on Desktop OpenGL 2.1 and OpenGLES 2 in the same code path without regression troubles (my current not-public regression suite reference images are using the commit just before mine).

go back and do several rounds of interactive rebasing to get a clean set of logically distinct commits.

As I didn't understand what you mean I guess I will have to spend more time in the git doc. :)

I will look at your commits but you have to consider it has been a refactor, not modifying few lines. Write the combiner has been quite "easy" but remove all deprecated stuff as been difficult (from july to last month). You can't remove nested classes easily, same for the Demuxer, I simply could not remove it in one nice commit, I've moved very slowly to remove things one by one without break everything (look at the line number of this PR: +2,201 −13,316). And basically, you can compile and run each commit without have any regression problem (based on the core regression test I run a huge amount of time).

Do you suggest to decrease the number of commits in favor of few big commits?

Anyway, I will try to rebase my work. I just hope I will not lost all this commits because I've tried to isolate the each modification granularly.

Narann commented 9 years ago

I've read some rebasing docs and tutorials (I like it!) and tryied to rebase and I have a lot conflicts I can't handle manually. But more I think about it more all of this make sense as my refactoring touch to almost every part of the code and some commits done in the master are not relevant anymore I've created some tickets.

Just to give you an overview:

 -#if SDL_VIDEO_OPENGL
 +#ifndef USE_GLES
  #include "OGLExtensions.h"
++<<<<<<< HEAD
 +#include "OGLFragmentShaders.h"
 +#else
++=======
+ #include "OGLSecondFragmentShaders.h"
+ #elif SDL_VIDEO_OPENGL_ES2
++>>>>>>> 

Even if I would merge such commit manually (I started but stopped to the fourth), each of them would be broken and would not run the make (something I tried to keep as much a possible). Once again, some commits are huge.

Because the refactoring started before the some commits I realize I should have think about it sooner. To my defense, the repo was almost dead when I started and I talk about my work many times on the mailing list without had any feedback. :) Each time I noticed a commit in the upstream I cherry picked it if it was still relevant.

What I suggest is to tag some commits with incremented minor version and a release note. I consider this commit as the latest of the not refactored branch. It still compile and run on OpenGL 1.4. My current concern is to reach the upstream asap (I could force the pull, I have the "permission" for this but I always prefer discuss.

My secret hope is: As soon the refactor (and some tags to help you downstream devs to keep track of this big move) is in the upstream, everything should be smooth again.

Is this ok for you?

I know it's not the perfect plan but I think version tags would help you in your regression concern.

littleguy77 commented 9 years ago

I definitely understand that it can be simply too hard to rebase when the branches diverge this much (as they often do with such large refactorings). That's why it was only a question/suggestion and not a request. Also, I used the term "rebase" perhaps too generically when I should have said "cleanup" or "straighten". I often use git rebase --interactive to simply reorder/squash/split commits without actually moving the base, i.e. just to remove "fixups" and other backtracking.

In a perfect world, it would be good pick the glLoadIdentity line out of commit 44c9482 and put it into 30fa02e, otherwise the GLES codepath doesn't build for all commits in between, which is bad if we ever need to git-bisect that section of history. The "easiest" way to do this would be

git checkout https://github.com/Narann/mupen64plus-video-rice
git rebase -i --preserve-merges 13c60346a

which brings up the following in a text editor

pick 30fa02e Remove deprecated unused opengl functions
pick 8c8fb26 merge OGLExtRender with OGLRender class. Potentially create some memory leak as the constructor and destructor are empty.
pick b7a67db Remove Force fog option (this option actually break a lot of stuff and make most games unplayables Remove OGLRender::SetFogEnable as it now do the exact same thing than CRender::SetFogEnable and so, it useless
[snip...]

which you would change the first line to

edit 30fa02e Remove deprecated unused opengl functions

Save and exit the text editor, then open osal_opengl.h and add the line back in:

  // No-op substitutions (unavailable in GLES2)
+ #define glLoadIdentity()
  #define glReadBuffer(x)
  #define glTexEnvi(x,y,z)

Save and exit the text editor, then

git add src/osal_opengl.h
git rebase --continue

Unfortunately, you will have to manually re-resolve the merge conflicts from 2121b18. :frowning: In the end, this might not be worth all the trouble. I certainly wouldn't blame you if you didn't want to do this.

But I thought it was worth providing a concrete example to better explain what I meant when I asked

Do you plan to rebase this pull request once it's tested and ready?

:grinning: Rebasing is definitely an art that requires a lot of practice (probably 2000 hours :)) I try to rebase/fixup as I go, otherwise it's much more difficult later.

littleguy77 commented 9 years ago

In any case, I appreciate that you opened it for discussion. It's fine by me if you are ready to merge. I'm sure we can work through any regressions that might come up, and I'm willing to pay that price for code that is easier to understand and maintain in the future.

Thanks for all your hard work :+1:

Narann commented 9 years ago

Thanks a lot for this tutorial @littleguy77! I will try to review every commits and see if rebase --interactive allow me to diminish the number of commits and keep them in a consistent order (starting by the glLoadIdentity example you gave to me)!

I definitely understand that it can be simply too hard to rebase when the branches diverge this much (as they often do with such large refactorings). That's why it was only a question/suggestion and not a request.

I like when peoples push each other to provide better code/solutions and I feel guilty for not succeed actually.

In the end, this might not be worth all the trouble. I certainly wouldn't blame you if you didn't want to do this.

I will try! I like the rebase promise, I want to test.

But I thought it was worth providing a concrete example to better explain what I meant when I asked

Do you plan to rebase this pull request once it's tested and ready?

Yes, defenitely! :)

Rebasing is definitely an art that requires a lot of practice (probably 2000 hours :)) I try to rebase/fixup as I go, otherwise it's much more difficult later.

And I think I will learn something trying to.

In any case, I appreciate that you opened it for discussion.

I personally think this refactor was an important move but as it doesn't provide any direct benefit to the end user against a cost ot change everything I'm always concerned about what other mupen64plus devs would think and I didn't want to "push" for such changes without discuss them and be sure I didn't miss something.

I'm sure we can work through any regressions that might come up, and I'm willing to pay that price for code that is easier to understand and maintain in the future.

Once again, I have a "not public yet" project to have a strong regression suite to test Dekstop OGL and OGLES on almost every N64 rom (+potential save states users could provide). It focus on speed (the idea: run multiple instances of mupen64plus at the same time) and I want it do be an helper for potential devs to test their code. Its not finished yet (far from actually). I use it myself but, while it's technically working, it's still a pain to use (lol).

Thanks for all your hard work :+1:

My pleasure! :)

I'm a bit concerned by not having any comment of @richard42 on this. I don't want to "bump" his authority. :(

Narann commented 9 years ago

I will add tag commits. But for now:

To answer @littleguy77:

Which specific commit(s) should I test in mupen64plus-ae?

From my perspective there is no specific commit to test. I try to push code working on every platform. But as you insist I would test this one https://github.com/Narann/mupen64plus-video-rice/commit/705dbcb9958c28532d22fecea0a33dab71b8271e having the Depth Offset config option.

Don't hesitate to tell me if there is a problem. :)

littleguy77 commented 9 years ago

Thanks. I plan to make three testing branches in mupen64plus-ae this week, one that contains this PR (squashed), one that contains @bsmiles32 PR (squashed) and one that contains both. That way @paulscode can just fetch, build, and post to his forum, and let some testers go at it :).

littleguy77 commented 9 years ago

I merged this PR into a test branch in mupen64plus-ae: https://github.com/littleguy77/mupen64plus-ae/commits/test-narann-rice

I've tested with various ROMs but don't get any video output. In Super Mario 64 I get a hard segfault about 10 seconds after restarting.

Any chance you can find an Android device to test on? I can walk you through setting up the mupen64plus-ae build. Once setup, it's easy to work with.

Narann commented 9 years ago

Thanks a lot for testing!

Just to be sure: You are talking about this three latest commits rights?

* 262858d fix  Seg fault when glGetString() retrun NULL #32
* 55a9b1e decrease code size removing duplicate code path in ConvertRGBA32()
* 59be54a decrease code size removing duplicate code path

When you say SM64 work about 10sec, can you see something? Textures are ok?

When you say you don't have any video, is the game running or is it instantly closing? If so, could you provide me the log please? https://github.com/Narann/mupen64plus-video-rice/commit/262858dfb83bfb38aad1cb4ef879ce0137228dc4 try to close the video plugin nicely if OGL is not detected. Maybe there is some returned strings which are NULL, I would be interest to see them. But as you say SM64 work for few seconds I doubt

If it's on SM64 I guess the problem appear in https://github.com/Narann/mupen64plus-video-rice/commit/59be54a4e6563966620a3bddfd3f37827614f59c. So I guess it's in ConvertRGBA16().

Could you compile in debug and use gdb to backtrace? This would point me where the segfault appear?

If not, could you compile moving from one commit to the other separately? If you fall on the first one, I have a couple of possible tests:

In ConvertRGBA16(), replace if (y&1) by if ((y&1)==1). Test,

In ConvertRGBA16(), replace:

uint8 * pByteSrc = (uint8 *)(tinfo.pPhysicalAddress);

To:

uint16 * pSrc = (uint16*)(tinfo.pPhysicalAddress); uint8 * pByteSrc = (uint8 *)pSrc;

Test.

I don't have any Android device but if you could point me the exact commit doing this it would really help me. I think I will have to use valgrind... :(

littleguy77 commented 9 years ago

Same issues in both cases: https://github.com/Narann/mupen64plus-video-rice/commit/705dbcb https://github.com/Narann/mupen64plus-video-rice/commit/262858d

As I said, games run (I hear audio and FPS counter looks accurate), but the screen is black. Happens for the handful of games tested (DK64, DK Racing, SMB 64). SMB 64 additionally crashes with a segfault after 10 seconds, possibly in the core. I'll post the log messages when I get some free time, and enable extended debugging options. Got my hands full with other stuff atm.

richard42 commented 9 years ago

It looks like CLANG is throwing some errors in the travis CI build, like this:

../../src/DeviceBuilder.cpp:161:27: error: 'm_pGraphicsContext' is a protected member of 'CGraphicsContext'

I'm not sure why GCC isn't complaining about the same thing.

Narann commented 9 years ago

Thanks Richard, Clang don't like friends. I will fix this.

richard42 commented 9 years ago

Yeah this is all part of the dark side of C++.

littleguy77 commented 9 years ago

@Narann I just pulled your latest into a mupen64plus-ae branch and the issues remain. No video output, although the crash in SM64 is gone. https://github.com/littleguy77/mupen64plus-ae/commits/test-narann-rice

I'll see what I can do about git-bisecting, but as I said before there's a big swath of commits where the GLES codepath is not buildable. In the worst case, if I have the time I'll see if I can rebase your commits and make sure each builds in GLES.

Narann commented 9 years ago

Hi @littleguy77, happy SM64 crash is gone but I'm not sure the real problem has disappear (so if you have any way to provide a traceback it would be welcome).

About video output. As you guess, I can't reproduce the problem. It can be anything so:

littleguy77 commented 9 years ago

I had to modify some of your code to use the m64p debug callback. printf and cout and friends are of no use on the Android platform because debugging is done remotely using logcat.

Here's the output when I enable OpenGL debugging

01-12 09:47:12.709: I/GameLifecycleHandler(27842): onCreate
01-12 09:47:13.635: I/GameLifecycleHandler(27842): onStart
01-12 09:47:13.635: I/GameLifecycleHandler(27842): onResume
01-12 09:47:13.667: I/Choreographer(27842): Skipped 57 frames!  The application may be doing too much work on its main thread.
01-12 09:47:13.713: I/GameLifecycleHandler(27842): surfaceCreated
01-12 09:47:13.713: I/GameLifecycleHandler(27842): surfaceChanged
01-12 09:47:14.066: I/GameLifecycleHandler(27842): onWindowFocusChanged: true
01-12 09:47:14.066: I/ae-bridge(27842): Loading native libraries
01-12 09:47:14.082: W/linker(27842): libmupen64plus-core.so has text relocations. This is wasting memory and prevents security hardening. Please fix.
01-12 09:47:14.085: I/input-android(27842): init()
01-12 09:47:14.085: I/ae-bridge(27842): Android_JNI_InitImports()
01-12 09:47:14.085: I/SDL(27842): SDL_Android_Init()
01-12 09:47:14.085: I/SDL(27842): SDL_Android_Init() finished!
01-12 09:47:14.085: V/UI-Console(27842):  __  __                         __   _  _   ____  _             
01-12 09:47:14.085: V/UI-Console(27842): |  \/  |_   _ _ __   ___ _ __  / /_ | || | |  _ \| |_   _ ___ 
01-12 09:47:14.085: V/UI-Console(27842): | |\/| | | | | '_ \ / _ \ '_ \| '_ \| || |_| |_) | | | | / __|  
01-12 09:47:14.085: V/UI-Console(27842): | |  | | |_| | |_) |  __/ | | | (_) |__   _|  __/| | |_| \__ \  
01-12 09:47:14.085: V/UI-Console(27842): |_|  |_|\__,_| .__/ \___|_| |_|\___/   |_| |_|   |_|\__,_|___/  
01-12 09:47:14.085: V/UI-Console(27842):              |_|         http://code.google.com/p/mupen64plus/  
01-12 09:47:14.085: V/UI-Console(27842): Mupen64Plus Console User-Interface Version 2.0.0
01-12 09:47:14.086: I/UI-Console(27842): attached to core library 'Mupen64Plus Core' version 2.0.0
01-12 09:47:14.086: I/UI-Console(27842):             Includes support for Dynamic Recompiler.
01-12 09:47:14.523: I/Core(27842): Goodname: Super Mario 64 (U) [!]
01-12 09:47:14.524: I/Core(27842): Name: SUPER MARIO 64      
01-12 09:47:14.524: I/Core(27842): MD5: 20B854B239203BAF6C961B850A4A51A2
01-12 09:47:14.524: I/Core(27842): CRC: 635a2bff 8b022326
01-12 09:47:14.524: I/Core(27842): Imagetype: .z64 (native)
01-12 09:47:14.524: I/Core(27842): Rom size: 8388608 bytes (or 8 Mb or 64 Megabits)
01-12 09:47:14.524: I/Core(27842): Version: 1444
01-12 09:47:14.524: I/Core(27842): Manufacturer: Nintendo
01-12 09:47:14.524: I/Core(27842): Country: USA
01-12 09:47:14.526: D/UI-Console(27842): Cheat codes disabled.
01-12 09:47:14.529: I/UI-Console(27842): using Video plugin: 'Mupen64Plus OpenGL Video Plugin by Rice' v2.0.0
01-12 09:47:14.530: I/UI-Console(27842): using Audio plugin: 'Mupen64Plus SDL Audio Plugin' v2.0.0
01-12 09:47:14.530: I/UI-Console(27842): using Input plugin: 'Mupen64Plus Android Input Plugin' v1.0.0
01-12 09:47:14.531: I/UI-Console(27842): using RSP plugin: 'Hacktarux/Azimer High-Level Emulation RSP Plugin' v2.0.0
01-12 09:47:14.601: I/Video(27842): Disabled SSE processing.
01-12 09:47:14.601: I/Video(27842): Found ROM 'SUPER MARIO 64', CRC ff2b5a632623028b-45
01-12 09:47:14.602: I/Video(27842): Texture loading option is enabled. Finding all hires textures
01-12 09:47:14.602: W/Video(27842): Couldn't open hi-res texture directory: /storage/emulated/0/mupen64plusae-v3-alpha/CoreConfig/UserData/mupen64plus/hires_texture/SUPER MARIO 64/
01-12 09:47:14.602: I/Video(27842): Initializing OpenGL Device Context.
01-12 09:47:14.603: I/Core(27842): Setting 32-bit video mode: 1067x800
01-12 09:47:14.603: I/GameSurface(27842): Creating GL context
01-12 09:47:14.603: V/GameSurface(27842): Found EGL display connection
01-12 09:47:14.603: V/GameSurface(27842): Initialized EGL display connection
01-12 09:47:14.603: V/GameSurface(27842): Found compatible EGL frame buffer configuration
01-12 09:47:14.604: V/GameSurface(27842): Created EGL rendering context
01-12 09:47:14.611: V/GameSurface(27842): Created EGL window surface
01-12 09:47:14.617: V/GameSurface(27842): Bound EGL rendering context to EGL window surface
01-12 09:47:14.617: W/Video(27842): Failed to set GL_SWAP_CONTROL to 0. (it's 32)
01-12 09:47:14.617: I/Video(27842): Using OpenGL: NVIDIA Tegra 3 - OpenGL ES 2.0 14.01003 : NVIDIA Corporation
01-12 09:47:14.637: I/Audio(27842): Initializing SDL audio subsystem...
01-12 09:47:14.648: V/SDL(27842): SDL audio: opening device
01-12 09:47:14.677: I/Core(27842): Starting R4300 emulator: Dynamic Recompiler
01-12 09:47:14.685: I/Core(27842): Init new dynarec
01-12 09:47:15.032: W/art(27842): Native thread exiting without having called DetachCurrentThread (maybe it's going to use a pthread_key_create destructor?): Thread[21,tid=28325,Native,Thread*=0x5b353d60,peer=0x136c5080,"Thread-1959"]
01-12 09:47:15.040: V/SDL(27842): SDL audio: opening device
01-12 09:47:16.194: E/Video(27842): OpenGL Error code 1282 in 'jni/./mupen64plus-video-rice/src/OGLTexture.cpp' line 111
01-12 09:47:16.194: E/Video(27842): OpenGL Error code 1282 in 'jni/./mupen64plus-video-rice/src/OGLTexture.cpp' line 136
01-12 09:47:16.194: E/Video(27842): OpenGL Error code 1282 in 'jni/./mupen64plus-video-rice/src/OGLRender.cpp' line 250
01-12 09:47:16.194: E/Video(27842): OpenGL Error code 1280 in 'jni/./mupen64plus-video-rice/src/OGLRender.cpp' line 924
01-12 09:47:16.194: E/Video(27842): OpenGL Error code 1280 in 'jni/./mupen64plus-video-rice/src/OGLRender.cpp' line 924
01-12 09:47:16.197: E/Video(27842): OpenGL Error code 1282 in 'jni/./mupen64plus-video-rice/src/OGLTexture.cpp' line 111
01-12 09:47:16.197: E/Video(27842): OpenGL Error code 1282 in 'jni/./mupen64plus-video-rice/src/OGLTexture.cpp' line 136
01-12 09:47:16.197: E/Video(27842): OpenGL Error code 1282 in 'jni/./mupen64plus-video-rice/src/OGLRender.cpp' line 250
01-12 09:47:16.200: E/Video(27842): OpenGL Error code 1282 in 'jni/./mupen64plus-video-rice/src/OGLTexture.cpp' line 111
01-12 09:47:16.200: E/Video(27842): OpenGL Error code 1282 in 'jni/./mupen64plus-video-rice/src/OGLTexture.cpp' line 136
01-12 09:47:16.201: E/Video(27842): OpenGL Error code 1282 in 'jni/./mupen64plus-video-rice/src/OGLRender.cpp' line 256
01-12 09:47:16.201: E/Video(27842): OpenGL Error code 1282 in 'jni/./mupen64plus-video-rice/src/OGLTexture.cpp' line 111
01-12 09:47:16.201: E/Video(27842): OpenGL Error code 1282 in 'jni/./mupen64plus-video-rice/src/OGLTexture.cpp' line 136
01-12 09:47:16.201: E/Video(27842): OpenGL Error code 1282 in 'jni/./mupen64plus-video-rice/src/OGLRender.cpp' line 256
01-12 09:47:19.752: I/GameLifecycleHandler(27842): onWindowFocusChanged: false
01-12 09:47:20.888: E/Video(27842): OpenGL Error code 1282 in 'jni/./mupen64plus-video-rice/src/OGLTexture.cpp' line 111
01-12 09:47:20.888: E/Video(27842): OpenGL Error code 1282 in 'jni/./mupen64plus-video-rice/src/OGLTexture.cpp' line 136
01-12 09:47:20.888: E/Video(27842): OpenGL Error code 1282 in 'jni/./mupen64plus-video-rice/src/OGLTexture.cpp' line 136
01-12 09:47:20.889: E/Video(27842): OpenGL Error code 1282 in 'jni/./mupen64plus-video-rice/src/OGLTexture.cpp' line 136
01-12 09:47:20.889: E/Video(27842): OpenGL Error code 1282 in 'jni/./mupen64plus-video-rice/src/OGLRender.cpp' line 256
01-12 09:47:20.898: E/Video(27842): OpenGL Error code 1282 in 'jni/./mupen64plus-video-rice/src/OGLTexture.cpp' line 111
01-12 09:47:20.898: E/Video(27842): OpenGL Error code 1282 in 'jni/./mupen64plus-video-rice/src/OGLTexture.cpp' line 136
01-12 09:47:20.898: E/Video(27842): OpenGL Error code 1282 in 'jni/./mupen64plus-video-rice/src/OGLTexture.cpp' line 136
01-12 09:47:20.898: E/Video(27842): OpenGL Error code 1282 in 'jni/./mupen64plus-video-rice/src/OGLTexture.cpp' line 136
01-12 09:47:20.898: E/Video(27842): OpenGL Error code 1282 in 'jni/./mupen64plus-video-rice/src/OGLRender.cpp' line 256
01-12 09:47:20.899: E/Video(27842): OpenGL Error code 1282 in 'jni/./mupen64plus-video-rice/src/OGLTexture.cpp' line 111
01-12 09:47:20.899: E/Video(27842): OpenGL Error code 1282 in 'jni/./mupen64plus-video-rice/src/OGLTexture.cpp' line 136
01-12 09:47:20.899: E/Video(27842): OpenGL Error code 1282 in 'jni/./mupen64plus-video-rice/src/OGLTexture.cpp' line 136
01-12 09:47:20.899: E/Video(27842): OpenGL Error code 1282 in 'jni/./mupen64plus-video-rice/src/OGLTexture.cpp' line 136
01-12 09:47:20.899: E/Video(27842): OpenGL Error code 1282 in 'jni/./mupen64plus-video-rice/src/OGLRender.cpp' line 256
01-12 09:47:20.899: E/Video(27842): OpenGL Error code 1282 in 'jni/./mupen64plus-video-rice/src/OGLTexture.cpp' line 111
01-12 09:47:20.899: E/Video(27842): OpenGL Error code 1282 in 'jni/./mupen64plus-video-rice/src/OGLTexture.cpp' line 136
01-12 09:47:20.899: E/Video(27842): OpenGL Error code 1282 in 'jni/./mupen64plus-video-rice/src/OGLTexture.cpp' line 136
01-12 09:47:20.899: E/Video(27842): OpenGL Error code 1282 in 'jni/./mupen64plus-video-rice/src/OGLTexture.cpp' line 136
01-12 09:47:20.899: E/Video(27842): OpenGL Error code 1282 in 'jni/./mupen64plus-video-rice/src/OGLRender.cpp' line 256
01-12 09:47:20.901: E/Video(27842): OpenGL Error code 1282 in 'jni/./mupen64plus-video-rice/src/OGLTexture.cpp' line 111
01-12 09:47:20.901: E/Video(27842): OpenGL Error code 1282 in 'jni/./mupen64plus-video-rice/src/OGLTexture.cpp' line 136
01-12 09:47:20.901: E/Video(27842): OpenGL Error code 1282 in 'jni/./mupen64plus-video-rice/src/OGLRender.cpp' line 250
01-12 09:47:20.901: E/Video(27842): OpenGL Error code 1282 in 'jni/./mupen64plus-video-rice/src/OGLTexture.cpp' line 111
01-12 09:47:20.902: E/Video(27842): OpenGL Error code 1282 in 'jni/./mupen64plus-video-rice/src/OGLTexture.cpp' line 136
01-12 09:47:20.902: E/Video(27842): OpenGL Error code 1282 in 'jni/./mupen64plus-video-rice/src/OGLRender.cpp' line 250
01-12 09:47:20.902: E/Video(27842): OpenGL Error code 1282 in 'jni/./mupen64plus-video-rice/src/OGLTexture.cpp' line 111
01-12 09:47:20.902: E/Video(27842): OpenGL Error code 1282 in 'jni/./mupen64plus-video-rice/src/OGLTexture.cpp' line 136
01-12 09:47:20.902: E/Video(27842): OpenGL Error code 1282 in 'jni/./mupen64plus-video-rice/src/OGLRender.cpp' line 250
01-12 09:47:20.902: E/Video(27842): OpenGL Error code 1282 in 'jni/./mupen64plus-video-rice/src/OGLTexture.cpp' line 111
01-12 09:47:20.902: E/Video(27842): OpenGL Error code 1282 in 'jni/./mupen64plus-video-rice/src/OGLTexture.cpp' line 136
01-12 09:47:20.902: E/Video(27842): OpenGL Error code 1282 in 'jni/./mupen64plus-video-rice/src/OGLRender.cpp' line 250
01-12 09:47:20.902: E/Video(27842): OpenGL Error code 1282 in 'jni/./mupen64plus-video-rice/src/OGLTexture.cpp' line 111
01-12 09:47:20.902: E/Video(27842): OpenGL Error code 1282 in 'jni/./mupen64plus-video-rice/src/OGLTexture.cpp' line 136
01-12 09:47:20.902: E/Video(27842): OpenGL Error code 1282 in 'jni/./mupen64plus-video-rice/src/OGLRender.cpp' line 250
01-12 09:47:20.905: E/Video(27842): OpenGL Error code 1282 in 'jni/./mupen64plus-video-rice/src/OGLTexture.cpp' line 111
01-12 09:47:20.905: E/Video(27842): OpenGL Error code 1282 in 'jni/./mupen64plus-video-rice/src/OGLTexture.cpp' line 136
01-12 09:47:20.905: E/Video(27842): OpenGL Error code 1282 in 'jni/./mupen64plus-video-rice/src/OGLTexture.cpp' line 111
01-12 09:47:20.905: E/Video(27842): OpenGL Error code 1282 in 'jni/./mupen64plus-video-rice/src/OGLTexture.cpp' line 136
01-12 09:47:20.914: E/Video(27842): OpenGL Error code 1282 in 'jni/./mupen64plus-video-rice/src/OGLTexture.cpp' line 111
01-12 09:47:20.914: E/Video(27842): OpenGL Error code 1282 in 'jni/./mupen64plus-video-rice/src/OGLTexture.cpp' line 136
01-12 09:47:20.932: E/Video(27842): OpenGL Error code 1282 in 'jni/./mupen64plus-video-rice/src/OGLTexture.cpp' line 111
01-12 09:47:20.932: E/Video(27842): OpenGL Error code 1282 in 'jni/./mupen64plus-video-rice/src/OGLTexture.cpp' line 136
01-12 09:47:20.932: E/Video(27842): OpenGL Error code 1282 in 'jni/./mupen64plus-video-rice/src/OGLRender.cpp' line 250
01-12 09:47:20.955: E/Video(27842): OpenGL Error code 1282 in 'jni/./mupen64plus-video-rice/src/OGLTexture.cpp' line 111
01-12 09:47:20.955: E/Video(27842): OpenGL Error code 1282 in 'jni/./mupen64plus-video-rice/src/OGLTexture.cpp' line 136
01-12 09:47:20.955: E/Video(27842): OpenGL Error code 1282 in 'jni/./mupen64plus-video-rice/src/OGLRender.cpp' line 250
01-12 09:47:20.956: E/Video(27842): OpenGL Error code 1282 in 'jni/./mupen64plus-video-rice/src/OGLTexture.cpp' line 111
01-12 09:47:20.956: E/Video(27842): OpenGL Error code 1282 in 'jni/./mupen64plus-video-rice/src/OGLTexture.cpp' line 136
01-12 09:47:20.956: E/Video(27842): OpenGL Error code 1282 in 'jni/./mupen64plus-video-rice/src/OGLRender.cpp' line 250
01-12 09:47:20.983: E/Video(27842): OpenGL Error code 1282 in 'jni/./mupen64plus-video-rice/src/OGLTexture.cpp' line 111
01-12 09:47:20.983: E/Video(27842): OpenGL Error code 1282 in 'jni/./mupen64plus-video-rice/src/OGLTexture.cpp' line 136
01-12 09:47:20.983: E/Video(27842): OpenGL Error code 1282 in 'jni/./mupen64plus-video-rice/src/OGLRender.cpp' line 250
01-12 09:47:21.003: E/Video(27842): OpenGL Error code 1282 in 'jni/./mupen64plus-video-rice/src/OGLTexture.cpp' line 111
01-12 09:47:21.003: E/Video(27842): OpenGL Error code 1282 in 'jni/./mupen64plus-video-rice/src/OGLTexture.cpp' line 136
01-12 09:47:21.003: E/Video(27842): OpenGL Error code 1282 in 'jni/./mupen64plus-video-rice/src/OGLRender.cpp' line 250
01-12 09:47:21.005: E/Video(27842): OpenGL Error code 1282 in 'jni/./mupen64plus-video-rice/src/OGLTexture.cpp' line 111
01-12 09:47:21.005: E/Video(27842): OpenGL Error code 1282 in 'jni/./mupen64plus-video-rice/src/OGLTexture.cpp' line 136
01-12 09:47:21.005: E/Video(27842): OpenGL Error code 1282 in 'jni/./mupen64plus-video-rice/src/OGLRender.cpp' line 250
01-12 09:47:21.029: E/Video(27842): OpenGL Error code 1282 in 'jni/./mupen64plus-video-rice/src/OGLTexture.cpp' line 111
01-12 09:47:21.030: E/Video(27842): OpenGL Error code 1282 in 'jni/./mupen64plus-video-rice/src/OGLTexture.cpp' line 136
01-12 09:47:21.030: E/Video(27842): OpenGL Error code 1282 in 'jni/./mupen64plus-video-rice/src/OGLRender.cpp' line 250
01-12 09:47:21.051: E/Video(27842): OpenGL Error code 1282 in 'jni/./mupen64plus-video-rice/src/OGLTexture.cpp' line 111
01-12 09:47:21.051: E/Video(27842): OpenGL Error code 1282 in 'jni/./mupen64plus-video-rice/src/OGLTexture.cpp' line 136
01-12 09:47:21.051: E/Video(27842): OpenGL Error code 1282 in 'jni/./mupen64plus-video-rice/src/OGLRender.cpp' line 250
01-12 09:47:21.076: E/Video(27842): OpenGL Error code 1282 in 'jni/./mupen64plus-video-rice/src/OGLTexture.cpp' line 111
01-12 09:47:21.076: E/Video(27842): OpenGL Error code 1282 in 'jni/./mupen64plus-video-rice/src/OGLTexture.cpp' line 136
01-12 09:47:21.076: E/Video(27842): OpenGL Error code 1282 in 'jni/./mupen64plus-video-rice/src/OGLRender.cpp' line 250
01-12 09:47:21.719: E/Video(27842): OpenGL Error code 1282 in 'jni/./mupen64plus-video-rice/src/OGLTexture.cpp' line 111
01-12 09:47:21.719: E/Video(27842): OpenGL Error code 1282 in 'jni/./mupen64plus-video-rice/src/OGLTexture.cpp' line 136
01-12 09:47:21.719: E/Video(27842): OpenGL Error code 1282 in 'jni/./mupen64plus-video-rice/src/OGLRender.cpp' line 256
01-12 09:47:21.719: E/Video(27842): OpenGL Error code 1280 in 'jni/./mupen64plus-video-rice/src/OGLRender.cpp' line 924
01-12 09:47:21.719: E/Video(27842): OpenGL Error code 1280 in 'jni/./mupen64plus-video-rice/src/OGLRender.cpp' line 924
01-12 09:47:21.719: E/Video(27842): OpenGL Error code 1280 in 'jni/./mupen64plus-video-rice/src/OGLRender.cpp' line 924
01-12 09:47:21.719: E/Video(27842): OpenGL Error code 1280 in 'jni/./mupen64plus-video-rice/src/OGLRender.cpp' line 924
01-12 09:47:21.719: E/Video(27842): OpenGL Error code 1282 in 'jni/./mupen64plus-video-rice/src/OGLTexture.cpp' line 111
01-12 09:47:21.719: E/Video(27842): OpenGL Error code 1282 in 'jni/./mupen64plus-video-rice/src/OGLTexture.cpp' line 136
01-12 09:47:21.719: E/Video(27842): OpenGL Error code 1282 in 'jni/./mupen64plus-video-rice/src/OGLRender.cpp' line 256
01-12 09:47:21.719: E/Video(27842): OpenGL Error code 1280 in 'jni/./mupen64plus-video-rice/src/OGLRender.cpp' line 924
01-12 09:47:21.719: E/Video(27842): OpenGL Error code 1280 in 'jni/./mupen64plus-video-rice/src/OGLRender.cpp' line 924
01-12 09:47:21.719: E/Video(27842): OpenGL Error code 1280 in 'jni/./mupen64plus-video-rice/src/OGLRender.cpp' line 924
01-12 09:47:21.719: E/Video(27842): OpenGL Error code 1280 in 'jni/./mupen64plus-video-rice/src/OGLRender.cpp' line 924
01-12 09:47:21.720: E/Video(27842): OpenGL Error code 1282 in 'jni/./mupen64plus-video-rice/src/OGLTexture.cpp' line 111
01-12 09:47:21.720: E/Video(27842): OpenGL Error code 1282 in 'jni/./mupen64plus-video-rice/src/OGLTexture.cpp' line 136
01-12 09:47:21.720: E/Video(27842): OpenGL Error code 1282 in 'jni/./mupen64plus-video-rice/src/OGLRender.cpp' line 256
01-12 09:47:21.720: E/Video(27842): OpenGL Error code 1280 in 'jni/./mupen64plus-video-rice/src/OGLRender.cpp' line 924
01-12 09:47:21.720: E/Video(27842): OpenGL Error code 1280 in 'jni/./mupen64plus-video-rice/src/OGLRender.cpp' line 924
01-12 09:47:21.720: E/Video(27842): OpenGL Error code 1280 in 'jni/./mupen64plus-video-rice/src/OGLRender.cpp' line 924
01-12 09:47:21.720: E/Video(27842): OpenGL Error code 1280 in 'jni/./mupen64plus-video-rice/src/OGLRender.cpp' line 924
01-12 09:47:21.720: E/Video(27842): OpenGL Error code 1282 in 'jni/./mupen64plus-video-rice/src/OGLTexture.cpp' line 111
01-12 09:47:21.720: E/Video(27842): OpenGL Error code 1282 in 'jni/./mupen64plus-video-rice/src/OGLTexture.cpp' line 136
01-12 09:47:21.720: E/Video(27842): OpenGL Error code 1282 in 'jni/./mupen64plus-video-rice/src/OGLRender.cpp' line 256
01-12 09:47:21.720: E/Video(27842): OpenGL Error code 1280 in 'jni/./mupen64plus-video-rice/src/OGLRender.cpp' line 924
01-12 09:47:21.720: E/Video(27842): OpenGL Error code 1280 in 'jni/./mupen64plus-video-rice/src/OGLRender.cpp' line 924
01-12 09:47:21.720: E/Video(27842): OpenGL Error code 1280 in 'jni/./mupen64plus-video-rice/src/OGLRender.cpp' line 924
01-12 09:47:21.720: E/Video(27842): OpenGL Error code 1280 in 'jni/./mupen64plus-video-rice/src/OGLRender.cpp' line 924
01-12 09:47:21.720: E/Video(27842): OpenGL Error code 1280 in 'jni/./mupen64plus-video-rice/src/OGLRender.cpp' line 924
01-12 09:47:21.720: E/Video(27842): OpenGL Error code 1280 in 'jni/./mupen64plus-video-rice/src/OGLRender.cpp' line 924
01-12 09:47:21.720: E/Video(27842): OpenGL Error code 1280 in 'jni/./mupen64plus-video-rice/src/OGLRender.cpp' line 924
01-12 09:47:21.720: E/Video(27842): OpenGL Error code 1280 in 'jni/./mupen64plus-video-rice/src/OGLRender.cpp' line 924
01-12 09:47:21.720: E/Video(27842): OpenGL Error code 1280 in 'jni/./mupen64plus-video-rice/src/OGLRender.cpp' line 924
01-12 09:47:21.720: E/Video(27842): OpenGL Error code 1280 in 'jni/./mupen64plus-video-rice/src/OGLRender.cpp' line 924
01-12 09:47:21.720: E/Video(27842): OpenGL Error code 1280 in 'jni/./mupen64plus-video-rice/src/OGLRender.cpp' line 924
01-12 09:47:21.720: E/Video(27842): OpenGL Error code 1280 in 'jni/./mupen64plus-video-rice/src/OGLRender.cpp' line 924
01-12 09:47:21.721: E/Video(27842): OpenGL Error code 1282 in 'jni/./mupen64plus-video-rice/src/OGLTexture.cpp' line 111
01-12 09:47:21.721: E/Video(27842): OpenGL Error code 1282 in 'jni/./mupen64plus-video-rice/src/OGLTexture.cpp' line 136
01-12 09:47:21.721: E/Video(27842): OpenGL Error code 1282 in 'jni/./mupen64plus-video-rice/src/OGLRender.cpp' line 256
01-12 09:47:21.721: E/Video(27842): OpenGL Error code 1280 in 'jni/./mupen64plus-video-rice/src/OGLRender.cpp' line 924
01-12 09:47:21.721: E/Video(27842): OpenGL Error code 1280 in 'jni/./mupen64plus-video-rice/src/OGLRender.cpp' line 924
01-12 09:47:21.721: E/Video(27842): OpenGL Error code 1280 in 'jni/./mupen64plus-video-rice/src/OGLRender.cpp' line 924
01-12 09:47:21.721: E/Video(27842): OpenGL Error code 1280 in 'jni/./mupen64plus-video-rice/src/OGLRender.cpp' line 924
01-12 09:47:21.721: E/Video(27842): OpenGL Error code 1282 in 'jni/./mupen64plus-video-rice/src/OGLTexture.cpp' line 111
01-12 09:47:21.721: E/Video(27842): OpenGL Error code 1282 in 'jni/./mupen64plus-video-rice/src/OGLTexture.cpp' line 136
01-12 09:47:21.721: E/Video(27842): OpenGL Error code 1282 in 'jni/./mupen64plus-video-rice/src/OGLRender.cpp' line 256
01-12 09:47:21.721: E/Video(27842): OpenGL Error code 1280 in 'jni/./mupen64plus-video-rice/src/OGLRender.cpp' line 924
... snip ...
01-12 09:47:22.072: E/Video(27842): OpenGL Error code 1280 in 'jni/./mupen64plus-video-rice/src/OGLRender.cpp' line 924
01-12 09:47:22.074: W/InputEventReceiver(27842): Attempted to finish an input event but the input event receiver has already been disposed.
01-12 09:47:22.074: I/GameLifecycleHandler(27842): onPause
01-12 09:47:22.074: D/Core(27842): Emulation paused.
01-12 09:47:22.863: I/Choreographer(27842): Skipped 46 frames!  The application may be doing too much work on its main thread.
01-12 09:47:22.976: I/GameLifecycleHandler(27842): surfaceDestroyed
01-12 09:47:22.976: D/Core(27842): Stopping emulation.
01-12 09:47:24.771: D/Core(27842): Saved state to: yyyy-mm-dd-hh-mm-ss.sav
01-12 09:47:24.809: I/Core(27842): R4300 emulator finished.
01-12 09:47:24.820: W/art(27842): Native thread exiting without having called DetachCurrentThread (maybe it's going to use a pthread_key_create destructor?): Thread[20,tid=28327,Native,Thread*=0x61181008,peer=0x1369a0e0,"Thread-1960"]
01-12 09:47:24.822: W/art(27842): Native thread exiting without having called DetachCurrentThread (maybe it's going to use a pthread_key_create destructor?): Thread[19,tid=28323,Native,Thread*=0x6129cd78,peer=0x13685080,"Thread-1957"]
01-12 09:47:24.823: I/GameSurface(27842): Destroying GL context
01-12 09:47:24.823: V/GameSurface(27842): Unbound EGL rendering context from EGL window surface
01-12 09:47:24.832: V/GameSurface(27842): Destroyed EGL window surface
01-12 09:47:24.833: V/GameSurface(27842): Destroyed EGL rendering context
01-12 09:47:24.833: V/GameSurface(27842): Terminated EGL display connection
01-12 09:47:24.837: D/Core(27842): Rom closed.
01-12 09:47:24.844: I/ae-bridge(27842): Unloading native libraries
01-12 09:47:25.141: I/GameLifecycleHandler(27842): onStop
01-12 09:47:25.141: I/GameLifecycleHandler(27842): onDestroy
littleguy77 commented 9 years ago

I'm going to try to rebase this tonight onto the tip of master. We really need the ability to git-bisect easily. Then I'll check it one commit at a time on GLES/Android.

littleguy77 commented 9 years ago

Sigh. The very first commit does not build for GLES. https://github.com/Narann/mupen64plus-video-rice/commit/2dbff149a58317bfaf9bf39ea68f6206f02911e6

Narann commented 9 years ago

Sorry for delay:

Thanks a lot! I see a many potential problems but it sounds very easy to fix (I just need to take some time on each of them).

One of them is here.

glDisable/Enable GL_TEXTURE_2D is not supported by OpenGL ES and is also useless in the way current code work.

But I can't remove this function without potentially break something as this function also call glActiveTexture(). I need to follow the active texture state when ever EnableTexUnit is called...

The GL_INVALID_OPERATION at line 250 is a mistery for me... OpenGL ES does not specify any GL_INVALID_OPERATION error for glDepthFunc. I will maybe find a hint in the OpenGL 2 specs.

And there is others weird ones I will investigate (Look like Texture::EndUpdate() is called before Texture::StartUpdate()... Weird).

Thanks for report. I will dig into this and keep you in touch.

About the commit you point, I was sure it was this one. There is a point where no "tiny" commits can do the thing. The commit you point is THE refactoring commit, all ones before are preparation, all other after are fixes and cleanup.

littleguy77 commented 9 years ago

Ok, I'm pretty much hitting one wall after another. I can't get beyond the second commit in this PR when compiled for a GLES platform. Here's what I did:

I made a new branch in mupen64plus-ae and overwrote the current video-rice code with each commit in this PR, one by one. No merging or anything, just a straight overwrite: https://github.com/littleguy77/mupen64plus-ae/commits/raw-narann-rice https://github.com/littleguy77/mupen64plus-ae/commits/raw-narann-rice?page=2

In an ideal world, each of these commits should build, as long as the Android makefile is updated to match changes to the Unix makefile.

So I started a new branch where I could perform the following cycle

I was hoping to get at least 10 or 20 commits fixed up this way, and perhaps understand the nature of the changes. But unfortunately I'm already stuck on the second commit (of 70) in this PR. Here was my progress: https://github.com/littleguy77/mupen64plus-ae/commits/test-narann-rice-buildup

Removing OGLExtCombiner in the second commit of this PR breaks all sorts of things in OGLES2FragmentShaders.cpp, and I have no idea how to proceed.

I understand that this is a big refactoring, and that sometimes you have no choice but to break the build for a commit or two. But from what I understand, the GLES codepath was not evolved concurrently with the GL codepath. OGLES2FragmentShaders.cpp is left untouched throughout the refactoring, except for a few variable/function renames. Then at the end of the PR, that file is simply removed altogether. Presumably the GL implementation absorbed it, but it's not clear when or where (or over how many commits). So there's no continuity for the GLES implementation. It just suddenly changes, with no way to trace its evolution.

I know you have put a TON of work into this, and I like how you have taken charge of a lot of long-standing issues on this project. I truly appreciate it. What I fear, however, is that the Android users will discover all sorts of regressions, and we'll simply have no efficient way to identify and resolve them. I fear it will be like starting all over again and we'll have a storm of furious users knocking on our door :(

Narann commented 9 years ago

Thanks for all this investigations.

As you notice, OpenGL and OpenGL ES code path where totally different from a combiner perceptive. It was two classes doing things in two different ways (notice it's the OpenGL ES code path which actually inspired me). The plan was: Make the code OpenGL 2.1 compliant and OpenGL ES 2 shouldn't be too difficult. So I choose to leave both "old OpenGL" and "old OpenGL ES" to write a "new OpenGL 2.1" from scratch. When the big refactoring start, I focus on make the "new OpenGL 2.1" working removing a lot of fixed pipeline stuff both "old OpenGL" and "old OpenGL ES" rely on. I'm actually quite surprise you where able to compile after the "big commit refactoring". OGLES2FragmentShaders class is still there but has been removed slowly (like a lot of things). At this point, from a code perspective, both "old OpenGL" and "old OpenGL ES" are disconnected from the rest and removed slowly.

I removed four level of inheritance of OpenGL classes. Many times during the process, I was to this to tell to Richard how impossible it was to refactor without breaking everything and create a(nother) fork(-for-hell). But there is enough forks on the N64 scene to don't add another one to the place so I pushed as much as possible to at least don't break the compilation.

At this point the only solution is to see what's wrong in the OpenGL ES code path and your log is valuable. It point some problems I will investigate one by one.

What I fear, however, is that the Android users will discover all sorts of regressions, and we'll simply have no efficient way to identify and resolve them. I fear it will be like starting all over again and we'll have a storm of furious users knocking on our door :(

If the current code is not ready I will not merge. The fact OpenGL ES give black result worry me: I spend my vacations trying to set a OpenGL ES env dev and fail. That's why I often call for devs used to OpenGL ES to provide feedback. For now the only valuable feedbacks was you, specially the OpenGL error log. I now have places to dig in. Let's fix problems one after the other and make the OpenGL ES code working.

littleguy77 commented 9 years ago

Thanks for the thoughtful reply. So it sounds like you have implemented mupen64plus-video-ricemk2 :P.

It's been a very long time since @Metricity wrote the GLES port, but if I recall correctly, he wrote and tested the GLES codepath on a desktop PC. He might have even been linking against OpenGL rather than OpenGLES, since as you point out GLES code is compatible with higher versions of OpenGL.

Have you ever tried doing that? You would have to mess around with the makefile and build flags quite a bit but it would be interesting to test. Or you could dredge up the original GLES port before it was pulled upstream, around this commit: https://github.com/mupen64plus-ae/mupen64plus-ae/commit/2f559f113ee448e197436a8bd5554ac197b4aa09

Now that downstream is fully synced, it should be a lot easier for Android folks to test upstream branches. I'll probably write some more pull scripts downstream to make the process really easy. And, with the sync done I have more time to help ;)

Narann commented 9 years ago

I've removed some function causing errors with OpenGL ES 2. Please update your branch to my latest commit.

Next, could you hard code those lines:

https://github.com/littleguy77/mupen64plus-ae/blob/test-narann-rice/jni/mupen64plus-video-rice/src/OGLTexture.cpp#L67

To this:

m_glFmt = GL_BGRA_EXT;
m_glType = GL_UNSIGNED_BYTE;

Those are ones used in old OpenGL ES code path:

https://github.com/mupen64plus/mupen64plus-video-rice/blob/master/src/OGLTexture.cpp#L136

I've added two OPENGL_CHECK_ERRORS; in my code here and here:

https://github.com/littleguy77/mupen64plus-ae/blob/test-narann-rice/jni/mupen64plus-video-rice/src/OGLTexture.cpp#L78 https://github.com/littleguy77/mupen64plus-ae/blob/test-narann-rice/jni/mupen64plus-video-rice/src/OGLTexture.cpp#L79

Because I suspect it fail on glTexImage2D() call.

If it doesn't work, could you try with this:

m_glFmt = GL_RGBA;
m_glType = GL_UNSIGNED_BYTE;

It's fully OpenGL ES 2 compliant (Actually, OpenGL ES 2 requiere internal format and format to be the same but it's weird it was working before while it was not the case...).

For the two GL_INVALID_OPERATION here and here I really don't see where it come from... GL_INVALID_OPERATION is not supposed to happen on OpenGL ES 2 for glDepthFunc... It seems to be related to depth buffer.

I see your log doesn't seems to have problem with current default depth buffer value (16). On my desktop it write this:

Video Warning: Failed to set GL_DEPTH_SIZE to 16. (it's 24)

So my desktop force GL_DEPTH_SIZE to be 24 but maybe not your. Could you try to change mupen64plus.cfg to this:

# Z-buffer depth (only 16 or 32)
OpenGLDepthBufferSetting = 24

But from what I read, Tegra 3 is 16 bits buffer only. I'm not sure it's related to this actually.

Could you disable the two glDepthFunc and try again to see if there is still some errors.

Sorry for all this troubles. I will try to have the OpenGL ES code path running on OpenGL.

PS: Nvidia Shield is damn sexy but such expensive. :'(

littleguy77 commented 9 years ago

Most mobile hardware seems to be limited to 16 bit depth from what I understand. It has never been an issue on Android. I'll follow through on your requests when I get some time.

littleguy77 commented 9 years ago

Sorry for the slow turnaround. Same problem. No video output, though the sound, input, etc. work fine. Here are the error messages (they are repeated hundreds of times in the log):

01-20 19:43:58.471: E/Video(19723): OpenGL Error code 1282 in 'jni/./mupen64plus-video-rice/src/OGLTexture.cpp' line 80
01-20 19:43:58.471: E/Video(19723): OpenGL Error code 1282 in 'jni/./mupen64plus-video-rice/src/OGLTexture.cpp' line 138

Silly me... I forgot to read your instructions. Trying now...

littleguy77 commented 9 years ago

I tried this

    m_glFmt = GL_BGRA_EXT;
    m_glType = GL_UNSIGNED_BYTE;

Didn't fix the black screen, same errors in log

Then I tried this

    m_glFmt = GL_RGBA;
    m_glType = GL_UNSIGNED_BYTE;

Didn't fix the black screen, but it got rid of the OpenGL errors. No errors whatsoever in the log :+1:

I'll take a peek at the depth buffer stuff next.

One thing that threw me more than once was #ifndef USE_GLES which my mind always wanted to read as #ifdef USE_GLES. At first I was modifying the wrong parts of the code... could be a source of insidious bugs.

littleguy77 commented 9 years ago

Setting depth buffer to anything other than 16-bit causes failures in context creation. So that's a red herring.

Edit: Removing the calls to glDepthFunc made no noticeable difference.

Narann commented 9 years ago

Thanks for your tests. Can you confirm the repeated error messages are always the same (same line numbers actually)? This would mean my tiny fixes had at fix some stuff.

The fact to can run without OpenGL errors hardcoding GL_RGBA is great! this would mean the OpenGL ES extension detection doesn't work. This also confirm the problem is not texture related. I will find other places where I could call gl* functions without the OpenGL error check. There is no reason for OpenGL to not raise error if everything is black.

Thanks for the depth buffer test...

FYI: Since few weeks I'm trying to compile Rice for my rpi. I have hard time and run around a lot of (SDL) issues but I'm making progress. I would really love to see Rice working on it (it was part of the original plan). I would really love to reproduce the problem and use apitrace to introspect what is happening on the OpenGL ES codepath. As I know your using Eclipse on NDK, have you tried OpenGL ES Tracer? It seems that generated gl trace file is browsable using apitrace. If so, I would love to get your generated file to potentially reproduce the problem.

EDIT: Oh! And I just found this. Do you think I could use an Android VM to debug? Have you any experience with this?

littleguy77 commented 9 years ago

As of this commit, I get this log.

As of this commit I get no errors in the log.

Actually, I think your extension check is working, since my device supports GL_BGRA_EXT.

m_glFmt = COGLGraphicsContext::Get()->IsSupportTextureFormatBRGA() ? GL_BGRA_EXT : GL_RGBA;

When I hard code GL_BGRA_EXT I get the same errors in the logcat. Only when I hard-code GL_RGBA do the errors disappear. Perhaps a clue?

littleguy77 commented 9 years ago

One thing I noticed, when I use glide64mk2, or vanilla rice, my core logs this:

I/Core(11052): Setting video mode: 800x600

Whereas when I use your refactored version of rice, my core logs this:

I/Core(11052): Setting 32-bit video mode: 800x600

I never see the 32-bit text normally. Perhaps there's an EGL context creation issue. I'll grep through stuff to see if I can isolate anything.

Narann commented 9 years ago

So you say your device support GL_BGRA_EXT but when you hard code GL_BGRA_EXT, OpenGL raise some errors... Weird...

About the Setting 32-bit video mode, very interesting I guess it's Android printing this? The only places I found something related to print video mode is here, here and here. Your 32bits word is surprising.

littleguy77 commented 9 years ago

Ok, I'm getting video now :+1:

It does indeed appear to be related to context creation mode, very well could be a bug in our java code. When I add this hack, everything seems to work fine, so far! https://github.com/littleguy77/mupen64plus-ae/commit/75b9aa04625775375c5671b8104068cec55412af

~~BTW I had to use a regex search for Setting .* video mode... the 32-bit part is from a variable ;) core -- vidext.c line 239~~

Narann commented 9 years ago

You see Mario jumping and everything? Awesome! Are the color corrects? If not, I would push further: What happen if you Hardcode GL_BGRA_EXT. Do you still see textures? If you see the textures, what happen if you put back my "extension check"? :)

littleguy77 commented 9 years ago

BLarghhh. I mis-spoke :frowning: I was accidentally using glide when I thought it was working. How embarassing. I checked vanilla rice and it produces the same 32-bit info message, so that is not likely the problem area.

Back to square 1.

Narann commented 9 years ago

T_T Ok, I will go back to my raspberry pi stuff. Please let's continue this discuss in the #41 it woul be more appropriate.

gizmo98 commented 9 years ago

@Narann. Your current source is not crashing anymore on a pi2. I can start mario64 for example (i can hear it). But the screen stays black. After exit there is a message:

Compile shader failed:
Shader type: Fragment
Info log:

GLSL code:
#version 100
precision lowp float; 
uniform vec4 uBlendColor;   
uniform vec4 uPrimColor;      
uniform vec4 uEnvColor;       
uniform vec3 uChromaKeyCenter;             
uniform vec3 uChromaKeyScale;          
uniform vec3 uChromaKeyWidth;            
uniform float uLodFrac;                   
uniform float uPrimLodFrac;                     
uniform float uK5;                       
uniform float uK4;                        
uniform sampler2D uTex0;                 
uniform sampler2D uTex1;                 
uniform vec4 uFogColor;                   

varying vec2 vertexTexCoord0;                        
varying vec2 vertexTexCoord1;                        
varying float vertexFog;                          
varying vec4 vertexShadeColor;                 

void main()                                        
{                                                        
vec4 outColor;                                  
vec3 AColor = uPrimColor.rgb;
vec3 BColor = vertexShadeColor.rgb;
vec3 CColor = texture2D(uTex0,vertexTexCoord0).rgb;
vec3 DColor = vertexShadeColor.rgb;
float AAlpha = uPrimColor.a;
float BAlpha = vertexShadeColor.a;
float CAlpha = texture2D(uTex0,vertexTexCoord0).a;
float DAlpha = vertexShadeColor.a;
vec3 cycle1Color = (AColor - BColor) * CColor + DColor;
float cycle1Alpha = (AAlpha - BAlpha) * CAlpha + DAlpha;
outColor.rgb = cycle1Color;
outColor.a = cycle1Alpha;
float coverage = 1.0;
coverage = step( 0.5, coverage );
outColor.a = coverage;
if( coverage < 0.1 ) discard;
   gl_FragColor = outColor;
} 

Program link failed.
Info log:
Nebuleon commented 9 years ago

This is fixed somewhat on GLES2 devices, building with USE_GLES2, as of Narann/mupen64plus-video-rice@02f595f644213a277a0ec724ae7cba5a8b54fb65 (in fix_gles). Super Mario 64 now displays an image that is not fully black after booting, however there are still some errors with shader compilation, and problems with the pixel format (BGR vs. RGB).

Screenshots: sm64-narann-rice-gles2-20150326-1 In which the color of the letters is correct, but not that of their borders, and there are white bars at the top and bottom where there should be black.

sm64-narann-rice-gles2-20150326-2 In which the color of the Super Mario 64 tiles is incorrect.

sm64-narann-rice-gles2-20150326-3 In which Mario's face is correct, but the alpha test and colors for PRESS START are wrong. There are also white bars visible while PRESS START is on the screen, which become black once it disappears.

A trimmed log follows. A shader fails to compile before the Cached Interpreter starts, and another after it starts.

[trimmed 9 lines of mupen64plus logo and startup information]
Core: Goodname: Super Mario 64 (U) [o1]
Core: Name: SUPER MARIO 64      
Core: MD5: AA7BCEC44B21B34D2F6ED5F8646882CE
Core: CRC: 635A2BFF 8B022326
Core: Imagetype: .z64 (native)
Core: Rom size: 8380416 bytes (or 7 Mb or 56 Megabits)
Core: Version: 1444
Core: Manufacturer: Nintendo
Core: Country: USA
[trimmed 14 lines of startup information for other plugins]
Video: Disabled SSE processing.
Video: Found ROM 'SUPER MARIO 64', CRC ff2b5a632623028b-45
Video: Initializing OpenGL Device Context.
Core: Setting 32-bit video mode: 320x240
fbdev_display succesful
Kernel: Vivante GPL kernel driver 4.6.6.1381
Physical address of internal memory: 00000000
* Video memory:
  Internal physical: 0x00000000
  Internal size: 0x00000000
  External physical: 00000000
  External size: 0x00000000
  Contiguous physical: 0x8c7edf80
  Contiguous size: 0x00400000
Succesfully opened device
native_fbdev: 2 buffers of 320x240
Framebuffer format: 6, flip_rb=0
Framebuffer format: 6, flip_rb=0
Video Warning: Failed to set GL_SWAP_CONTROL to 0. (it's 32)
Video: Using OpenGL: Gallium 0.4 on Vivante GC860 rev 4621, Vivante GPL kernel dr - OpenGL ES 2.0 Mesa 9.3.0-devel : etnaviv
Compile shader failed:
Shader type: Fragment
Info log:
0:2(14): error: no precision specified this scope for type `vec4'

GLSL code:
#version 100
uniform vec4 uFillColor;                                   
void main()                                                
{                                                          
gl_FragColor = uFillColor;                                 
}
Program link failed.
Info log:
error: program lacks a fragment shader

Audio: Initializing SDL audio subsystem...
[trimmed 4 lines of Input-SDL failures]
Core: Starting R4300 emulator: Cached Interpreter
Compile shader failed:
Shader type: Fragment
Info log:
0:2(14): error: no precision specified this scope for type `vec4'
0:4(14): error: no precision specified this scope for type `vec2'
0:7(6): error: no precision specified this scope for type `vec4'
0:8(7): error: no precision specified this scope for type `float'

GLSL code:
#version 100
uniform vec4 uBlendColor;                          
uniform sampler2D uTex0;                                   
varying vec2 vertexTexCoord0;                              
void main()                                                
{                                                          
vec4 outColor = texture2D(uTex0,vertexTexCoord0);           
float coverage = 1.0;
if( coverage < 0.1 ) discard;
if( outColor.a < uBlendColor.a ) discard;
    gl_FragColor = outColor;                               
}                                                          

Program link failed.
Info log:
error: program lacks a fragment shader
Nebuleon commented 9 years ago

Fixing the shader errors related to missing precisions in vecN and float data types requires, as you said on IRC, adding lowp before vecN and float. Adding it in at least the lines referenced in the failing shaders allows the alpha test to pass, and the white bars disappear.

Keep up the good work! :D

Narann commented 9 years ago

Thanks a lot for your patience Nebuleon! Now I have an idea where the problem come from.

About texture in BGRA. This is a known issue. I will investigate it as soon as I've found how to deal properly with the pgl vs gl prefix problem.

As @richard42 point me, I will have to deal with rice repo history to know why we had to use other function pointer (pgl) and if we could fix it and rely _only on SDL's one. I've isolate two time step:

https://github.com/mupen64plus/mupen64plus-video-rice/blob/d1dd90e8fe984003261c2a1ddb26ecceacb0a437/src/OGLExtensions.h#L20 https://github.com/mupen64plus/mupen64plus-video-rice/blob/a20f82c2d8af47aa873dca4a7904e3c8f998807c/src/OGLExtensions.h#L30

First is from the original commit:

This is only necessary because Windows does not contain development support for OpenGL versions beyond 1.1

And second seems to blame Linux:

The function pointer types are defined here because as of 2009 some OpenGL drivers under Linux do 'incorrect' things which mess up the SDL_opengl.h header, resulting in no function pointer typedefs at all, and thus compilation errors.

@Richard42 gave some informations on the ML. Also here.

I need to investigate this. I'm surpised about how Linux driver could make SDL_opengl.h messing up.

This fix_gles branch compile fine on my Linux Desktop OpenGL. It seems to compile fine on your GLES2 hardware so now I need to test on Windows and Mac just to be sure. Then I will try to know why those function pointers were needed.

gizmo98 commented 9 years ago

Well done Narann! RPI2 with your fix_gles branch: img_1237 img_1243

Narann commented 9 years ago

Great! I'm very happy to finally see this.

gizmo98 commented 9 years ago

I have three issues with a pi2. Wrong colors, zelda crash after ~10minutes (could be a rpi problem) and broken shading. I thought shading is broken with every gles device because every rice plugin had this issue (mupen64plus-ae, mupen64plus-libretro, old rice plugin and this wip). So i was surprised mario's head is shaded correctly on nebulon's vivante gpu. More surprising I ported mupen64plus-pandora's rice plugin yesterday and the shading is also correct.

gizmo98 commented 9 years ago

Example: mupen64plus-libretro + rice plugin: img_1246

mupen64plus + mupen64plus-pandora rice plugin: img_1245

Narann commented 9 years ago

Thanks:

@Nebuleon: How did you take your screenshots? Are they taken from the device or via device emulator?

gizmo98 commented 9 years ago
  1. Only textures.
  2. Could be a problem with my pi. Does not run overall stable since yesterday (temperature, memory split or configuration).
  3. All rice plugins (mupen64plus/mupen64plus-video-rice; ricrpi/mupen64plus-gles2n64; mupen64plus-libretro; mupen64plus-ae) except of mupen64plus-pandora.
Nebuleon commented 9 years ago

@Narann (Your question was added with an edit, so I didn't see it in the GitHub mail)

My screenshots were taken on the device itself, via fbgrab.

gizmo98 commented 9 years ago

@Narann I replaced ricrpis gles2rice sources with gles2rice pandora sources. You can find the initial commit here: https://github.com/gizmo98/mupen64plus-video-gles2rice/commit/dc48ace24b3e14c25fc88d6afacb490c5061568d

If there is a important difference between rice plugins you should find it there.