[Bug 60074] New: Incorrect `GL_FRAMEBUFFER_SRGB` handling on the default framebuffer

[WineHQ Bugzilla]

http://bugs.winehq.org/show_bug.cgi?id=60074

            Bug ID: 60074
           Summary: Incorrect `GL_FRAMEBUFFER_SRGB` handling on the
                    default framebuffer
           Product: Wine
           Version: 11.14
          Hardware: x86-64
                OS: Linux
            Status: UNCONFIRMED
          Severity: normal
          Priority: P2
         Component: opengl
          Assignee: [e-mail]
          Reporter: [e-mail]
  Target Milestone: ---
      Distribution: ---

Created attachment 81584
  --> http://bugs.winehq.org/attachment.cgi?id=81584
Patch

Somehow related to https://bugs.winehq.org/show_bug.cgi?id=59436 which has been
fixed upstream alternatively. 

While investigating a color issue affecting Roon under Wine with Xwayland, I
found that `GL_FRAMEBUFFER_SRGB` remained enabled while rendering to the
default framebuffer. This produced incorrect color output because the default
framebuffer should not always inherit the application's logical sRGB state.

### Root cause

Wine correctly tracks the logical OpenGL enable state through
`set_context_attribute()`, but `GL_FRAMEBUFFER_SRGB` requires additional
handling because its physical state depends on the currently bound draw
framebuffer.

An application may also call:


glEnable(GL_FRAMEBUFFER_SRGB);


while the default framebuffer is bound. In that case, simply disabling sRGB in
`glBindFramebuffer(0)` is not sufficient because the subsequent `glEnable()`
immediately re-enables it.

### Proposed solution

The patch introduces four changes:

1. Add `GL_FRAMEBUFFER_SRGB` to Wine's tracked enable state.
2. Generate a manual wrapper for `glEnable()`.
3. Prevent the physical `glEnable(GL_FRAMEBUFFER_SRGB)` call when the current
draw framebuffer is the default framebuffer, while still updating Wine's
logical state through `set_context_attribute()`.
4. Update `set_current_fbo()` so that switching draw framebuffers restores the
correct physical OpenGL state:

   * disable sRGB when binding framebuffer 0;
   * re-enable it when binding a user FBO if the logical state is enabled.

### Why a wrapper?

The wrapper is necessary because `glEnable()` is executed before
`set_context_attribute()`.

Without intercepting `glEnable(GL_FRAMEBUFFER_SRGB)`, an application can
immediately re-enable sRGB on the default framebuffer, bypassing the correction
performed during framebuffer switches.

### Validation

The patch was tested with Roon, which originally exhibited incorrect colors.

Instrumentation confirmed the following sequence:

* `glEnable(GL_FRAMEBUFFER_SRGB)` is issued while the default framebuffer is
bound.
* The wrapper blocks the physical OpenGL enable while preserving the logical
state.
* When Roon switches to its off-screen FBO, `GL_FRAMEBUFFER_SRGB` is correctly
enabled.
* When it returns to the default framebuffer, `GL_FRAMEBUFFER_SRGB` is disabled
again.

The resulting rendering is correct and no regressions were observed during
testing.

I'd appreciate comments on whether this approach fits Wine's intended handling
of framebuffer-dependent OpenGL state, or whether there is a preferred
architectural solution.

-- 
Do not reply to this email, post in Bugzilla using the
above URL to reply.
You are receiving this mail because:
You are watching all bug changes.