glClearBuffer & glReadAttachmentPixel

[Krogoth100]

Add gl.ClearBuffer(drawBufferIndex, clearingFunc, r,g,b,a)
Clears an individual draw buffer of an FBO and properly clears integer textures (the latter is more important)
drawBufferIndex special values: 0 - depth, -1 - stencil
gl.ClearBuffer(0, nil, depth) - clear depth buffer
gl.ClearBuffer(-1, nil, value) - clear stencil buffer
gl.ClearBuffer(1, nil, r,g,b,a) - clear float color buffer 1
gl.ClearBuffer(2, 2, r,g,b,a) - clear unsigned integer color buffer 2

Add gl.ReadAttachmentPixel(attachmentIndex, x, y, internalFormat)
A version of ReadPixels that reads a single pixel, but does it from specified FBO attachment and works with many formats (e.g. integer textures).
(See implementation for details)
Don't give same treatment to gl.ReadPixels, that function is complex as it is
attachmentIndex special values: 0 - depth
local depth = gl.ReadAttachmentPixel(0, x,y, GL_DEPTH_COMPONENT32F) - read FBO depth
local r,g,b,a = gl.ReadAttachmentPixel(2, x,y, GL_RGBA16F) - read color attachment 2

[lhog]

Add assert(false) for the default case.

[Krogoth100]

This is a utility function. Assert must or must not be placed in a function that uses it, not here.

[lhog]

I dislike 0, -1 and nils in this API, as much as possible the API should be self-explainatory, which is not the case here. Use semantics of https://springrts.com/wiki/Lua_FBO_and_RBO ("depth", "stencil", "color0", etc) or literal GL constants.

Also there's no need to for a user to explicitly select what kind of format the attached texture has, the implementation should either store it somewhere or retrieve it from the GL server - https://registry.khronos.org/OpenGL-Refpages/gl4/html/glGetFramebufferAttachmentParameter.xhtml

[Krogoth100]

I dislike 0, -1 and nils in this API, as much as possible the API should be self-explainatory, which is not the case here. Use semantics of https://springrts.com/wiki/Lua_FBO_and_RBO ("depth", "stencil", "color0", etc) or literal GL constants.

Also there's no need to for a user to explicitly select what kind of format the attached texture has, the implementation should either store it somewhere or retrieve it from the GL server - https://registry.khronos.org/OpenGL-Refpages/gl4/html/glGetFramebufferAttachmentParameter.xhtml

Why don't we keep such info as part of FBO class ourselves?

[lhog]

Because it was rarely (if it all) needed before

[Krogoth100]

Going to update this later.

[Krogoth100]

There is good possibility this GetFramebuffer... thing is busted.
I may be needing to update FBO class to retain info of its attachments.

[lhog]

What do you mean "busted"?

[Krogoth100]

What do you mean "busted"?

Not busted, I got it working in the end.
But this function is limited. The only useful is GL_FRAMEBUFFER_ATTACHMENT_COMPONENT_TYPE, but that is not the only thing I need.
I am planning to make Lua FBOs store attachment formats tomorrow, so they can be retrieved. What do you think?

[lhog]

Yeah no reason not to store additional information, if you need it.

[lhog]

Why do you need such function? GPU-->CPU read is almost always bad and will cause significant performance implications. If anything consider using PBOs and respective asynchronous API https://riptutorial.com/opengl/example/28872/using-pbos

[Krogoth100]

Why do you need such function? GPU-->CPU read is almost always bad and will cause significant performance implications. If anything consider using PBOs and respective asynchronous API https://riptutorial.com/opengl/example/28872/using-pbos

Do we already have anything like this implemented? Any reference to Recoil code.

[Krogoth100]

Why do you need such function? GPU-->CPU read is almost always bad and will cause significant performance implications. If anything consider using PBOs and respective asynchronous API https://riptutorial.com/opengl/example/28872/using-pbos

Performance is not priority. Think of this function as a function for a game editor or debug process, not the game itself. glReadPixels as it is works incorrectly for integer textures and doesn't allow reading individual attachments from within big FBO.
I don't have experience with PBO. Can you clarify, if I am to read a pixel data from the game render, does it allow to read the result of this exact frame which is happening right now, or it will be one of the previous frames?

[lhog]

Do we already have anything like this implemented? Any reference to Recoil code.

Nope but have a look at this old thread https://community.khronos.org/t/pbo-glreadpixels-not-so-fast/55783/13

does it allow to read the result of this exact frame which is happening right now, or it will be one of the previous frames?

Nope, but usually getting the pixel data a few frames late is fine.

[Krogoth100]

Do we already have anything like this implemented? Any reference to Recoil code.

Nope but have a look at this old thread https://community.khronos.org/t/pbo-glreadpixels-not-so-fast/55783/13

does it allow to read the result of this exact frame which is happening right now, or it will be one of the previous frames?

Nope, but usually getting the pixel data a few frames late is fine.

I am aware of glReadPixels performance. If I hit a bottleneck with this, I am going to investigate it further.
The purpose of this function is not fast performant texture read, it is to momentarily read the data from texture, same as gl.ReadPixels, limited to a single pixel, but applied to individual attachment and able to work with integer textures.
This function is a part of an update that enables work with integer textures and individual attachments (glClearBuffer). (Proper setup of integer textures was fixed few PRs ago)

[Krogoth100]

Would be good if someone actually tests this later, to find out how slow it really is.
Can do this test with gl.ReadPixels as well.
But this is not the point of this PR.

[Krogoth100]

@lhog
Made auto-identification of types for both functions
Check out latest:
72e6841

I wasn't going to update how ActiveFBO/RawBindFBO does the actual binding.
But I added necessary pieces so that current draw/read FBO can be retrieved.

[lhog]

API is better but not ideal yet, I think it's best to stick to string literals for color attachments in that case too. You might want to go switch/case(s) route which is more readable and utilize hashString() for that matter.

[Krogoth100]

API is better but not ideal yet, I think it's best to stick to string literals for color attachments in that case too. You might want to go switch/case(s) route which is more readable and utilize hashString() for that matter.

I am not a fan of "colori" stuff. What purpose does it serve? It is also 0-based, not what Lua should be.
What is supposed advantage of
gl.ClearBuffer("color0", ...)
vs
gl.ClearBuffer(1, ...)
?

[Krogoth100]

There is another thing that must be said, ClearBuffer doesn't operate on attachment, but a draw buffer.
ClearBuffer - draw buffer, ReadAttachmentPixel - attachment. They are essentially equal, unless FBOs attachment indices are setup with gaps (which I don't want to consider, because there is no reason to build your FBO like that, at least not for Lua tasks).
Therefore, to unite and simplify, it uses a term of "slot", which is a number, or "depth"/"stencil" when it's not.
This is the main argument, below is a bit offtopic.

On the other hand, the current FBO is setup using this construction table:
{
depth = attachment,
stencil = attachment,
color0 = attachment,
color1 = attachment,
color2 = attachment,
..
color15 = attachment,
drawbuffers = { GL_COLOR_ATTACHMENT0_EXT, GL_COLOR_ATTACHMENT3_EXT, ..}
}
Which, in my opinion, is bad to begin with.
A preferred form would be:
{
depth = attachment,
stencil = attachment,
colors = {attachments...},
}
Every one who uses current interface has to type this "colori" thing and then repeat GL constants in another table, because it is made so and it has to be done.
So you advise me to correct an interface to comply to this bad implementation, so it has similarity with this "colori" thingy, which should not be there in the first place.

Hopefully it explains why I don't like it.

[Krogoth100]

Although its reasonable for modifying fbo like
fbo.color0 = ...
(Still not justifying why it is 0-based when this interface is for Lua)

[lhog]

So @Krogoth100 do you plan to incorporate the changes I requested?

[Krogoth100]

Working on another stuff rn
May be best to close, as I apparently am the only one working with integer textures

[lhog]

Yes, this feature is for your game only, but there's nothing wrong with that. My only concern here is that it complies to the API standard I feel are right for the job. If this ask is too much - feel free to close.

[Krogoth100]

Yes, this feature is for your game only, but there's nothing wrong with that. My only concern here is that it complies to the API standard I feel are right for the job. If this ask is too much - feel free to close.

I will post updated version in beginning of September.

[Krogoth100]

@lhog Done.

[lhog]

LGTM, can you resolve the merge conflict please?

[Krogoth100]

LGTM, can you resolve the merge conflict please?

Done

[sprunk]

Is there any change other than adding gl.ClearBuffer and gl.ReadAttachmentPixel, i.e. is there no change at all if a game doesn't use them?

[Krogoth100]

Is there any change other than adding gl.ClearBuffer and gl.ReadAttachmentPixel, i.e. is there no change at all if a game doesn't use them?

There is only cosmetic code change besides that, should be no effect

[lhog]

Sorry I was (and still is) terribly busy IRL and I missed the merge window. Will merge as soon as we release a stable version.

[saurtron]

Updated this PR at #1807 (merged current master)

[lhog]

Superseded by #1807