If we need to render something into a texture, a GLRenderTarget must be pushed on a stack, e.g.

GLRenderTarget::push(target);

// Draw

GLRenderTarget::pop();

But it's worth to mention that a couple of methods don't play along very nice with this design, namely GLRenderTarget::blitFramebuffer(). It just binds the default framebuffer object to GL_READ_FRAMEBUFFER, it doesn't care whether there is a GLRenderTarget on the stack, etc. I recommend to fix these methods instead. I don't think that we need to change the value of the default framebuffer object.

In D25904#575817, @zzag wrote:

If we need to render something into a texture, a GLRenderTarget must be pushed on a stack, e.g.

GLRenderTarget::push(target);

// Draw

GLRenderTarget::pop();

But it's worth to mention that a couple of methods don't play along very nice with this design, namely GLRenderTarget::blitFramebuffer(). It just binds the default framebuffer object to GL_READ_FRAMEBUFFER, it doesn't care whether there is a GLRenderTarget on the stack, etc. I recommend to fix these methods instead. I don't think that we need to change the value of the default framebuffer object.

The use case is a bit different here. The change is meant for allowing internal post-processing steps to be applied as in D25907 after the Scene rendering is done. But GlRenderTarget is meant for being used in the main Scene rendering.

For example it assumes the viewport to be of the same size as the texture it renders to and at position (0, 0). But for our final rendering we need the viewport to be of different nature because of scaling and since we "cut out" a piece of the overall screen geometry.

I'm not saying it is impossible to reuse GlRenderTarget for post-processing after Scene rendering has concluded but it was not designed for that and adapting it might become a hassle. But if in the future we find more use cases for post-processing besides rotation in aforementioned patch (I think of Night Color in case hardware gamma manipulation is not possible or not desirable) it might make sense to extend the API around it.

Hmm, okay, fair enough. Can you rename setKWinFramebuffer to setDefaultFramebufferObject? It's not clear what "kwin framebuffer" is.

In D25904#575886, @zzag wrote:

Hmm, okay, fair enough. Can you rename setKWinFramebuffer to setDefaultFramebufferObject? It's not clear what "kwin framebuffer" is.

I used the name s_defaultFramebuffer all the time in my working branch but changed it now for upload. ;)

The reason is that the Default Framebuffer is in OpenGl a fixed notion that names the framebuffer with id 0, see [1].

Since we have this here to use it also with framebuffer objects the variable would need some other name. But I'm open to other suggestions. I thougth of originalFramebuffer or backendFramebuffer.

[1] https://www.khronos.org/opengl/wiki/Default_Framebuffer

backendFramebuffer is a good one.