Skip to content

EGLBE: Support additional named framebuffer functions - #286

Open
tahmid-23 wants to merge 2 commits into
VirtualGL:mainfrom
tahmid-23:named-framebuffer
Open

EGLBE: Support additional named framebuffer functions#286
tahmid-23 wants to merge 2 commits into
VirtualGL:mainfrom
tahmid-23:named-framebuffer

Conversation

@tahmid-23

Copy link
Copy Markdown

Fixes #136.

This interposes the majority of the remaining named framebuffer functions (as enumerated in the issue) by checking for framebuffer 0.

One minor behavioral change is in getFramebufferAttachmentParameteriv(), which now checks for pname == GL_FRAMEBUFFER_ATTACHMENT_OBJECT_TYPE, since it might replace the parameter value if the original value collides with GL_RENDERBUFFER (which is admittedly pathological). I changed it here since this check also applies to getNamedFramebufferAttachmentParameteriv().

Since the EGL back end emulates GLX Pbuffers with FBOs, the "default framebuffer"
(0) is really a named FBO.  Interpose the remaining named framebuffer functions
that accept the default framebuffer (0) as an argument and translate 0 to the
emulating FBO:

  glBlitNamedFramebuffer()
  glCheckNamedFramebufferStatus[EXT]()
  glClearNamedFramebuffer{iv,uiv,fv,fi}()
  glGetFramebufferParameterivEXT()
  glGetNamedFramebufferAttachmentParameteriv[EXT]()
  glInvalidateNamedFramebuffer{Data,SubData}()

glIsFramebuffer[EXT]() is intentionally not interposed:  applications that
iterate the framebuffer namespace (e.g. piglit's object-namespace-pollution test)
rely on it reporting the true availability of names.

Also restrict the existing default-FB attachment OBJECT_TYPE fixup (RENDERBUFFER
-> FRAMEBUFFER_DEFAULT) to the attachment-parameter query for which it applies.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@dcommander

Copy link
Copy Markdown
Member

Great. I will take a look at this in the coming weeks. Comments/questions:

  • Please add a copyright attribution for yourself to the copyright header of any modified files, using the existing attribution format.
  • Does this fix a known issue with a specific application? The lack of such applications was the primary reason why EGL back end: Support for additional named framebuffer functions #136 hasn't been given a high priority. The last I checked, even piglit didn't test the functions in question.

@tahmid-23

tahmid-23 commented Jul 1, 2026

Copy link
Copy Markdown
Author

Minecraft (Java Edition)
It only uses one of these functions, though, I just did the rest for completeness

@dcommander

Copy link
Copy Markdown
Member

I'm just now getting around to reviewing the code. Unfortunately, because AI was used to generate the patch, I cannot accept it as-is. Because it's impossible to know the code sources that were used to train Claude and other AI models, it's impossible to know whether relicensing that code under an open source license violates someone else's copyright. It is, at best, a legal gray area.

The implementation also seems unnecessarily complex, at least on the surface. I will need to revisit it, but this project is a high priority.

@tahmid-23

Copy link
Copy Markdown
Author

I'm just now getting around to reviewing the code. Unfortunately, because AI was used to generate the patch, I cannot accept it as-is. Because it's impossible to know the code sources that were used to train Claude and other AI models, it's impossible to know whether relicensing that code under an open source license violates someone else's copyright. It is, at best, a legal gray area.

I can understand where you're coming from. Do you think it'd still be usable somehow? Would the patch need to be rewritten black-box? I guess that's not possible for you and me because we've seen this patch, right? I'm not sure how licensing strictly works.

The implementation also seems unnecessarily complex, at least on the surface. I will need to revisit it, but this project is a high priority.

Any particular part you're referring to? Most of the functions are thin wrappers, no? There is the bit with the enum remapping, but I do believe it should be necessary?

@dcommander

Copy link
Copy Markdown
Member

I can understand where you're coming from. Do you think it'd still be usable somehow? Would the patch need to be rewritten black-box? I guess that's not possible for you and me because we've seen this patch, right? I'm not sure how licensing strictly works.

A clean room isn't required. I just need to revisit the code from first principles and make sure that every coding decision flows from human experience. Some of the code I come up with may be "obvious", i.e. the only reasonable way that it can be implemented, and thus look very similar to the PR. That's fine. I just need to be able to justify it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

EGL back end: Support for additional named framebuffer functions

2 participants