On 30 April 2015 at 00:00, Stefan Dösinger stefan@codeweavers.com wrote:
diff --git a/dlls/wined3d/directx.c b/dlls/wined3d/directx.c index 36be539..ad9b496 100644 --- a/dlls/wined3d/directx.c +++ b/dlls/wined3d/directx.c @@ -4095,7 +4095,7 @@ static BOOL wined3d_check_pixel_format_color(const struct wined3d_gl_info *gl_in BYTE redSize, greenSize, blueSize, alphaSize, colorBits;
/* Float formats need FBOs. If FBOs are used this function isn't called */
- if (format->flags[WINED3D_GL_RES_TYPE_TEX_2D] & WINED3DFMT_FLAG_FLOAT)
- if (format->flags[WINED3D_GL_RES_TYPE_RB] & WINED3DFMT_FLAG_FLOAT)
That's not wrong as such, but it is pretty arbitrary. WINED3DFMT_FLAG_FLOAT is a basic property of the format, like e.g. the component masks and sizes, it shouldn't depend on the resource type. Once we'll get proper integer types for d3d10, it will probably make sense to keep track of the type (INT/UINT/FLOAT/UNORM/SNORM) as an enumeration instead, perhaps even per-component.
@@ -4178,8 +4178,8 @@ HRESULT CDECL wined3d_check_depth_stencil_match(const struct wined3d *wined3d, ds_format = wined3d_get_format(&adapter->gl_info, depth_stencil_format_id); if (wined3d_settings.offscreen_rendering_mode == ORM_FBO) {
if ((rt_format->flags[WINED3D_GL_RES_TYPE_TEX_2D] & WINED3DFMT_FLAG_RENDERTARGET)&& (ds_format->flags[WINED3D_GL_RES_TYPE_TEX_2D] & (WINED3DFMT_FLAG_DEPTH | WINED3DFMT_FLAG_STENCIL)))
if ((rt_format->flags[WINED3D_GL_RES_TYPE_RB] & WINED3DFMT_FLAG_RENDERTARGET)&& (ds_format->flags[WINED3D_GL_RES_TYPE_RB] & (WINED3DFMT_FLAG_DEPTH | WINED3DFMT_FLAG_STENCIL)))
This is actually wrong. At least, as long as we don't allow (plain) renderbuffers to be used for FBO attachments. There's nothing in the spec that says formats that can be used for color rendering with renderbuffers have to be available for color rendering with textures as well, and in fact that's much of the reason for this patch set.
@@ -998,9 +998,9 @@ static BOOL fbo_blit_supported(const struct wined3d_gl_info *gl_info, enum wined break;
case WINED3D_BLIT_OP_DEPTH_BLIT:
if (!(src_format->flags[WINED3D_GL_RES_TYPE_TEX_2D] & (WINED3DFMT_FLAG_DEPTH | WINED3DFMT_FLAG_STENCIL)))
if (!(src_format->flags[WINED3D_GL_RES_TYPE_RB] & (WINED3DFMT_FLAG_DEPTH | WINED3DFMT_FLAG_STENCIL)))
Similar to WINED3DFMT_FLAG_FLOAT, these are basic properties of the format.
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
Am 2015-04-30 um 11:09 schrieb Henri Verbeet:
That's not wrong as such, but it is pretty arbitrary. WINED3DFMT_FLAG_FLOAT is a basic property of the format, like e.g. the component masks and sizes, it shouldn't depend on the resource type. Once we'll get proper integer types for d3d10, it will probably make sense to keep track of the type (INT/UINT/FLOAT/UNORM/SNORM) as an enumeration instead, perhaps even per-component.
Yeah, as I said in the mail that introduced the split, I could move things like FLAG_FLOAT and FLAG_GETDC into a separate, resource-independent flags field. Or we just keep it the way it is and track things like float per component later on.
I'm using WINED3D_GL_RES_TYPE_RB instead of 2D here because this function is doing checks for the onscreen WGL buffer. It's not the same as a renderbuffer but it has the similar limitations and abilities that I don't see a reason to add a separate WGL buffer resource type at this point.
if ((rt_format->flags[WINED3D_GL_RES_TYPE_RB] & WINED3DFMT_FLAG_RENDERTARGET)&& (ds_format->flags[WINED3D_GL_RES_TYPE_RB] & (WINED3DFMT_FLAG_DEPTH | WINED3DFMT_FLAG_STENCIL)))This is actually wrong. At least, as long as we don't allow (plain) renderbuffers to be used for FBO attachments. There's nothing in the spec that says formats that can be used for color rendering with renderbuffers have to be available for color rendering with textures as well, and in fact that's much of the reason for this patch set.
I'm not really making a distinction between "is this format a depth stencil format" and "Can a resource with this type and format be created". Maybe we should add some WINED3DFMT_FLAG_SUPPORTED flag and check it in resource_init regardless of what the app requests?
WINED3DFMT_FLAG_TEXTURE doesn't work for this purpose because when the application creates a non-texture depth stencil surface (or even the implicit depth stencil) we have no reason to require WINED3DFMT_FLAG_TEXTURE.
On 30 April 2015 at 21:45, Stefan Dösinger stefandoesinger@gmail.com wrote:
if ((rt_format->flags[WINED3D_GL_RES_TYPE_RB] & WINED3DFMT_FLAG_RENDERTARGET)&& (ds_format->flags[WINED3D_GL_RES_TYPE_RB] & (WINED3DFMT_FLAG_DEPTH | WINED3DFMT_FLAG_STENCIL)))This is actually wrong. At least, as long as we don't allow (plain) renderbuffers to be used for FBO attachments. There's nothing in the spec that says formats that can be used for color rendering with renderbuffers have to be available for color rendering with textures as well, and in fact that's much of the reason for this patch set.
I'm not really making a distinction between "is this format a depth stencil format" and "Can a resource with this type and format be created". Maybe we should add some WINED3DFMT_FLAG_SUPPORTED flag and check it in resource_init regardless of what the app requests?
I think WINED3DFMT_FLAG_FBO_ATTACHABLE mostly does what we want as long as FBO ORM is used, perhaps it should be generalized to e.g. WINED3DFMT_FLAG_RENDERABLE.
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
Am 2015-04-30 um 22:33 schrieb Henri Verbeet:
I think WINED3DFMT_FLAG_FBO_ATTACHABLE mostly does what we want as long as FBO ORM is used, perhaps it should be generalized to e.g. WINED3DFMT_FLAG_RENDERABLE.
I'm inclined to keep it called WINED3DFMT_FLAG_FBO_ATTACHABLE and just not check it when we're using backbuffer ORM. We already have WINED3DFMT_FLAG_RENDERTARGET that more or less mirrors WINED3DFMT_FLAG_FBO_ATTACHABLE for color buffers. Having a FLAG_RENDERABLE in addition would be confusing IMO.
There's an argument to be made that with something that doesn't have FBO in its name we could mark depth formats supported or unsupported in the format table initialization and simplify the non-FBO depth format checks in check_device_format. But that doesn't really work because depth format compatibility depends on the onscreen depth format we end up using with onscreen ORM.
On 4 May 2015 at 13:42, Stefan Dösinger stefandoesinger@gmail.com wrote:
Am 2015-04-30 um 22:33 schrieb Henri Verbeet:
I think WINED3DFMT_FLAG_FBO_ATTACHABLE mostly does what we want as long as FBO ORM is used, perhaps it should be generalized to e.g. WINED3DFMT_FLAG_RENDERABLE.
I'm inclined to keep it called WINED3DFMT_FLAG_FBO_ATTACHABLE and just not check it when we're using backbuffer ORM. We already have WINED3DFMT_FLAG_RENDERTARGET that more or less mirrors WINED3DFMT_FLAG_FBO_ATTACHABLE for color buffers. Having a FLAG_RENDERABLE in addition would be confusing IMO.
It wouldn't really be in addition, since WINED3DFMT_FLAG_FBO_ATTACHABLE can go away then, but sticking with WINED3DFMT_FLAG_FBO_ATTACHABLE is probably ok in the short term. Ideally WINED3DFMT_FLAG_RENDERTARGET would just go away as well, but there are some tricky bits wrt. formats that never should be available for render targets on the one hand and formats that always should be available on the other hand.
-
Henri Verbeet -
Stefan Dösinger