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

Broken compilation with GLES headers #66

Closed fzurita closed 7 years ago

fzurita commented 8 years ago

I just pulled the latest version of this plugin and compiled for Android and there a few issues that didn't exist before.

First issue, in OGLRender.cpp, we have this code:

    if( COGLGraphicsContext::Get()->IsSupportDepthClampNV() )
    {
        glEnable(GL_DEPTH_CLAMP_NV);
        OPENGL_CHECK_ERRORS;
    }

This doesn't compile in Android because of lack of full OpenGL. I was able to get it to compile by adding by #ifdefing it out with USE_GLES.

Next issue, graphics seemed to have stopped working correctly. Here is a screenshot of Mario 64.

super_mario_64-000

I'm not sure which commit caused graphics to do this. It would require some bisecting to find out.

The only thing that has color are untextured polygons it seems.

Narann commented 8 years ago

It could have been better to create two separated issues for this.

1) Could you paste the compilation error message? I guess GL_DEPTH_CLAMP_NV is the problem. 2) I sometime have this graphical issue when I run mupen64plus the first time. Do you also have this issue when you run it multiple time?

fzurita commented 8 years ago

Yeah, I probably should had made two issues

1) Yes, GL_DEPTH_CLAMP_NV is the problem 2) Yep, it's always the same issue, it's never fixed

orbea commented 8 years ago

Does PR https://github.com/mupen64plus/mupen64plus-video-rice/pull/67 fix it for you? It allows me to build here on Slackware64-current and start Orcarina of Time without problems.

fzurita commented 8 years ago

I already attempted that same fix and it didn't work for me.

It was easy to make build by #ifdefing out the offending code when USE_GLES is defined. That unfortunately still leads to the black textures.

orbea commented 8 years ago

Can you paste the build errors with my fix? What platform are you building on?

fzurita commented 8 years ago

I'm building in Android.

I'm sure that your fix doesn't lead to any build errors, but GL_DEPTH_CLAMP_NV doesn't work for GLES 2.0, only for 3.0 and up. I can't remember what when I tried your approach, but I do remember it didn't work.

I'll try your approach again tonight at some point so I can give more details.

orbea commented 8 years ago

Super Mario 64 is pretty broken here with GLES and OpenGL btw, different than your issue though. I will have to post a new issue for it later as its likely not affected by my fix.

orbea commented 8 years ago

You're right, the problem is deeper than a missing define, its not supported by GLES2.

This may provide some clues for a better solution? https://stackoverflow.com/questions/5960757/how-to-emulate-gl-depth-clamp-nv

orbea commented 8 years ago

I found the first bad commit with git bisect.

e34b8d3fda48b615dfdd0ea28fee7a8930f5d880 is the first bad commit
commit e34b8d3fda48b615dfdd0ea28fee7a8930f5d880
Author: Dorian Fevrier <fevrier.dorian@yahoo.fr>
Date:   Fri May 22 00:50:57 2015 -0400

    fix regression on Clay Fighter. This regression seems to have always exists on OpenGL ES code path

:040000 040000 45c312d181f794ca717ae4a02c6c03452b65a477 36ab6a2dde4336a734c8062fe75705b5b1c8e847 M  src

https://github.com/mupen64plus/mupen64plus-video-rice/commit/e34b8d3fda48b615dfdd0ea28fee7a8930f5d880

fzurita commented 8 years ago

Ok, this is amusing. I just recompiled everything after just doing the below quoted code to fix compilation, and everything works fine, the texture issue is gone:

#ifndef USE_GLES
    if( COGLGraphicsContext::Get()->IsSupportDepthClampNV() )
    {
        glEnable(GL_DEPTH_CLAMP_NV);
        OPENGL_CHECK_ERRORS;
    }
#endif

This didn't work 15 days ago, I got the result of the screenshot above. I don't understand... Maybe the above issue doesn't happen every time.

Also, I do remember that quitting and restarting the plugin didn't fix it.

fzurita commented 8 years ago

I just tried a 2nd device just to make sure, and I'm still not getting the issue in the screenshot. Must had been some kind of transient.

orbea commented 8 years ago

Also reading https://www.opengl.org/registry/specs/NV/depth_clamp.txt indicates GL_NV_depth_clamp is part of the OpenGL 1.2.1 specification, not GLES3. If it works on your android device now I guess the fix might be good. Reading the rice issue reports show some issues that are hard to reproduce and do not occur every time and from debugging GLupeN64 I have run into several issues that only happened a few times and not since for w/e reason... Could be something similar here.

fzurita commented 8 years ago

Ok, I was able to recreate the issues above once I switched to an optimized build, the debug build doesn't have this problem.

Edit: This may have nothing to do with doing an optimized build. It may be more related to doing a clean build when I did the optimized build.

orbea commented 8 years ago

Can you try the adding:

#define GL_DEPTH_CLAMP_NV 0x864F

To line 33 here with a clean build and no other changes.

https://github.com/mupen64plus/mupen64plus-video-rice/blob/master/src/osal_opengl.h#L33

fzurita commented 8 years ago

Just tried it, same issue. As in, it compiles fine, but textures are black.

orbea commented 8 years ago

Thanks! It might be potentially unrelated then. If you could it might help to git checkout b14242e8e91f93d89e483e68f20aec915718d7f0 where it should build with GLES, does it still occur there? If not try to git checkout e34b8d3fda48b615dfdd0ea28fee7a8930f5d880 and then applying the above #define GL_DEPTH_CLAMP_NV 0x864F to src/osal_opengl.h to see if that causes it. If there is still no problem you could try using git bisect to find where the black textures where introduced.

Likewise if it does occur try going farther back to see where it was introduced. If you can test an OpenGL platform you could see if the problem is any different on that. I will try doing that for my mario 64 problem with both OpenGL and GLES to see if its related.

orbea commented 8 years ago

I can reproduce the black textures with OpenGL if I go all the way back to commit eb85e422adb2554f9cb631846cddd25891035316. Seems this game has been broken a long time and my PR probably only allows you to reproduce it again. Unless someone has a better idea I would suggest merging my PR and then making a new issue for the texture problems.

fzurita commented 8 years ago

There is already an existing issue for the texture problems, no need to make a new one: https://github.com/mupen64plus/mupen64plus-video-rice/issues/41

It looks like the changes that broke GLES have a long history: https://github.com/mupen64plus/mupen64plus-video-rice/pull/31

In pull request #31, @Narann seems to suggest that it was fixed by modifying the way GL functions were retrieved. This doesn't seem to be working in Android. I guess further talk of the black textured polygons can be moved to #41.

richard42 commented 7 years ago

ok, thanks. I've pushed a fix for the build issues brought up with PR #67