Re: [PATCH 2/4] wined3d: Keep track of renderbuffer capabilities.
On 30 April 2015 at 00:00, Stefan Dösinger <stefan(a)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. -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBAgAGBQJVQoZbAAoJEN0/YqbEcdMwUf0P/RfOs3qlOmR0QQp1GHruCuzz KMX5hh9Mr52MQ6Q7ZGmN2WxsWRsm0uyoIozzyjRnJncrwtYr9PH4mGn9RL3Yu4So AJMrmLWxhzoW07pBfWkiOwCnxej29pI3A2tZ2T2Gqhlb/ShiruBjwuWr7STrhg1L DSB6vDp3os7yo4w+itPyXYPRyxC/OlOXCMt27YKNKYmHdhGTYR3/TPM4yswp1zI0 0V4AFDrjtOD/c80mBVFKGcFUbN0aYlLKzHrCyXGpgutIfc8ULJOAKFlUvhP4I3so VeYtlnlMLcbxaOqd7VuteSvZl/hXwrOBOd7hy2Cfop/aHkmx6jCIL3qHowlGhZpc ADz98OkGyKX+xJ0KUmKpI1iLLsvkig5vAZsKGcawgEvAM4sycBaLLZPh5rmruXr3 ZhpmbKIONXnbNKxOx/iV02Vl7ItY0ziSsdtwrPb2GKhLhdOlohrsa0CByyY7LS6s gmZQ5IMlYaq9QLWjHxX2DW0J/3q/ovQXEMVjKoo+tK8F5pR1f5egXuDgBkOsw8z5 yTz4nvQmHoPdbLPIeiW/SC4+D6T7aOctWoy10If+mRR+VVq7uKMTicXkdgnhS2RM m7SpKTfy8vhVblVZ594kXFCPqPo5cm/CWWH7DnxaRcN3B6tbS2d5ZeamJ6qnOyps LIyM343lMK6fDVpsj0+I =trjB -----END PGP SIGNATURE-----
On 30 April 2015 at 21:45, Stefan Dösinger <stefandoesinger(a)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. -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBAgAGBQJVR1sLAAoJEN0/YqbEcdMwc0IP/j1+Y8GynkGpe5vJ1oatNmip Zp2ndEoglG7C5K4dpqX0uYqUA0sTY0NcccZrwzFEKIf/1+V4532g5s/Nqf729Tcm Xnqjm2BU/SZltnR35ZlXdm4zB244SpdMbDSY39zbHDzsI/bxXd2w1RcFrKWqU50K Joe0SyX+yY3FKg5VgSEf2uXsk42ItRJetPCMzUemjxg3u8fzGyHAokOjMnov5Ml5 NPhytT4wA0/Oo6Wzm/GIk2ZXD7JN10H+kuYCXKstyTpV0nHxA3L5n82Spi/ANuls U9Mp5A+JRUOd69ysKeVmg7XStuBhYGcRAFIFfS/lWoFaMF6Ej1VtZWyYwfURZJCQ KwhBOwgo4E5m7HigB6wa8Ejg9pgx3bhs/YFnLttv+5/LsDpP6tcT5K29ddf5AOBg S3V6wvlrkedq21OS6aVzXzF37I7k0AchIw3Ha3h/WLN8wjC5PCwv3Y7a4ivkoZ6F 9R1wvgIGpbeSyC5BPohmD04BrwlesZVJlUZUr714+yjmgAZyTRHQavUNazwRmyI+ 6UIaqI2EFsYmvG7OvGDVyQ1hv2Jv77tQxGaSNrsG+d2DpKnl/FE5nwiTO5xtmmQN i+qKEfZU75vyKBdveJboBOt+l57hLcVi4Yg0r9j859M7Y0ateG8GEyOmRqwqu0qZ k4VzvBVINOdGdSkiSszS =g3D+ -----END PGP SIGNATURE-----
On 4 May 2015 at 13:42, Stefan Dösinger <stefandoesinger(a)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.
participants (2)
-
Henri Verbeet -
Stefan Dösinger