Re: [PATCH 1/5] wined3d: Fix sRGB clears with no support for ARB_framebuffer_sRGB.
On 27 January 2017 at 19:52, Matteo Bruni <mbruni(a)codeweavers.com> wrote:
- if (!gl_info->supported[ARB_FRAMEBUFFER_SRGB] && needs_srgb_write(context, state, fb)) + if (!gl_info->supported[ARB_FRAMEBUFFER_SRGB] + && ((!(context->d3d_info->wined3d_creation_flags & WINED3D_SRGB_READ_WRITE_CONTROL) + && fb->render_targets[0] && fb->render_targets[0]->format_flags & WINED3DFMT_FLAG_SRGB_WRITE) + || state->render_states[WINED3D_RS_SRGBWRITEENABLE])) I think this looks suspicious, why is this more correct? In particular, wouldn't this enable sRGB clears on formats without WINED3DFMT_FLAG_SRGB_WRITE? Patch 2/5 hints that that may be intentional, but in that case, shouldn't this be independent of ARB_framebuffer_sRGB support?
2017-01-27 22:00 GMT+01:00 Henri Verbeet <hverbeet(a)gmail.com>:
On 27 January 2017 at 19:52, Matteo Bruni <mbruni(a)codeweavers.com> wrote:
- if (!gl_info->supported[ARB_FRAMEBUFFER_SRGB] && needs_srgb_write(context, state, fb)) + if (!gl_info->supported[ARB_FRAMEBUFFER_SRGB] + && ((!(context->d3d_info->wined3d_creation_flags & WINED3D_SRGB_READ_WRITE_CONTROL) + && fb->render_targets[0] && fb->render_targets[0]->format_flags & WINED3DFMT_FLAG_SRGB_WRITE) + || state->render_states[WINED3D_RS_SRGBWRITEENABLE])) I think this looks suspicious, why is this more correct? In particular, wouldn't this enable sRGB clears on formats without WINED3DFMT_FLAG_SRGB_WRITE? Patch 2/5 hints that that may be intentional, but in that case, shouldn't this be independent of ARB_framebuffer_sRGB support?
Yes, the idea is to do the "manual" sRGB correction of the clear color when ARB_framebuffer_sRGB is not supported even if the format doesn't advertise WINED3DFMT_FLAG_SRGB_WRITE. The story behind this goes like "oh, the test always expects the sRGB corrected color, which seems weird for formats / GPUs not supporting sRGB. Let me fix the test. Oh, wait, the test now fails on the GF4 Go (which doesn't advertise WINED3DFMT_FLAG_SRGB_WRITE for the relevant texture format) and actually it was passing before. I guess wined3d is wrong then..." So, this is the fix spurred from that test result, in that we do the manual clear color sRGB correction if ARB_framebuffer_sRGB is not supported AND we're on d3d9, regardless of the format flags. If ARB_framebuffer_sRGB is supported we enable GL_FRAMEBUFFER_SRGB in context_apply_clear_state() instead and the condition there should still be fine (I think?).
On 27 January 2017 at 22:22, Matteo Bruni <matteo.mystral(a)gmail.com> wrote:
Yes, the idea is to do the "manual" sRGB correction of the clear color when ARB_framebuffer_sRGB is not supported even if the format doesn't advertise WINED3DFMT_FLAG_SRGB_WRITE. The story behind this goes like "oh, the test always expects the sRGB corrected color, which seems weird for formats / GPUs not supporting sRGB. Let me fix the test. Oh, wait, the test now fails on the GF4 Go (which doesn't advertise WINED3DFMT_FLAG_SRGB_WRITE for the relevant texture format) and actually it was passing before. I guess wined3d is wrong then..."
So, this is the fix spurred from that test result, in that we do the manual clear color sRGB correction if ARB_framebuffer_sRGB is not supported AND we're on d3d9, regardless of the format flags. If ARB_framebuffer_sRGB is supported we enable GL_FRAMEBUFFER_SRGB in context_apply_clear_state() instead and the condition there should still be fine (I think?). Yes, but now (in d3d9) you'd get sRGB clears on e.g. WINED3DFMT_B5G6R5_UNORM when ARB_framebuffer_sRGB is not supported, while you wouldn't (necessarily) when it is.
2017-01-27 22:29 GMT+01:00 Henri Verbeet <hverbeet(a)gmail.com>:
On 27 January 2017 at 22:22, Matteo Bruni <matteo.mystral(a)gmail.com> wrote:
Yes, the idea is to do the "manual" sRGB correction of the clear color when ARB_framebuffer_sRGB is not supported even if the format doesn't advertise WINED3DFMT_FLAG_SRGB_WRITE. The story behind this goes like "oh, the test always expects the sRGB corrected color, which seems weird for formats / GPUs not supporting sRGB. Let me fix the test. Oh, wait, the test now fails on the GF4 Go (which doesn't advertise WINED3DFMT_FLAG_SRGB_WRITE for the relevant texture format) and actually it was passing before. I guess wined3d is wrong then..."
So, this is the fix spurred from that test result, in that we do the manual clear color sRGB correction if ARB_framebuffer_sRGB is not supported AND we're on d3d9, regardless of the format flags. If ARB_framebuffer_sRGB is supported we enable GL_FRAMEBUFFER_SRGB in context_apply_clear_state() instead and the condition there should still be fine (I think?). Yes, but now (in d3d9) you'd get sRGB clears on e.g. WINED3DFMT_B5G6R5_UNORM when ARB_framebuffer_sRGB is not supported, while you wouldn't (necessarily) when it is.
Ahhh, you're right, I didn't think of that. Hmm, not sure how to fix it properly then without introducing more flags... I guess first of all I should check what happens when clearing those never-supposed-to-be-sRGB formats.
participants (2)
-
Henri Verbeet -
Matteo Bruni