Rice code, few questions.

[Dorian FEVRIER]

Hi all!
I have few questions:

I was walking in Rice code and I've seen a part of code that seems useless, I would like to have your advise:
http://pastebin.com/pQiTdwzg
is ClearDeviceObjects() useful?

Second:
in src/OGLRender.cpp:101, there is dirty code to find the clamp to edge extention.

As GL_CLAMP_TO_EDGE is part of OpenGL 1.2 I suggest to don't have all this test and set just this:

OGLXUVFlagMaps[TEXTURE_UV_FLAG_CLAMP].realFlag = GL_CLAMP_TO_EDGE;

So, we don't need m_bSupportClampToEdge anymore.

Same for GL_MIRRORED_REPEAT which is in OpenGL 1.4. I suggest just use:

OGLXUVFlagMaps[TEXTURE_UV_FLAG_MIRROR].realFlag = GL_MIRRORED_REPEAT;

If we do this, a part of the code (few lines) could be merges between OGLES and OGL.

I just wonder if this is truly what we want. I mean, GL_CLAMP and GL_CLAMP_TO_EDGE are two different way to deal with texture.

Can someone confirm GL_CLAMP is used in any way and OGLXUVFlagMaps[TEXTURE_UV_FLAG_CLAMP].realFlag is used only when we want a clamp to the edges? I can't get that...

Same for OGLXUVFlagMaps[TEXTURE_UV_FLAG_MIRROR].realFlag = GL_MIRRORED_REPEAT.

Do we really use it when we want mirrored repeat and nothing else?

If this both flags are now not dynamic (like it was before), is there a valid reason to keep this structure:

UVFlagMap OGLXUVFlagMaps[] =
{
    {TEXTURE_UV_FLAG_WRAP, GL_REPEAT},
    {TEXTURE_UV_FLAG_MIRROR, GL_MIRRORED_REPEAT_ARB},
    {TEXTURE_UV_FLAG_CLAMP, GL_CLAMP},
};

I was looking in the UVFlagMap structure:

typedef struct
{
    TextureUVFlag   N64flag;
    uint32          realFlag;
} UVFlagMap;

N64flag doesn't seems to be used anywhere. and only realFlag seems to be used.

So if we decide there is not multiple possibilities, could we choose to remove this system?

I just try to understand. :)

Another point. There is a lot of work do define functions in OGLExtentions.h and cpp.
A convinient way would be to define GL_GLEXT_PROTOTYPES just before:

#include <SDL_opengl.h>

This way, SDL would include almost every gl function declared in OGLExtentions.
Is there a good reason to not define GL_GLEXT_PROTOTYPES?

A start of an (interesting) answer here: http://stackoverflow.com/questions/17524794/defining-gl-glext-prototypes-vs-getting-function-pointers

http://stackoverflow.com/questions/7532110/vertex-buffer-objects-with-sdl

OGLExtentions.h and cpp still include SDL_opengl.h instead of osal_opengl.h.

Is this normal?

Same, reading this in OGLExtentions.h:

/* 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.
*/

Make me think the commentor wasn't aware of GL_GLEXT_PROTOTYPES. From the SDL_opengl.h, the only thing to do to have function pointers is use GL_GLEXT_PROTOTYPES before.

If we do it well, OGLExtentions.cpp shouldn't be necessary anymore.

I can do all of this in few commits and send a push request if you want.

Thanks in advance! :)

Rice (and certainly others) need a lot of cleanup but this look possible (from my investigations).

A first step would be to remove anything extensions related if the extentions is now in OGL 2 and more.

I've dig a little in OGLFragmentShaders and it is really obsolete. It rely on a very old shader approach. Seems to be hard to update but maybe possible.

For now, I'm slowly replacing old stuff (pgl*, ARB, etc...) by new one (and so, decrease code size) and everything seems to work fine.

[Dorian Fevrier]

And I'm still in the code and would like to know your opinion guys about removing this kind of code:

I mean, this are use in DeviceBuilder to create code like this:

                switch(m_deviceType)
                {
                case OGL_1_1_DEVICE:
                    m_pColorCombiner = new COGLColorCombiner(pRender);
                    DebugMessage(M64MSG_VERBOSE, "OpenGL Combiner: Basic OGL");
                    break;
                case OGL_1_2_DEVICE:
                case OGL_1_3_DEVICE:
                    m_pColorCombiner = new COGLColorCombiner2(pRender);
                    DebugMessage(M64MSG_VERBOSE, "OpenGL Combiner: OGL 1.2/1.3");
                    break;
                case OGL_1_4_DEVICE:
                    m_pColorCombiner = new COGLColorCombiner4(pRender);
                    DebugMessage(M64MSG_VERBOSE, "OpenGL Combiner: OGL 1.4");
                    break;
                case OGL_1_4_V2_DEVICE:
                    m_pColorCombiner = new COGLColorCombiner4v2(pRender);
                    DebugMessage(M64MSG_VERBOSE, "OpenGL Combiner: OGL 1.4 Version 2");
                    break;
                case OGL_TNT2_DEVICE:
                    m_pColorCombiner = new COGLColorCombinerTNT2(pRender);
                    DebugMessage(M64MSG_VERBOSE, "OpenGL Combiner: TNT2");
                    break;
                case NVIDIA_OGL_DEVICE:
                    m_pColorCombiner = new COGLColorCombinerNvidia(pRender);
                    DebugMessage(M64MSG_VERBOSE, "OpenGL Combiner: Nvidia");
                    break;
                case OGL_FRAGMENT_PROGRAM:
                    m_pColorCombiner = new COGL_FragmentProgramCombiner(pRender);
                    DebugMessage(M64MSG_VERBOSE, "OpenGL Combiner: Fragment Program");
                    break;
                 default:
                    break;
                }

Some of them seems broken (OGL_1.1)
As we are now considered fully OpenGL 2 (as Richard asked here) are you ok guys to let me clean this place as if only OGL_FRAGMENT_PROGRAM was avaible (what will wil use in OpenGL 2 and what OpenGL ES 2 rely on)?

Without all of this, we could remove this:

# OpenGL level to support (0=auto, 1=OGL_1.1, 2=OGL_1.2, 3=OGL_1.3, 4=OGL_1.4, 5=OGL_1.4_V2, 6=OGL_TNT2, 7=NVIDIA_OGL, 8=OGL_FRAGMENT_PROGRAM)
OpenGLRenderSetting = 0

And a lot of files (OGLCombinerBlaBla.cpp).

The idea, as it was suggested here, was to remove old legacy code related to OpenGL version < 2 and use a "modern" OpenGL (In a OpenGL 2 constrain of course) code that could look like OpenGL ES 2 code which is already here. Reading the OpenGL ES 2 code, I think Rice should work like this (shader only approach) even for OpenGL 2.

Any feedback appreciate.

--
You received this message because you are subscribed to the Google Groups "mupen64plus" group.
To unsubscribe from this group and stop receiving emails from it, send an email to mupen64plus...@googlegroups.com.
To post to this group, send email to mupen...@googlegroups.com.
Visit this group at http://groups.google.com/group/mupen64plus.
For more options, visit https://groups.google.com/d/optout.

[Richard Goedeken]

Hi Dorian,

Rice has always been a bit of a mess. I haven't looked closely at the
clamping flags that you mentioned, but it wouldn't surprise me if there was
unnecessary and/or unused code and data members in there. Cleaning it up is a
good thing.

Regarding the OGLExtensions.cpp/.h, I recommend leaving it there because this
is the most portable and fool-proof way of linking to the OpenGL functions
across all platforms. In the past we had problems with the SDL headers even
with the GL_GLEXT_PROTOTYPES macro.

Regarding all of the crazy opengl combiner classes, I wouldn't be opposed to
trashing everything except the fragment program combiner. The other ones are
really primitive and don't even support all of the n64 texturing modes.

Richard

> --
> You received this message because you are subscribed to the Google Groups
> "mupen64plus" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to mupen64plus...@googlegroups.com

> <mailto:mupen64plus...@googlegroups.com>.

> To post to this group, send email to mupen...@googlegroups.com

> <mailto:mupen...@googlegroups.com>.