CPVRTPFXEffect::Destroy() doesn’t look right

Tagged: , ,

This topic contains 4 replies, has 2 voices, and was last updated by  Joe Davis 3 years, 3 months ago.

Viewing 5 posts - 1 through 5 (of 5 total)
  • Author
    Posts
  • #31747

    Hi there,

    In my project I have a system for caching textures shared between multiple objects in my scene. This is my project’s implementor of your PFXEffectDelegate interface, and the PFX loading code calls into PVRTPFXOnLoadTexture when it needs a texture.

    Everything is working well, until I start testing what happens when my application goes into the background state. Then I noticed that none of the textures loaded by the PFX are being deleted.

    I’m calling the CPVRPFXEffect::Destroy() method on the PFX object. The docs state that it “Deletes the gl program object and texture data.”, but after taking at look at it I can’t see that it does either of these things:
    void CPVRTPFXEffect::Destroy()
    {
    {
    if(m_uiProgram != 0)
    {
    GLint val;
    glGetProgramiv(m_uiProgram, GL_DELETE_STATUS, &val);
    if(val == GL_FALSE)
    {
    glDeleteProgram(m_uiProgram);
    }
    m_uiProgram = 0;
    }
    }

    m_bLoaded = false;
    }

    I’ve had a little check of the GL spec and it won’t delete the program if there are shaders attached.

    As far as the textures are concerned, is something else cleaning them up? – I sort of feel that the PFXEffectDelegate interface needs another method: PVRTPFXOnDeleteTexture().

    In my case I could then use this notification to decrement the usage of this texture in my cache and actually delete the texture if their are no more usages of it anywhere.

    Have I understood the intent of this part of the SDK correctly?

    Kind regards,

    Andre.

    #38847

    Joe Davis
    Member

    Hi Andre,

    Thanks for reporting the issue. You’re correct that this was a bug in our code. Our SDK team have implemented a fix for our 3.4 SDK (due ~September). In the meantime, here’s a snippet so you can fix the issue locally:

    /*!***************************************************************************
    @Function Destroy
    @Description Deletes the gl program object and texture data.
    *****************************************************************************/
    void CPVRTPFXEffect::Destroy()
    {
    {
    if(m_uiProgram != 0)
    {
    GLint val;
    glGetProgramiv(m_uiProgram, GL_DELETE_STATUS, &val);
    if(val == GL_FALSE)
    {
    glDeleteProgram(m_uiProgram);
    }
    m_uiProgram = 0;
    for(unsigned int uiTex = 0; uiTex < m_Textures.GetSize();++uiTex)
    {
    glDeleteTextures(1,&m_Textures[uiTex].ui);
    m_Textures[uiTex].ui = 0;
    }
    }
    }

    m_bLoaded = false;
    }

    Thanks,
    Joe

    #38848

    Hi Joe,

    Thanks for that. I’ve actually made my own mod to my copy of the SDK that works well.

    As I mentioned in my earlier post I wanted my TextureCache class (which is an implementor of the PVREffectDelegate interface) to receive callbacks when a texture is no longer needed by a particular PFX.

    You already have the PVRTPFXEffectDelegate::PVRTPFXOnLoadTexture() callback declared, but you didn’t have a corresponding callback for releasing a texture.

    So I’ve written PVRTPFXEffectDelegate::PVRTPFXOnReleaseTexture(), and a few supporting changes under the covers to call it when a texture is being released.

    This is nice as it allows my TextureCache to share textures efficiently between multiple POD/PFXs I have loaded.

    I think what i’ve done is general enough that you might want to consider including it in your code for everyone’s benefit. I’ve done my best to follow the style of the existing code – all the changes are in PVRTPFXParserAPI.cpp/.h

    I’m happy to email the modded files for your guys to take a look.

    Kind regards,

    Andre.

    #38849

    Joe Davis
    Member

    Hi Andre,

    Sounds like a better solution. If you attach your code to a support ticket, I’ll pass it on to our SDK team.

    Thanks,
    Joe

    #38850

    Joe Davis
    Member

    Hi Andre,

    Thanks for sharing your code with us. I’ve attached your code to bug report BRN49173 so the changes can be considered for a future release.

    Thanks,
    Joe

Viewing 5 posts - 1 through 5 (of 5 total)
You must be logged in to reply to this topic.