Re: [2/4] WineD3D: Make context_attach_surface_fbo srgb aware

[Henri Verbeet]

2009/9/10 Stefan Dösinger :
> @@ -402,7 +433,7 @@ static void context_apply_fbo_entry(struct wined3d_context *context, struct fbo_
>          for (i = 0; i < GL_LIMITS(buffers); ++i)
>          {
>              IWineD3DSurface *render_target = device->render_targets[i];
> -
>        context_attach_surface_fbo(context, GL_FRAMEBUFFER_EXT, i, render_target);
> -
>        context_attach_surface_fbo(context, GL_FRAMEBUFFER_EXT, i, render_target, SRGB_ANY);
>    }
>
>    /* Apply depth targets */

I don't think SRGB_ANY is correct here. context_apply_fbo_entry() very
much cares about which texture is attached to the FBO. If rendering to
sRGB textures was supported properly this would depend on the sRGB
write state, but since it isn't this should aways be SRGB_RGB.
> @@ -2041,7 +2072,7 @@ struct wined3d_context *ActivateContext(IWineD3DDeviceImpl *This, IWineD3DSurfac
>                      FIXME("Activating for CTXUSAGE_BLIT for an offscreen target with ORM_FBO. This should be avoided.\n");
>                      ENTER_GL();
>                      context_bind_fbo(context, GL_FRAMEBUFFER_EXT, &context->dst_fbo);
> -
>                context_attach_surface_fbo(context, GL_FRAMEBUFFER_EXT, 0, target);
> -
>                context_attach_surface_fbo(context, GL_FRAMEBUFFER_EXT, 0, target, SRGB_ANY);
>                context_attach_depth_stencil_fbo(context, GL_FRAMEBUFFER_EXT, NULL, FALSE);
>                LEAVE_GL();
>            } else {

Similar to above.
> @@ -6045,7 +6045,7 @@ static void color_fill_fbo(IWineD3DDevice *iface, IWineD3DSurface *surface,
>          context = ActivateContext(This, NULL, CTXUSAGE_RESOURCELOAD);
>          ENTER_GL();
>          context_bind_fbo(context, GL_FRAMEBUFFER_EXT, &context->dst_fbo);
> -
>    context_attach_surface_fbo(context, GL_FRAMEBUFFER_EXT, 0, surface);
> -
>    context_attach_surface_fbo(context, GL_FRAMEBUFFER_EXT, 0, surface, SRGB_ANY);
>    context_attach_depth_stencil_fbo(context, GL_FRAMEBUFFER_EXT, NULL, FALSE);
> }

Afaik color fill colors are specified in RGB space. Unless you have a
test that shows otherwise, this should be SRGB_RGB.
> @@ -6463,7 +6463,7 @@ void stretch_rect_fbo(IWineD3DDevice *iface, IWineD3DSurface *src_surface, WINED
>          TRACE("Source surface %p is offscreen\n", src_surface);
>          ENTER_GL();
>          context_bind_fbo(context, GL_READ_FRAMEBUFFER_EXT, &context->src_fbo);
> -
>    context_attach_surface_fbo(context, GL_READ_FRAMEBUFFER_EXT, 0, src_surface);
> -
>    context_attach_surface_fbo(context, GL_READ_FRAMEBUFFER_EXT, 0, src_surface, SRGB_ANY);
>    glReadBuffer(GL_COLOR_ATTACHMENT0_EXT);
>    checkGLcall("glReadBuffer()");
>    context_attach_depth_stencil_fbo(context, GL_READ_FRAMEBUFFER_EXT, NULL, FALSE);
>
> @@ -6503,7 +6503,7 @@ void stretch_rect_fbo(IWineD3DDevice *iface, IWineD3DSurface *src_surface, WINED
>      ENTER_GL();
>      context_bind_fbo(context, GL_DRAW_FRAMEBUFFER_EXT, &context->dst_fbo);
> -
>    context_attach_surface_fbo(context, GL_DRAW_FRAMEBUFFER_EXT, 0, dst_surface);
> -
>    context_attach_surface_fbo(context, GL_DRAW_FRAMEBUFFER_EXT, 0, dst_surface, SRGB_ANY);
>    glDrawBuffer(GL_COLOR_ATTACHMENT0_EXT);
>    checkGLcall("glDrawBuffer()");
>    context_attach_depth_stencil_fbo(context, GL_DRAW_FRAMEBUFFER_EXT, NULL, FALSE);

This doesn't work either. While it probably doesn't matter if the blit
is done in RGB or sRGB space, it still needs to be the same color
space for both surfaces (and one of those can be onscreen).

On a higher level, I'm not really happy with how sRGB textures
currently fit in and how that's starting to spread to everything that
has to deal with surfaces.