Possible bug?

If you have any questions on programming, this is the place to ask them, whether you're a newbie or an experienced programmer. Discussion on programming in general is also welcome. We will help you with programming homework, but we will not do your work for you! Any porting requests must be made in Developmental Ideas.
Post Reply
Chilly Willy
DC Developer
DC Developer
Posts: 414
https://www.artistsworkshop.eu/meble-kuchenne-na-wymiar-warszawa-gdzie-zamowic/
Joined: Thu Aug 20, 2009 11:00 am
Has thanked: 0
Been thanked: 2 times

Possible bug?

Post by Chilly Willy »

I'm not the expert on OpenGL, but if I've read various guides right, I think this is a bug.

In glTexImage2D(), you find this code:

Code: Select all

    GL_KOS_TEXTURE_UNIT[GL_KOS_ACTIVE_TEXTURE]->width   = width;
    GL_KOS_TEXTURE_UNIT[GL_KOS_ACTIVE_TEXTURE]->height  = height;
    GL_KOS_TEXTURE_UNIT[GL_KOS_ACTIVE_TEXTURE]->mip_map = level;
    GL_KOS_TEXTURE_UNIT[GL_KOS_ACTIVE_TEXTURE]->color   = type;

    GLuint bytes = level ? glKosMipMapTexSize(width, height) : (width * height * 2);

    GL_KOS_TEXTURE_UNIT[GL_KOS_ACTIVE_TEXTURE]->data = pvr_mem_malloc(bytes);

    if(data) {
The problem is that it's always allocating pvr memory. Everything I've found via google says you can call glTexImage2D() on the same texture to update it. So it SHOULD check if the bound texture already has pvr memory allocated before allocating some, like this:

Code: Select all

    GL_KOS_TEXTURE_UNIT[GL_KOS_ACTIVE_TEXTURE]->width   = width;
    GL_KOS_TEXTURE_UNIT[GL_KOS_ACTIVE_TEXTURE]->height  = height;
    GL_KOS_TEXTURE_UNIT[GL_KOS_ACTIVE_TEXTURE]->mip_map = level;
    GL_KOS_TEXTURE_UNIT[GL_KOS_ACTIVE_TEXTURE]->color   = type;

    GLuint bytes = level ? glKosMipMapTexSize(width, height) : (width * height * 2);

    if(!GL_KOS_TEXTURE_UNIT[GL_KOS_ACTIVE_TEXTURE]->data)
        GL_KOS_TEXTURE_UNIT[GL_KOS_ACTIVE_TEXTURE]->data = pvr_mem_malloc(bytes);

    if(data) {
The recommended function to modify an existing texture is glTexSubImage2D() since it doesn't do anything but change the pixels in a region of an existing texture, but that function isn't defined in kgl. glTexImage2D() is possibly slower because you could pass a different size or format as well as new data, meaning it might need to re-allocate memory if the size changed. I don't think the existing function can handle that, but not allocating pvr memory if it's already allocated would at least handle updating the texels. To handle changing the texture size, maybe something like this:

Code: Select all

    if(GL_KOS_TEXTURE_UNIT[GL_KOS_ACTIVE_TEXTURE]->data) {
        /* pre-existing texture - check if changed */
        if(GL_KOS_TEXTURE_UNIT[GL_KOS_ACTIVE_TEXTURE]->width != width ||
           GL_KOS_TEXTURE_UNIT[GL_KOS_ACTIVE_TEXTURE]->height != height ||
           GL_KOS_TEXTURE_UNIT[GL_KOS_ACTIVE_TEXTURE]->mip_map != level ||
           GL_KOS_TEXTURE_UNIT[GL_KOS_ACTIVE_TEXTURE]->color != type) {
            /* changed - free old texture memory */
            pvr_mem_free(GL_KOS_TEXTURE_UNIT[GL_KOS_ACTIVE_TEXTURE]->data);
            GL_KOS_TEXTURE_UNIT[GL_KOS_ACTIVE_TEXTURE]->data = NULL;
        }
    }

    if(!GL_KOS_TEXTURE_UNIT[GL_KOS_ACTIVE_TEXTURE]->data) {
        /* need texture memory */
        GL_KOS_TEXTURE_UNIT[GL_KOS_ACTIVE_TEXTURE]->width   = width;
        GL_KOS_TEXTURE_UNIT[GL_KOS_ACTIVE_TEXTURE]->height  = height;
        GL_KOS_TEXTURE_UNIT[GL_KOS_ACTIVE_TEXTURE]->mip_map = level;
        GL_KOS_TEXTURE_UNIT[GL_KOS_ACTIVE_TEXTURE]->color   = type;

        GLuint bytes = level ? glKosMipMapTexSize(width, height) : (width * height * 2);

        GL_KOS_TEXTURE_UNIT[GL_KOS_ACTIVE_TEXTURE]->data = pvr_mem_malloc(bytes);
    }

    if(data) {
With the above code, if the texture already exists and you aren't changing any of the parameters, just updating the texels, it won't do anything. If you change any of the parameters, it frees the memory, then the next part allocates new memory and sets the new parameters.
User avatar
PH3NOM
DC Developer
DC Developer
Posts: 576
Joined: Fri Jun 18, 2010 9:29 pm
Has thanked: 0
Been thanked: 5 times

Re: Possible bug?

Post by PH3NOM »

Interesting, I have not considered that before. Your solution looks like it should do the trick!
Chilly Willy
DC Developer
DC Developer
Posts: 414
Joined: Thu Aug 20, 2009 11:00 am
Has thanked: 0
Been thanked: 2 times

Re: Possible bug?

Post by Chilly Willy »

It's one of those weird things you run into by accident. I'm working on video playing and wanted a quick update for the player OpenGL texture and was googling for various ways people do this. On "old" OpenGL, you use glSubTexImage2D when available, or glTexImage2D if not, and something else for "new" OpenGL (I didn't pay attention to that since I know we're using "old" OpenGL). I had previously looked at glTexImage2D and remembered that it did a pvr_mem_malloc for the texture. Using OpenGL lets me stretch the source however I want for the output.
Chilly Willy
DC Developer
DC Developer
Posts: 414
Joined: Thu Aug 20, 2009 11:00 am
Has thanked: 0
Been thanked: 2 times

Re: Possible bug?

Post by Chilly Willy »

One change on the code - needed to pull out the bytes variable since it's used later and inside the if statement it's not always evaluated.

Code: Select all

    if(GL_KOS_TEXTURE_UNIT[GL_KOS_ACTIVE_TEXTURE]->data) {
        /* pre-existing texture - check if changed */
        if(GL_KOS_TEXTURE_UNIT[GL_KOS_ACTIVE_TEXTURE]->width != width ||
           GL_KOS_TEXTURE_UNIT[GL_KOS_ACTIVE_TEXTURE]->height != height ||
           GL_KOS_TEXTURE_UNIT[GL_KOS_ACTIVE_TEXTURE]->mip_map != level ||
           GL_KOS_TEXTURE_UNIT[GL_KOS_ACTIVE_TEXTURE]->color != type) {
            /* changed - free old texture memory */
            pvr_mem_free(GL_KOS_TEXTURE_UNIT[GL_KOS_ACTIVE_TEXTURE]->data);
            GL_KOS_TEXTURE_UNIT[GL_KOS_ACTIVE_TEXTURE]->data = NULL;
        }
    }

    GLuint bytes = level ? glKosMipMapTexSize(width, height) : (width * height * 2);

    if(!GL_KOS_TEXTURE_UNIT[GL_KOS_ACTIVE_TEXTURE]->data) {
        /* need texture memory */
        GL_KOS_TEXTURE_UNIT[GL_KOS_ACTIVE_TEXTURE]->width   = width;
        GL_KOS_TEXTURE_UNIT[GL_KOS_ACTIVE_TEXTURE]->height  = height;
        GL_KOS_TEXTURE_UNIT[GL_KOS_ACTIVE_TEXTURE]->mip_map = level;
        GL_KOS_TEXTURE_UNIT[GL_KOS_ACTIVE_TEXTURE]->color   = type;

        GL_KOS_TEXTURE_UNIT[GL_KOS_ACTIVE_TEXTURE]->data = pvr_mem_malloc(bytes);
    }
Post Reply