-
Notifications
You must be signed in to change notification settings - Fork 69
Remove unused FBO code and add groundwork for FBO blit functions #1813
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
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
1e6353a to
b69b55d
Compare
illwieckz
reviewed
Sep 24, 2025
| // void (*glBlitFramebuffer) (GLint srcX0, GLint srcY0, GLint srcX1, GLint srcY1, GLint dstX0, GLint dstY0, GLint dstX1, GLint dstY1, GLbitfield mask, GLenum filter); | ||
| PFNGLBLITFRAMEBUFFERPROC glBlitFramebuffer; | ||
|
|
||
| /* Unused for now, part of GL_EXT_framebuffer_multisample: |
Member
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For consistence you can edit the comment accordingly like you did below.
Member
Author
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Undid the change to that other comment.
Member
|
We don't use renderbuffers. The renderbuffer size limit does not apply to an FBO.
Replace GL_FRAMEBUFFER with GL_DRAW_FRAMEBUFFER everywhere. Binding GL_FRAMEBUFFER to x is defined as binding GL_DRAW_FRAMEBUFFER to x then binding GL_READ_FRAMEBUFFER to x. Currently we only use the DRAW aspect. The double binding can be a nuisance when adding functionality (blit) that needs the read aspect.
Co-Authored-By: VReaperV <reaper9977@gmail.com>
Strangely, we didn't use any functions from EXT_framebuffer_blit before this, despite it being a required extension (if ARB_framebuffer_blit is unavailable) and a comment saying "our code is known to require EXT_framebuffer_blit".
Provide the alternative function to use if GL_ARB_framebuffer_object is not available but GL_EXT_framebuffer_object is. The missing shim could result in a crash if the former extension is not available and dynamic lights are enabled.
b69b55d to
ffc6f25
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The last 2 things are needed for FBO blitting. I need FBO blitting to fix #1783, while it is also used in #1480 so I'm trying to reuse the code from there. But I've tried to make R_BindFBO more robust by making GL_DRAW_FRAMEBUFFER the default instead of GL_FRAMEBUFFER because with the latter, you get issues that it matters what order you do the bindings in because it overwrites both READ and DRAW framebuffers.