glClearBuffer & glReadAttachmentPixel (updated) #1807
Conversation
+glClearBuffer +glReadAttachmentPixel
Keep track of active Lua FBO Store and reuse attachment format info
saurtron
changed the title
|
IDK about this one. Some parts of the PR are nice, the other ones like the ability to read the attachment pixel are very questionable. |
Yeah I don't know about that, saw it was approved and updated it just in case. Anyways if glReadAttachmentPixel is more controversial this could be split in two. Maybe @Krogoth100 has some use case for glReadAttachmentPixel that justifies it. |
| namespace Impl { | ||
| template<class Type> | ||
| static inline void ReadAttachmentPixel(lua_State* L, GLint x, GLint y, GLenum format, GLenum readType, int fSize) { | ||
| Type data[fSize]; |
There was a problem hiding this comment.
Perhaps we could avoid VLA? As far as I understand fSize is never more than 4 anyway
| Type data[fSize]; | |
| std::array <Type, 4> data; | |
| assert(fSize <= data.size()); |
| const GLint y = luaL_checkint(L, 3); | ||
|
|
||
| const auto activeLuaFBO = CLuaHandle::GetActiveFBOs(L).GetActiveReadFBO(); | ||
| assert(activeLuaFBO); |
There was a problem hiding this comment.
Is some fallback FBO always active, or will this trigger if I call it inside e.g. GameFrame or other non-drawing context?
| case hashString("color14"): { attachment = GL_COLOR_ATTACHMENT14; } break; | ||
| case hashString("color15"): { attachment = GL_COLOR_ATTACHMENT15; } break; | ||
| case hashString("depth"): { attachment = GL_DEPTH_ATTACHMENT; } break; | ||
| default: assert(false); |
There was a problem hiding this comment.
Shouldn't assert on user input
| case hashString("color15"): { bufferType = GL_COLOR; attachment = GL_COLOR_ATTACHMENT15; drawBuffer = 15; } break; | ||
| case hashString("depth"): { bufferType = GL_DEPTH; attachment = GL_DEPTH_ATTACHMENT; drawBuffer = 0; } break; | ||
| case hashString("stencil"): { bufferType = GL_STENCIL; attachment = GL_STENCIL_ATTACHMENT; drawBuffer = 0; } break; | ||
| default: assert(false); |
There was a problem hiding this comment.
Shouldn't assert on user input
| int LuaOpenGL::ReadAttachmentPixel(lua_State* L) | ||
| { | ||
| GLenum attachment; | ||
| switch(hashString(luaL_checkstring(L, 1))) { |
| } | ||
| } | ||
|
|
||
| int LuaOpenGL::ReadAttachmentPixel(lua_State* L) |
There was a problem hiding this comment.
Documentation would be good. https://github.com/beyond-all-reason/spring/pull/935/files#r1281757982 says that this is rather for development (editor/debug) and can have low perf, which are important caveats
Updated #935 (merged master and resolved some conflicts)
I think it should be fine but just in case I uploaded to a separate PR.