Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FrameBuffer may not always dispose texture #3324

Closed
mgsx-dev opened this issue Aug 5, 2015 · 3 comments
Closed

FrameBuffer may not always dispose texture #3324

mgsx-dev opened this issue Aug 5, 2015 · 3 comments

Comments

@mgsx-dev
Copy link
Contributor

mgsx-dev commented Aug 5, 2015

Hi,

Has discussed here, it is not possible to dispose framebuffer and use the texture later since dispose method dispose the texture as well.
It's not possible to change the colorTexture neither which is a big limitation for some use-cases :

  • use the same FBO to render multiple textures.
  • texture swap for multipass shader algorithm.

FBO and texture attached to it may not have the same lifecycle.
For now my "ugly" workaround is to extends FrameBuffer in that way :

public class RenderToTexture extends FrameBuffer
{
  ...
    public Texture swapColorTexture()
    {
        Texture result = colorTexture;
        setupTexture();     

        bind();
        Gdx.gl20.glFramebufferTexture2D(GL20.GL_FRAMEBUFFER, GL20.GL_COLOR_ATTACHMENT0, GL20.GL_TEXTURE_2D,
                colorTexture.getTextureObjectHandle(), 0);
        unbind();

        return result;
    }

}

But it's not a perfect solution for many reasons :

  • This generate one texture more which is never used (just used to dispose it).
  • FrameBufferCubeMap needs to be extended in the same way.
  • this not cover all the use cases.

I could make a pull request with modifications on GLFrameBuffer class but i want to discuss about the solution before.

IMO, the color buffer may have to be controlled by user code.
The idea would be :

  • default behavior still unchanged, a color texture is always created, attached and disposed.
  • a new method to create and attach a new texture.
  • a new method to attach a texture created before.
  • a new method to detach the current texture (to not be automatically disposed when FBO is disposed).
  • when the "manual mode" is used, user code is responsible to dispose detached textures.

What do you think ?

@xoppa xoppa closed this as completed in b38a1db Aug 5, 2015
@xoppa
Copy link
Member

xoppa commented Aug 5, 2015

Thanks! Should be fixed now, you can now override the disposeColorTexture method and take ownership from that point on. The general rule of thumb is: if you create it, you dispose it. I don't think that encouraging changing (attaching/detaching) ownership is a good idea.

It would be possible to accept a Texture in the constructor and thus move ownership completely out of the FrameBuffer. But that would get messy when the texture isn't managed (currently the texture is recreated on context loss).

@mgsx-dev
Copy link
Contributor Author

mgsx-dev commented Aug 5, 2015

Thanks for the very quick commit (less than 1 hour :-))

That's solve the first use case which is probably the most important.

For the others use cases, I understand your point, maybe frame buffer have to be implemented specifically for those.

@makoConstruct
Copy link

I think a public Texture disposeAndTakeColorTexture() method would be appreciated, if that seems reasonable. Otherwise I think this is a code fragment most users are going to want:

class YieldingFrameBuffer extends FrameBuffer{// if you call disposeAndTakeColorTexture, colorBuffer is prevented from being disposed and given to you
    private boolean yielding = false;
    public YieldingFrameBuffer(Pixmap.Format format, int width, int height, boolean hasDepth){
        super(format, width, height, hasDepth);
    }
    @Override protected void disposeColorTexture(Texture t){
        if(!yielding) t.dispose();
    }
    public Texture disposeAndTakeColorTexture(){
        Texture r = getColorBufferTexture();
        yielding = true;
        dispose();
        return r;
    }
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants