Bug 634

Thank you Andres for reviewing and helping to mature the API.

(In reply to comment #0)
> I've been looking at the implementation of the new FBO offscreen mechanism, and
> one question I have so far is why you chose to have two FBObjects with one
> texture attachment each, instead of a single one with two texture attachements.

This was actually my first approach of implementing GLFBODrawable.
Then it stroke me that this would not comply w/ the OpenGL FRAMEBUFFER[_OBJECT]
specification.

<http://jogamp.org/deployment/jogamp-next/javadoc/jogl/javadoc/javax/media/opengl/GLFBODrawable.html>

"It would be possible to implement double buffering simply using texture attachments with one framebuffer. This would require mode selection and hence complicate the API. Besides, it would not support differentiation of read and write framebuffer and hence not be spec compliant. "

[1] Background ..
=================

OpenGL uses the semantic of framebuffer (FB).
A FB is something GL can render to, i.e. a render target or in GLSL terms an output buffer.
GL may use multiple FBs as render targets and also can use multiple FBs for swapping.
Hence we can select individual FBs via 
  - [a.1] DrawBuffer(..) and 
  - [b.1] BindFramebuffer(..),
as well as query them via 
  - [a.2] glGet*(GL_DRAW_BUFFER) and 
  - [b.2] getDefault[Read/Write]Framebuffer() (custom API)

Semantics are very similar regarding build-in FBs ( a.1 and a.2 )
and FBO's FB (b.1 and b.2).

In short, GL handles rendering targets and read sources via FB not their configuration.

Further more, there are FBO impl. which cannot handle multiple colorbuffer attachments.

The BACK FB is the one we render into.
The FRONT FB is the one we use, i.e. 'see'.

+++

[2] Design / Impl. Transparency:
=================

Given the above - FBObject reflects _only_ the FBO semantics,
where GLFBODrawable simply exposes the FBO as a 'build-in' FB.

If we would have used simply multiple texture colorbuffer attachments,
which is n/a on some GL impl. (mobile .. etc),
we could not support an on-the-fly glReadPixels(..) or any other 
multiple render - or - read buffer semantics, since this is only possible via FB names in GL.

We also intercept BindFramebuffer() calls 
to return values of getDefault[Read/Write]Framebuffer() if available, 
i.e. if GLFBODrawable is in use.

This makes our GLFBODrawable implementation completely API transparent
resulting in a working offscreen FBO drawable implementation for e.g. OSX
w/o any user code change and still being able to utilize user FBO's,
switching back to the 'default' FBO, ReadPixels and also utilizing 
the 'default' read/write FBOs as rendering targets or source.

> 
> First, please correct me if my understand of how the mechanism currently works
> is wrong: When creating an FBO-backed drawing surface, there are two different
> scenarios: with and w/out multisampling. 
Yes.

> In both cases, two separate FBObjects
> are created, representing the front and back buffers. 
MSAA uses a hidden unaccessible BACK FBO. We only allow this to be 
queried via FBObject .. which may cause confusion.

> In the case of no
> multisampling, each buffer contains one texture attachement, and their roles as
> back (draw) and front (read) buffers are exchanged in each frame. 
Yes.

1 FBObject == 1 FBO name

> In the multisampled case, the back buffer contains the MSAA FBO, and a sampling buffer
> sink (another FBObject) which in turns contains the texture attachement where
> the MSAA render buffer is blitted to. 
Yes, in short - MSAA uses only one FBObject, which represents both,
the FRONT and [not accessible] BACK buffer.

Hence FBObject contains the semantics of blitting the native multisampled FBO
to an accessible [BACK] FBO.
This path has been chosen to
  - expose this detail w/ FBObject to the user
  - simplify impl. due to required reconfiguration of the samplink sink [FRONT FBO]
    in case of a reconfiguration of the 'source' BACK FBO.

> The front buffer simply appears to
> contain a reference to the texture attached to the sampling sink in the back
> buffer. 
Yes, we have hardcoded this in case of MSAA. 
It would technically be possible to just use a colorbuffer as well .. 

> 
> This stroked me at first as slightly odd, since I tend to think of an offscreen
> surface as a single FBO object with (potentially) two textures attached to its
> color attachement points. In this way, you alway bind the same FBO, but the
> textures are alternatively swapped between the two attachment points. 

In no MSAA - yes. OpenGL does the same.
In MSAA - we use the implicit extra FBO in FBObject (see above).

> I understand the logic of thinking of each buffer as a separate FBO, with a
> single texture attached it, and swapping the buffers instead of the textures.

.. as does OpenGL .. and see use case ([1] Background ..)

> 
> This logic works well in the non-multisampled case. However it breaks down
> somehow in the multisampled scenario because now the front and back buffers
> don't swap anymore, since the back buffer is fixed to be the MSAA FBO. 

Well .. it's actually the same thing as OpenGL does.
We have an implicit not accessible multisampled BACK buffer
in OpenGL, as well as in FBObject.
The swap command in GLFBODrawable issues the sync,
i.e. samples from unaccessible-BACK -> accessible-FRONT.
In short - MSAA is _always_ double buffered.

> 
> But this inconsistency would disappear if using a single FBO, always containing
> two texture attachements. 
> If num-samples = 0, all you do is to swap the textures. 
See above ([2] Design / Impl. Transparency). 
This way we would break GL analogy and some use cases
like glReadPixels(..) or FBO-MRT.. plus n/a on some platforms (only 1 colorbuffer 
attachment).

Again: I see the advantage of this - and I have started w/ it.
We could add a mode for GLFBODrawable to allow this - I still have the code
floating around here somewhere :)

But this would not supply the user with a complete transparent implementation!
Note: We even return the current GLFBODrawable FBObject's FBO 
via gl.glGetDefaultReadFramebuffer();

> In the multisampled case, the FBO would contain an internal source
> MSAA FBO (instead of a sink FBO) which is blitted at the end of each frame
> alternatively to each one of the textures. An advantage of this approach is
> that you will always have the previous frame available in the front texture,
> even if in the multisampled case the MSAA FBO is blitted to the back texture
> before the end of the frame (for example if the user needs to call
> readPixels()). 
So this would imply:
 - 1 multisampled source/BACK FB and 
 - 2 sampled FRONT FB, used alternately

That would be tripple buffering.
GL's double buffering doesn't allow you to have a previous rendered result avail.
I know what you like to say, i.e. you may could 'use' the BACK buffer as a source,
but mind that the spec says: reading _and_ writing to the same FB is undefined!

> Currently, an early blit of the MSAA buffer would erase the
> previous frame, since there is a single texture used by both the front and the
> back buffers.

As does GL  if you call swapBuffer/clearBuffer.

> 
> Any comments?

I agree that the exposed API of the current design might be confusing.

Regarding your suggestions (impl. and API clarifications):

E1 - GLFBODrawable: We could add support for n-buffers, i.e. tripple, quad .. etc buffer swap,
        the impl. is already taken this into account. -> GLFBODrawable

E2 - FBObject could be enhanced by allowing the 'sampling sink' (FRONT) to be re-attached,
        which would allow E1 to work.
        Reattaching the sink may require it to be reconfigured ofc to match the new source.
        Switching a sink FBO-F1 from source FBO-B1 to FBO-B2 implies:
             - FBO-B1.setSink(null)
             - FBO-B2.setSink(FBO-F1)

        Switching the semantics, i.e. setSink(..) -> setSource(..) doesn't make a difference IMHO.

I like to cont. this discussion regarding API enhancements (E1 - E2)
and the API design incl. background.

In the end we hopefully can generate a documentation from this discussion here 
and the prev. one.
  <http://forum.jogamp.org/querying-textures-bound-to-default-draw-read-framebuffers-td4026564.html>

> > In the multisampled case, the FBO would contain an internal source
> > MSAA FBO (instead of a sink FBO) which is blitted at the end of each frame
> > alternatively to each one of the textures. An advantage of this approach is
> > that you will always have the previous frame available in the front texture,
> > even if in the multisampled case the MSAA FBO is blitted to the back texture
> > before the end of the frame (for example if the user needs to call
> > readPixels()). 
> So this would imply:
>  - 1 multisampled source/BACK FB and 
>  - 2 sampled FRONT FB, used alternately
> 
> That would be tripple buffering.
> GL's double buffering doesn't allow you to have a previous rendered result
> avail.
> I know what you like to say, i.e. you may could 'use' the BACK buffer as a
> source,
> but mind that the spec says: reading _and_ writing to the same FB is undefined!
> 
> > Currently, an early blit of the MSAA buffer would erase the
> > previous frame, since there is a single texture used by both the front and the
> > back buffers.
> 
> As does GL  if you call swapBuffer/clearBuffer.

What makes me bring this particular issue up is the following sequence of operations:

// Read current contents of the back framebuffer
glReadBuffer(GL_BACK);
glReadPixels(..., pixBuff);

// Apply some pixel transformations on pixBuff...

// Copy pixBuff to a texture
glBindTexture(GL_TEXTURE_2D, id);
glTexSubImage2D(GL_TEXTURE_2D, ... pixBuff);

// Draw the texture covering the entire viewport...

As far I know, this is a valid approach (not necessarily the fastest, but that's not the point here) to do something like applying an post-processing filter on the current frame, with the option of rendering additional geometry after applying the filter (to see an demonstration of this idea, download the 2.0b6 release of Processing (http://processing.org/download/) and run the Topics/Shaders/EdgeFilter example)

So my questions is what would be the equivalent code when using an MSAA FBO layer?

(In reply to comment #2)
> > > In the multisampled case, the FBO would contain an internal source
> > > MSAA FBO (instead of a sink FBO) which is blitted at the end of each frame
> > > alternatively to each one of the textures. An advantage of this approach is
> > > that you will always have the previous frame available in the front texture,
> > > even if in the multisampled case the MSAA FBO is blitted to the back texture
> > > before the end of the frame (for example if the user needs to call
> > > readPixels()). 
> > So this would imply:
> >  - 1 multisampled source/BACK FB and 
> >  - 2 sampled FRONT FB, used alternately
> > 
> > That would be tripple buffering.
> > GL's double buffering doesn't allow you to have a previous rendered result
> > avail.
> > I know what you like to say, i.e. you may could 'use' the BACK buffer as a
> > source,
> > but mind that the spec says: reading _and_ writing to the same FB is undefined!
> > 
> > > Currently, an early blit of the MSAA buffer would erase the
> > > previous frame, since there is a single texture used by both the front and the
> > > back buffers.
> > 
> > As does GL  if you call swapBuffer/clearBuffer.
> 
> What makes me bring this particular issue up is the following sequence of
> operations:
> 
> // Read current contents of the back framebuffer
> glReadBuffer(GL_BACK);
> glReadPixels(..., pixBuff);
> 
> 

SNIP

> As far I know, this is a valid approach

You are right, this is possible w/ native FBs,
but it's invalid w/ FBO/MSAA due to the inaccessible BACK buffer.

>  (not necessarily the fastest, but
> that's not the point here) to do something like applying an post-processing
> filter on the current frame, with the option of rendering additional geometry
> after applying the filter (to see an demonstration of this idea, download the
> 2.0b6 release of Processing (http://processing.org/download/) and run the
> Topics/Shaders/EdgeFilter example)
> 
> So my questions is what would be the equivalent code when using an MSAA FBO
> layer?

As you suggested, we would require multiple buffers.

After solving the locking problem and adding more control of the GLEventListstener
in GLAutoDrawable - I will add such a unit test, i.e. a motion blurred gears
utilizing multiple buffers w/ FBO.

So all in all great point for usability, thank you!

How about the other issues of our discussion ?
Since you didn't reply in detail - I assume they are somewhat reasonable and fine.

~Sven

> How about the other issues of our discussion ?
> Since you didn't reply in detail - I assume they are somewhat reasonable and
> fine.

I need some more time to think about the other items and give you some feedback. I will post back soon.

http://jogamp.org/git/?p=jogl.git;a=commit;h=dbc260bd2e917ee9001461749c99da0c9cbfaf9a

> > What makes me bring this particular issue up is the following sequence of
> > operations:
> > 
> > // Read current contents of the back framebuffer
> > glReadBuffer(GL_BACK);
> > glReadPixels(..., pixBuff);
> > 
> > 
> 
> SNIP

In regards to the glReadBuffer(GL_BACK) situation, I eventually found the following code to do the same thing using the GLFBODrawable (based on the discussion we had on the forum <http://forum.jogamp.org/querying-textures-bound-to-default-draw-read-framebuffers-td4026564.html>):

// Blit all the current contents of the MSAA buffer into the front texture
GLCanvas canvas = (GLCanvas)drawable;
fboDrawable = (GLFBODrawable)canvas.getDelegatedDrawable();
drawFBO = fboDrawable.getFBObject(GL.GL_BACK);
drawFBO.syncSamplingSink(gl);
drawFBO.bind(gl);

// Bind front FBO and set the read buffer
glBindFramebuffer(context.getDefaultReadFramebuffer());
glReadBuffer(GL.GL_COLOR_ATTACHMENT0);

// Read pixels, etc.
glReadPixels(..., pixBuff);
...

This appears to work fine. Are you planing to modify the FBObject API in order to make this simpler?

My main concern was different though. Basically, that once you do the syncSampling call, the front texture doesn't contain the previous frame anymore, and I was thinking that perhaps the structure of the FBObject could be modified to take this into account. This is was I was tying to suggest in the first post with:

> In the multisampled case, the FBO would contain an internal source
> MSAA FBO (instead of a sink FBO) which is blitted at the end of each frame
> alternatively to each one of the textures.

But I just realized that the OpenGL specification doesn't guarantee the front texture to contain a valid copy from the previous frame in the first place:

http://www.opengl.org/wiki/Default_Framebuffer

where it states that:

"Rendering into, or reading from, the front buffer is not advisable.

The buffer swap does not have to be a true swap. In a true swap, the back buffer becomes the front buffer and vice-versa. If you were to read from the back buffer after a true swap, it would hold the previous contents of the front buffer."

so now I agree that there shouldn't be modifications in FBObject to accomodate a functionality that is not part of the spec. Still, because there might be usage cases in Processing where the current pixels from the back buffer (current frame) are needed at the same time as the pixels from the previous frame, I need a way of handling this situation. But I think I can take care of it by myself by copying the contents the back buffer right at the end of the frame before the swap into a "prev-frame" texture created specifically for this purpose.

> I like to cont. this discussion regarding API enhancements (E1 - E2) 
> and the API design incl. background.

I don't have strong feedback to provide about n buffer support. Sounds like an interesting feature to have, although I won't be making use of it, at least for the time being.

(In reply to comment #6)
> In regards to the glReadBuffer(GL_BACK) situation, I eventually found the
> following code to do the same thing using the GLFBODrawable (based on the
> discussion we had on the forum
> <http://forum.jogamp.org/querying-textures-bound-to-default-draw-read-framebuffers-td4026564.html>):
> 
> // Blit all the current contents of the MSAA buffer into the front texture
> GLCanvas canvas = (GLCanvas)drawable;
> fboDrawable = (GLFBODrawable)canvas.getDelegatedDrawable();
> drawFBO = fboDrawable.getFBObject(GL.GL_BACK);
> drawFBO.syncSamplingSink(gl);

The bind command is only required if you want to _write_ to the FBO,
hence not needed here to ReadPixels.

> drawFBO.bind(gl);
> 

This following BindFramebuffer is already done w/ syncSamplingSync()
using the default read framebuffer 0.

As documented in FBObject.sync*(), this is only true if used with
GLFBODrawableImpl, which hooks the magic '0' w/ GLContext/GLDrawable's 
getDefault[Read/Write]Framebuffer().

You may remember our initial discussion about this, i.e.
how to employ using FBO when user uses their own FBO objects
and reset to the default framebuffer '0'.

> // Bind front FBO and set the read buffer
> glBindFramebuffer(context.getDefaultReadFramebuffer());

This is redundant since we already have set the
READ_FRAMEBUFFER. I.e. we utilize ReadPixel in all UITestCases
where we never set the ReadBuffer explicitly.

> glReadBuffer(GL.GL_COLOR_ATTACHMENT0);
> 
> // Read pixels, etc.
> glReadPixels(..., pixBuff);
> ...
> 
> This appears to work fine. Are you planing to modify the FBObject API in order
> to make this simpler?

So .. in the end this will turn out to be:

> GLCanvas canvas = (GLCanvas)drawable;
> fboDrawable = (GLFBODrawable)canvas.getDelegatedDrawable();
> drawFBO = fboDrawable.getFBObject(GL.GL_BACK);
> drawFBO.syncSamplingSink(gl);
> // Read pixels, etc.
> glReadPixels(..., pixBuff);

Now we do read from the actual FRONT FB, i.e. just sampled BACK FB.

> 
> My main concern was different though. Basically, that once you do the
> syncSampling call, the front texture doesn't contain the previous frame
> anymore, and I was thinking that perhaps the structure of the FBObject could be
> modified to take this into account. This is was I was tying to suggest in the
> first post with:
> 
> > In the multisampled case, the FBO would contain an internal source
> > MSAA FBO (instead of a sink FBO) which is blitted at the end of each frame
> > alternatively to each one of the textures.
> 
> But I just realized that the OpenGL specification doesn't guarantee the front
> texture to contain a valid copy from the previous frame in the first place:
> 
> http://www.opengl.org/wiki/Default_Framebuffer
> 
> where it states that:
> 
> "Rendering into, or reading from, the front buffer is not advisable.

Well, it's not forbidden..
The one really concerning usage of FB's is to read from the BACK
_while_ writing to it - it's undefined.
Again, impossible to read from the MSAA BACK.

> 
> The buffer swap does not have to be a true swap. In a true swap, the back
> buffer becomes the front buffer and vice-versa. If you were to read from the
> back buffer after a true swap, it would hold the previous contents of the front
> buffer."

I see. And ofc in MSAA we have different restrictions, i.e. only able to read the FRONT.
However, we have full control over the buffer lifecycle, as we demonstrated in the above code.

> 
> so now I agree that there shouldn't be modifications in FBObject to accomodate
> a functionality that is not part of the spec. 
Aye.

> Still, because there might be
> usage cases in Processing where the current pixels from the back buffer
> (current frame) are needed at the same time as the pixels from the previous
> frame, I need a way of handling this situation. 
Aye - and I agree we should support this.

> But I think I can take care of
> it by myself by copying the contents the back buffer right at the end of the
> frame before the swap into a "prev-frame" texture created specifically for this
> purpose.
Well, I dislike the idea of another round of copying pixel here, it's redundant and 
bad for performance.

> 
> > I like to cont. this discussion regarding API enhancements (E1 - E2) 
> > and the API design incl. background.
> 
> I don't have strong feedback to provide about n buffer support. Sounds like an
> interesting feature to have, although I won't be making use of it, at least for
> the time being.

So .. I will go ahead w/:

- The n-buffer strategy

And also:

- Replace a buffer - call it user-swap, so a user is able to 
  take one FBO out of the loop and use it at will.

Both are similar and will not take a lot effort, but ease the GPU and avoids copying pixels.

Let's see how it goes .. looking fwd to your reviews and cooperation, thank you Andres.

Note: Exchanging the FBObject samplingSink is already done and actually clarifies the code a lot,
at least for me :)

> > But I think I can take care of
> > it by myself by copying the contents the back buffer right at the end of the
> > frame before the swap into a "prev-frame" texture created specifically for this
> > purpose.
> Well, I dislike the idea of another round of copying pixel here, it's redundant
> and 
> bad for performance.

I agree. After thinking a bit about this, it seems to me that the simplest approach in the multisampled case would be to have 3 separate FBO objects: two holding the color textures, and the third containing the (non-readable) MSA buffers. Something like this:

http://imgur.com/VPIKV

In this way, the previous frame is always available regardless of whether MSAA is being used or not, and there is no need of additional pixels copies. Simultaneously reading the current frame from the back buffer (after blitting) and the previous frame from the front buffer (w/out any additional copy operation) becomes possible. I will probably use this structure in the updated offscreen mode in Processing. 

As you currently do in JOGL for the MSAA situation, rendering always happens on the MSAA FBO. The main difference is that in JOGL the back and the MSAA FBOs are the same, but here they are separate. The contents of the MSAA FBO are always blitted into the color texture of the back buffer (either at the end of the frame or mid-frame in case a sync is explicitly required). I find this mechanism a bit cleaner. However, because of the object swapping, this approach needs to constantly update the sink reference in the MSAA FBO object, and not sure if it accurately follows the way the default framebuffers behave in OpenGL. 

I'm not sure how you are planning to take care of the prev frame/pixel copy issue in the MSAA mode, but maybe this diagram helps in our discussion.

Disregard my last comment. I figured out how to have a readable texture holding the previous frame, w/out the need of additional texture copies, and using the current mechanism in FBObject. Basically, in GLEventListener I do:

public void display(GLAutoDrawable glDrawable) {
  GLWindow glWindow = (GLWindow)glDrawable;
  fboDrawable = (GLFBODrawable)glWindow.getDelegatedDrawable();
  backFBO = fboDrawable.getFBObject(GL.GL_BACK);

  if (frontFBO == null) {
    // init front FBO
    frontFBO = new FBObject();
    frontFBO.reset(gl, pg.width, pg.height);
    frontFBO.attachTexture2D(gl, 0, true);
    sinkFBO = backFBO.getSamplingSinkFBO();
  } else {
    // swap front and sink FBO
    FBObject temp = sinkFBO;
    sinkFBO = frontFBO;
    frontFBO = temp;
    backFBO.setSamplingSink(sinkFBO); 
  }

  backTex  = (FBObject.TextureAttachment) sinkFBO.getColorbuffer(0);
  frontTex = (FBObject.TextureAttachment)frontFBO.getColorbuffer(0);

  ...
}

The trick is to swap the sink FBO with the front FBO (the back FBO with the MSAA is never touched). So frontTex always contains the last frame, and backTex can be used to retrieve the current frame by calling backFBO.syncSamplingSink(gl)

So far this approach seems to work. Let me know if you have any comments and/or suggestions.

(In reply to comment #9)
...
>     backFBO.setSamplingSink(sinkFBO); 
> 
>   backTex  = (FBObject.TextureAttachment) sinkFBO.getColorbuffer(0);
>   frontTex = (FBObject.TextureAttachment)frontFBO.getColorbuffer(0);
> 
>   ...
> 
> The trick is to swap the sink FBO with the front FBO (the back FBO with the
> MSAA is never touched). So frontTex always contains the last frame, and backTex
> can be used to retrieve the current frame by calling
> backFBO.syncSamplingSink(gl)
> 
> So far this approach seems to work. Let me know if you have any comments and/or
> suggestions.

Yes, the new method and late changes to FBObject were introduced due to 
this discussion.

Ofc - we need to expose it more friendly from GLFBODrawable* .. will do this after the stencil
bugfix .. before RC12

(In reply to comment #10)
> (In reply to comment #9)
> ...
> >     backFBO.setSamplingSink(sinkFBO); 
> > 
> >   backTex  = (FBObject.TextureAttachment) sinkFBO.getColorbuffer(0);
> >   frontTex = (FBObject.TextureAttachment)frontFBO.getColorbuffer(0);
> > 
> >   ...
> > 
> > The trick is to swap the sink FBO with the front FBO (the back FBO with the
> > MSAA is never touched). So frontTex always contains the last frame, and backTex
> > can be used to retrieve the current frame by calling
> > backFBO.syncSamplingSink(gl)
> > 
> > So far this approach seems to work. Let me know if you have any comments and/or
> > suggestions.
> 
> Yes, the new method and late changes to FBObject were introduced due to 
> this discussion.
> 
> Ofc - we need to expose it more friendly from GLFBODrawable* .. will do this
> after the stencil
> bugfix .. before RC12

Please don't change the API too much, it is working great so far :-)

Another, related, question. The format of the FBObject.TextureAttachment used as the color buffer in front FBO is 0x8058, which is not GL_RGBA. What is this format?