Mipmapping

Sage
Posts: 1,066
Joined: 2004.07
Post: #1
I can't seem to get mipmapping to work with my texture code. Here is my function CreateTexture():
Code:
#include "main.h"
#include "Texturing.h"

signed int CreateTexture2D( GLuint &theTextureId,
                            Uint16 &theTextureWidth,
                            Uint16 &theTextureHeight,
                            GLushort *&theTextureImage,
                            const char *theTextureImagePath )
{
    SDL_Surface *theTextureSurface;
    Uint32      *theTextureSurfacePixels;
    
    if(( theTextureSurface = IMG_Load( theTextureImagePath )) == NULL )
    {
        fprintf( stderr,IMG_GetError());
        return -1;
    }
    
    {
        SDL_Surface *temporarySurface = SDL_DisplayFormat( theTextureSurface );
        if( temporarySurface == NULL )
        {
            fprintf( stderr,
                     IMG_GetError());
            return -1;
        }
        SDL_FreeSurface( theTextureSurface );
        theTextureSurface = temporarySurface;
    }
    
    SDL_LockSurface( theTextureSurface );
    
    theTextureWidth = theTextureSurface->w;
    theTextureHeight = theTextureSurface->h;
    theTextureSurfacePixels = ( Uint32* )theTextureSurface->pixels;
    theTextureImage = ( GLushort* )malloc(( sizeof( GLushort ) * ( theTextureWidth * theTextureHeight )));
    
    {
        Uint16 x,
        y2;
        Sint16 y;
        Uint32 position;
        Uint8 r,
            g,
            b,
            discardA;
        double rPercent,
            gPercent,
            bPercent;
        
        
        
        for( y = ( theTextureHeight - 1 ),
             y2 = 0;
            
             y >= 0;
            
             y --,
             y2 ++ )
        {
            for( x = 0;
                 x < theTextureWidth;
                 x ++ )
            {
                position = ( x + ( theTextureWidth * y ));
                SDL_GetRGBA( theTextureSurfacePixels[ position ],
                             theTextureSurface->format,
                             &r,
                             &g,
                             &b,
                             &discardA );
                rPercent = ((( double )r / 255.0 ) * 100.0 );
                gPercent = ((( double )g / 255.0 ) * 100.0 );
                bPercent = ((( double )b / 255.0 ) * 100.0 );
                r = ( Uint8 )( rPercent * ( 31.0 / 100.0 ));
                g = ( Uint8 )( gPercent * ( 31.0 / 100.0 ));
                b = ( Uint8 )( bPercent * ( 31.0 / 100.0 ));
                position = ( x + ( theTextureWidth * y2 ));
                theTextureImage[ position ] = 0;
                theTextureImage[ position ] |= (( r << 11 )
                                                | ( g << 6 )
                                                | ( b << 1 ));
            }
        }
    }
    
    SDL_UnlockSurface( theTextureSurface );
    SDL_FreeSurface( theTextureSurface );
    
    glGenTextures( 1, &theTextureId );
    glBindTexture( GL_TEXTURE_2D, theTextureId );
    glTexParameteri( GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_NEAREST );
    glTexParameteri( GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_NEAREST );
    glTexImage2D( GL_TEXTURE_2D, 0, GL_RGBA,
                  theTextureWidth, theTextureHeight,
                  0, GL_RGBA, GL_UNSIGNED_SHORT_5_5_5_1,
                  theTextureImage );
    gluBuild2DMipmaps( GL_TEXTURE_2D, 3, theTextureWidth, theTextureHeight, GL_RGB, GL_UNSIGNED_BYTE, theTextureImage);
    
    return 0;
}


When I ran the debugger (as this crashed) I found that there was something wrong with my gluBuild2DMipmaps() function. I've looked in the OpenGL spec file and various other sites telling me about the parameters and I matched them up with mine as well as I could but I must be missing something. Any ideas?
Quote this message in a reply
Sage
Posts: 1,232
Joined: 2002.10
Post: #2
Could you please explain, step by step, what this code is supposed to be doing? This is probably the most convoluted texture loading code I've ever seen.
Quote this message in a reply
Sage
Posts: 1,066
Joined: 2004.07
Post: #3
I actually didn't write this texture code so I'm not sure step by step how it works. The reason I was going for mipmapping was to attempt to fix some bleeding with images that I was having. Somehow (I'm not good with Photoshop so I'm not sure what I did) I fixed the problem messing with the files in Photoshop. Thanks for your help but I don't really need it anymore.
Quote this message in a reply
Sage
Posts: 1,232
Joined: 2002.10
Post: #4
Right, well, since this thread is in a public forum, let's just go ahead and analyze this code for the benefit of any other newbies who might stumble in here and blindly copy it without thinking. Maybe we can preempt a few questions that way.

Here's my interpretation:
* load file into a surface (read from disk, decompress, etc)
* convert the surface pixels into framebuffer format (ignoring alpha)
* lock surface for pixel twiddling
* allocate a 16 bpp work buffer
* iterate through the surface, copying every pixel into the work buffer:
* * flip Y
* * convert (extremely inefficiently) 32 bpp to 16 bpp, zero alpha
* unlock surface and free it
* generate and bind to a 2D texture
* use nearest min & mag filtering for this texture (no mipmaps)
* build 32 bpp RGBA texture from 16 bpp work buffer
* build 24 bpp RGB mipmapped texture from 24 bpp work buffer

OK. Now, there are several problems here. First of all, functional problems that will crash your app:

* there is an implicit assumption that the surface is going to be 32 bpp after SDL_DisplayFormat. That might not be the case (if your framebuffer is 16 bpp because your display is running in thousands of colors, for example.)
* in the last step, you're asking GL to copy from your 16 bpp work buffer as if it were 24 bpp. This will read past the end of the buffer. Even if you luck out and don't crash, you'll get a garbled texture.

Second, conceptual problems with mipmaps:

* you create a 2D texture, and then immediately throw that away and create a 2D mipmapped texture with the same id. You don't need to do both.
* you use nearest minification, so the mipmaps will never be used.

Third, conceptual problems with alpha:
* you've previously stated that you're trying to use PNGs for alpha masking. Well, that's not going to work if you're throwing away the alpha channel.

Fourth, no support for NPOT textures:
* the GL_TEXTURE_2D target will only work with power-of-two image sizes. If you pass glTexImage2D an image with non-power-of-two sizes, it will fail. glGetError() would catch that, but you need to use padding, or stitching, or GL_TEXTURE_RECTANGLE_EXT if you want NPOT textures.
* gluBuild2DMipmaps will squish any NPOT images into POT sizes, via gluScaleImage. But you better be sure this is what you want, since it will lose data, possibly resulting in blurry textures.

Then there are inefficiencies arising from conceptual misunderstanding of how GL works:

* you're flipping Y, maybe because SDL loads the image upside down. There's no need to do this for every texture, just fix your coordinates instead (either your projection, or your texture coordinates.) Or just flip your data on disk.
* you're converting your pixels three times, when you only need one time:
* * convert from "image format" (could be 8, 16, 24, 32 bpp) to "display format" (could be 16 or 32 bpp)
* * convert from "display format" (assuming it is 32 bpp) to 16 bpp, flipping Y
* * convert from 16 bpp to "GL internal format" (32, then 24 bpp)
* glTexImage2D explicitly converts from "format, type" to "internalformat". So, you can avoid all of this repeated conversion by just going directly from "image format" (as returned by IMG_Load) to "internal format":
Code:
GLenum internal_format = GL_RGBA8; // could be GL_RGB5_A1, if you don't need 8 bit alpha and want to save VRAM
GLenum img_format, img_type;
switch (theTextureSurface->format->BitsPerPixel) {
    case 32: img_format = GL_RGBA; img_type = GL_UNSIGNED_BYTE; break;
    case 24: img_format = GL_RGB;  img_type = GL_UNSIGNED_BYTE; break;
    case 16: img_format = GL_RGBA; img_type = GL_UNSIGNED_SHORT_5_5_5_1; break;
    default: img_format = GL_LUMINANCE; img_type = GL_UNSIGNED_BYTE; break;
}
glTexImage2D(GL_TEXTURE_2D, 0, internal_format,
             theTextureSurface->w, theTextureSurface->h, 0,
             img_format, img_type, theTextureSurface->pixels);
* and, you leak theTextureImage. Always balance malloc() with free().

So, in summary:
* RTFM.
* Don't use code without knowing how it works.

[Edit: added alpha channel and NPOT notes.]
Quote this message in a reply
Moderator
Posts: 700
Joined: 2002.04
Post: #5
Ouch Blush After that particular critique, I feel I might upload the fixed code sometime soon... Blush

(catches all the errors mentioned above (at least in the unmodified code before SimReality/Nick started trying to implement mip-mapping) although still loads the image upside down... that's because the original texture loader was quickly built from a pixmap loader.)

Mark Bishop
--
Student and freelance OS X & iOS developer
Quote this message in a reply
Post Reply 

Possibly Related Threads...
Thread: Author Replies: Views: Last Post
  GL_TEXTURE_3D and mipmapping TomorrowPlusX 30 10,267 Nov 10, 2006 12:26 PM
Last Post: TomorrowPlusX