Re: [PATCH 3/8] wined3d: Keep track of renderbuffer capabilities.
On 22 May 2015 at 10:11, Stefan Dösinger <stefan(a)codeweavers.com> wrote:
@@ -1145,18 +1151,21 @@ static HRESULT texture_init(struct wined3d_texture *texture, const struct wined3 /* WINED3DUSAGE_DEPTHSTENCIL needs WINED3DFMT_FLAG_DEPTH or WINED3DFMT_FLAG_STENCIL, not both. */
if ((format->flags[WINED3D_GL_RES_TYPE_TEX_2D] & required_flags) == required_flags - && (!(desc->usage & WINED3DUSAGE_DEPTHSTENCIL) || (format->flags[WINED3D_GL_RES_TYPE_TEX_2D] & ds_flags)) + && check_ds_support(format, WINED3D_GL_RES_TYPE_TEX_2D, desc->usage) && (gl_info->supported[WINED3D_GL_NORMALIZED_TEXRECT] || (desc->width == pow2_width && desc->height == pow2_height))) gl_type = WINED3D_GL_RES_TYPE_TEX_2D; else if ((format->flags[WINED3D_GL_RES_TYPE_TEX_RECT] & required_flags) == required_flags - && (!(desc->usage & WINED3DUSAGE_DEPTHSTENCIL) || (format->flags[WINED3D_GL_RES_TYPE_TEX_RECT] & ds_flags))) + && check_ds_support(format, WINED3D_GL_RES_TYPE_TEX_RECT, desc->usage)) gl_type = WINED3D_GL_RES_TYPE_TEX_RECT; + else if ((format->flags[WINED3D_GL_RES_TYPE_RB] & required_flags) == required_flags + && check_ds_support(format, WINED3D_GL_RES_TYPE_RB, desc->usage)) + gl_type = WINED3D_GL_RES_TYPE_RB; else gl_type = WINED3D_GL_RES_TYPE_TEX_2D; /* No error, may be SCRATCH pool or emulated conditional np2. */
This is kind of ok because the existing code is like this, but notice the similarity to some of the code in resource_init(). In fact, is there any reason the caller needs to pass "gl_type" to resource_init() instead of letting it figure it out on its own?
-static void delete_fbo_attachment(const struct wined3d_gl_info *gl_info) +static void delete_fbo_attachment(const struct wined3d_gl_info *gl_info, + enum wined3d_gl_resource_type d3d_type, GLuint *object) { - gl_info->fbo_ops.glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, - GL_TEXTURE_2D, 0, 0); + switch (d3d_type) + { + case WINED3D_GL_RES_TYPE_TEX_1D: + case WINED3D_GL_RES_TYPE_TEX_2D: + case WINED3D_GL_RES_TYPE_TEX_RECT: + case WINED3D_GL_RES_TYPE_TEX_3D: + case WINED3D_GL_RES_TYPE_TEX_CUBE: + gl_info->gl_ops.gl.p_glDeleteTextures(1, object); + break; + + case WINED3D_GL_RES_TYPE_RB: + gl_info->fbo_ops.glDeleteRenderbuffers(1, object); + break; + + case WINED3D_GL_RES_TYPE_BUFFER: + case WINED3D_GL_RES_TYPE_COUNT: + break; + } } Why is "object" a pointer?
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Am 2015-05-25 um 10:27 schrieb Henri Verbeet:
This is kind of ok because the existing code is like this, but notice the similarity to some of the code in resource_init(). In fact, is there any reason the caller needs to pass "gl_type" to resource_init() instead of letting it figure it out on its own? The idea is to keep the switch between the different 2D types in the 2D texture init parts of the code instead of the generic code.
-static void delete_fbo_attachment(const struct wined3d_gl_info *gl_info) +static void delete_fbo_attachment(const struct wined3d_gl_info *gl_info, Why is "object" a pointer? Symmetry with create_and_bind_fbo_attachment(). But yeah, it's not really necessary, I'll change this.
-----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBAgAGBQJVYuDJAAoJEN0/YqbEcdMwJO4QAJS9Kb6d/0Wq5ioRpOeK0DI/ pLD7J9/jzRkH1DYNQzdhyH9LtAj4ugIx5XjjzR1tIsVXz8spjSDDMGDVF0QCEa5a wK0fckdV31QdFG81fdLgKsR+U/+ZehFgy4s/lBZugDq+foGpfsbQwLMb/nDzG9AS SEoWVOQYKEfOY3bdpsJxTx/vjK9d6Dt/KVGAlJFEPT5Xm64k74BkBW8MDfmN//Oe +HDdHnaxTi4seDbtcK2NLyHGU+YPaHt1YgK0HtLZW1uJ0IlvrV2O2XrSEmbGQUpu 7EkyNaYvZFJKzB+0EjGZrBIN2M38C0mLn8b9stE/FBAkt42e0YTGbXyo8/DENci6 xkn+ZMH4mtg2IaW9wxO2Xu92YOId3X5tyIYSE2zA+5/hXtCGCxOPdRzjVCO5TvV3 RHUg28pt8RPikHTnglV5NzsL8wuA/yaPgLPj0uGbLCz4t/1OApWnGWX7L5P9v5BT SuW6H9iJHWmXarbobgpHqmmqKA2+sPhmUjHCVJW4OM/WmLUGgRSZ6a0sRrbcXY8P 8bbtWNkhNq2JhaAcVMNJ0PgAvQfEvxSrRQyxAJQgj9Sf87/+KXYy0eV8pwYs8i2A IdBDW0wGW5y+40+08WHAa+k+IC43nMaGT77NhHm+OHLJecEkm671cDi5cQRLX20z RqDmOWRybXghDocnS8WH =70T8 -----END PGP SIGNATURE-----
On 25 May 2015 at 10:43, Stefan Dösinger <stefandoesinger(a)gmail.com> wrote:
Am 2015-05-25 um 10:27 schrieb Henri Verbeet:
This is kind of ok because the existing code is like this, but notice the similarity to some of the code in resource_init(). In fact, is there any reason the caller needs to pass "gl_type" to resource_init() instead of letting it figure it out on its own? The idea is to keep the switch between the different 2D types in the 2D texture init parts of the code instead of the generic code.
I guess, but it doesn't really seem worth it if that means duplicating much of the generic usage/format validation code.
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Am 2015-05-25 um 10:50 schrieb Henri Verbeet:
I guess, but it doesn't really seem worth it if that means duplicating much of the generic usage/format validation code. I tried to move the code to resource_init and the non power of 2 handling is a big problem. resource_init doesn't care about power of 2 and non power of 2 sizes, and I don't think it should. It would need to know that though to switch between 2D and RECT.
Adding a flag for "this resource type supports NP2 textures" isn't really an option either. If we have neither TEXTURE_NP2 nor TEXTURE_RECTANGLE (some GL ES systems according to Matteo) we want to use 2D textures with padding. The other relatively minor problem is handling subresources. The solution here is to never set or read the resource type on subresources, which we already do, and don't do capability validation on subresources. Right now we read format flags from subresources though . -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBAgAGBQJVZFDRAAoJEN0/YqbEcdMwZWoP/jhFewDCNegghTsvWiiWmwvV sHTn+tVduHCUqnZDYoytEYmQbAKC6nZCvAe+29j8OW/mP9/AraKc0edscrVha7ya uH0lgWWSITSSNQwg7VZMnIqq7zmrrO8BwSDLfNZILTtFzAZf2q72qknkkyiN62ui oVi1NjsHrBOBed7AyqVj+7JpilZ6DB1q7zrJ48p/k2sOEWJ8zcoqLcAlBXe51XQ8 sxXX4+zOmkNzg8n/x0bXAqZbgWHKRIzGbyI15iSY5cf849syNYKhJqVsL2hN0BZJ 53l+jdWVQYXNs+/jwIIOsStt7obvkcNeQj7hqZQGdPGjDxhhDStwxMRnFjZQmrcF RRTWKVAyXRk9I+SybHIn9Q9sR/7qN1GRv962xi856vf/F6SuLw5NE7+tZ8t2/UoD XlQkeDVNr/bcGeieFnjUlXZDwkv+EhEe1s7/tr+cEbrAxE+ze5Etj8/b+wTFgjP5 tc5M7SLZWQC8GrKGoOotR/CPPdHB0bXoZ4keYvmtUCKE9Vp/Qd7y3pLoZV9N9ZF7 r7NsjH2uOun4m1PaQ/EnsfxWvp8BbRKTsa34XYEWN6yaVVAjd9DvS4B/nZP73MDo bK4TQvpQq21GMWprAnBAi+mwcdNrjbkEPJm1EXPeHpgacJ9aQJG7gjAcuKc6+8oI k4ptRVihfkc98lnlQd4N =NsZR -----END PGP SIGNATURE-----
On 26 May 2015 at 12:54, Stefan Dösinger <stefandoesinger(a)gmail.com> wrote:
I tried to move the code to resource_init and the non power of 2 handling is a big problem. resource_init doesn't care about power of 2 and non power of 2 sizes, and I don't think it should. It would need to know that though to switch between 2D and RECT.
What is the issue exactly?
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Am 2015-05-26 um 13:03 schrieb Henri Verbeet:
On 26 May 2015 at 12:54, Stefan Dösinger <stefandoesinger(a)gmail.com> wrote:
I tried to move the code to resource_init and the non power of 2 handling is a big problem. resource_init doesn't care about power of 2 and non power of 2 sizes, and I don't think it should. It would need to know that though to switch between 2D and RECT.
What is the issue exactly? I'd have to move or duplicate the power of two size calculation (texture.c lines 1092 - 1123) to resource_init, and I don't think the generic resource code should have to deal with this functionality that is very specific to 2D textures. Passing a "this is a np2 texture" flag to resource_init is a possible option, but it would still take quite a bit of 2D texture specific code along with it (in particular the switch between true NP2, faked WINED3D_GL_NORMALIZED_TEXRECT NP2, ARB_texture_rectangle and 2D texture emulated NP2).
I'm not saying we can't do it that way. I just don't think it is nicer. I'll see if I can extract the flag checks into a separate function that texture_init and resource_init can call. -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBAgAGBQJVZFoKAAoJEN0/YqbEcdMwk8cP/2yLi7Ol7FJU/YFsjA50IAGG 0WpmXBNw93UQLU8DK63+l4+0y+ExqpvO27ACkmMotwAVJT8WhNIK8wxWQ2uUiirD jXCLq1dq70o4X7xm5odjY0x2k6zqDcbeptdQ0j9U0cy8GxedmII8wKEH7FZEOtsW 2v/6BCP7z300nfDGu3Gpv9i9M0KZ0OyS2D8CiPgxl1k4TU2diDEcTHwCPEAPTOpB 73a9aWJfn27nGAYJcGlpe14lNxBuHTzP8E+QSk2Jiv3lySPWi3WonArmjTRnexkW vgoR5ZCn+7SYo99qMJI2TM2gvdDKwbfckriSnJLGNiZjgi2OWRwXN1TVCWn6W0gu kZQ/cjrEfgkxb87vpQViM/+mwoKcdLL4PDJyHWIx5cBkjkyCbaGsGpzI5MTlvYa4 SNs83DRdGlAt5GQo7tAORRYBbUPXfVI1zCtwYwfVwOWmPQ10WppnKy4zuH+TOad4 H07sRm+xhYWBqw5TB11z3j0Yp/HSfnvYeks66ZdHe8iVL0YuzSONfy3gVX2DQqoK xY8tERj9IkRwUcpyepV5nO57tk32VRiUv3FWqyx7g8xWJTd17gF2fXZPSpAhTbcX 778iXhtmPFm/6wedHcr7a2fECM3ZaaKSaX+Ch9jhKadvxSIx57TjmlntWv7jt8OE BzPGbwd56AVgu0uJmi47 =lNvw -----END PGP SIGNATURE-----
On 26 May 2015 at 13:33, Stefan Dösinger <stefandoesinger(a)gmail.com> wrote:
I'd have to move or duplicate the power of two size calculation (texture.c lines 1092 - 1123) to resource_init, and I don't think the generic resource code should have to deal with this functionality that is very specific to 2D textures. Passing a "this is a np2 texture" flag to resource_init is a possible option, but it would still take quite a bit of 2D texture specific code along with it (in particular the switch between true NP2, faked WINED3D_GL_NORMALIZED_TEXRECT NP2, ARB_texture_rectangle and 2D texture emulated NP2).
You don't really need pow2_width/pow2_height itself though, you just need to determine if the resource dimensions are powers of two, which is much easier. (I.e., "!(desc->width & desc->width - 1)", etc.)
2015-05-26 12:54 GMT+02:00 Stefan Dösinger <stefandoesinger(a)gmail.com>:
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
Am 2015-05-25 um 10:50 schrieb Henri Verbeet:
I guess, but it doesn't really seem worth it if that means duplicating much of the generic usage/format validation code. I tried to move the code to resource_init and the non power of 2 handling is a big problem. resource_init doesn't care about power of 2 and non power of 2 sizes, and I don't think it should. It would need to know that though to switch between 2D and RECT.
Adding a flag for "this resource type supports NP2 textures" isn't really an option either. If we have neither TEXTURE_NP2 nor TEXTURE_RECTANGLE (some GL ES systems according to Matteo) we want to use 2D textures with padding.
Just to clarify on this point, even though it's quite tangential to the discussion: basic GL ES 2 has pretty much the same limitations as a D3D device with D3DPTEXTURECAPS_NONPOW2CONDITIONAL: NP2 textures are supported as long as you use CLAMP_TO_EDGE texcoord wrapping and don't make use of mipmapping in minification filtering. In theory D3D applications should check the cap bit and avoid that kind of usage, except often times they don't. The texture handling stuff related with WINED3D_GL_NORMALIZED_TEXRECT and WINED3D_TEXTURE_COND_NP2 mostly does the right thing here and yeah, it would be nice if that stays around. BTW, rectangle textures are not supported in OpenGL ES.
participants (3)
-
Henri Verbeet -
Matteo Bruni -
Stefan Dösinger