-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
Am 2015-02-18 um 14:20 schrieb Matteo Bruni:
- if (pitch & (texture_resource->format->byte_count - 1))
- {
WARN("Pitch unsupported, not a multiple of the texture format byte width.\n");return WINED3DERR_INVALIDCALL;- }
I'd prefer this to be an ERR instead, and the return statement removed. The idea is that since we don't ever expect this check to be true it shouldn't result in different behavior and can be compiled out in a build where debugging is disabled.
The basic idea originated from Henri: https://www.winehq.org/pipermail/wine-devel/2013-August/100974.html .
On 18 February 2015 at 16:07, Stefan Dösinger stefandoesinger@gmail.com wrote:
Am 2015-02-18 um 14:20 schrieb Matteo Bruni:
- if (pitch & (texture_resource->format->byte_count - 1))
I don't think this would do the right thing for formats where byte_count isn't a power of two. I also think the check belongs in wined3d_texture_update_desc(), since otherwise the texture would be in an inconsistent state if it fails. (That actually also applies to failing to create the DIB section, but that one seems much harder to avoid.)
- {
WARN("Pitch unsupported, not a multiple of the texture format byte width.\n");return WINED3DERR_INVALIDCALL;- }
I'd prefer this to be an ERR instead, and the return statement removed. The idea is that since we don't ever expect this check to be true it shouldn't result in different behavior and can be compiled out in a build where debugging is disabled.
It's a bit debatable in this case, since that depends on assumptions about callers outside of wined3d.
2015-02-18 16:44 GMT+01:00 Henri Verbeet hverbeet@gmail.com:
On 18 February 2015 at 16:07, Stefan Dösinger stefandoesinger@gmail.com wrote:
Am 2015-02-18 um 14:20 schrieb Matteo Bruni:
- if (pitch & (texture_resource->format->byte_count - 1))
I don't think this would do the right thing for formats where byte_count isn't a power of two. I also think the check belongs in wined3d_texture_update_desc(), since otherwise the texture would be in an inconsistent state if it fails. (That actually also applies to failing to create the DIB section, but that one seems much harder to avoid.)
In theory there are no non-power of two byte_count texture formats from ddraw and d3d9ex always passes 0 as pitch. Actually initially I wrote the check as:
if (WARN_ON(d3d) && pitch % texture_resource->format->byte_count)
I can go back to something like that, it's probably more consistent with the point below.
- {
WARN("Pitch unsupported, not a multiple of the texture format byte width.\n");return WINED3DERR_INVALIDCALL;- }
I'd prefer this to be an ERR instead, and the return statement removed. The idea is that since we don't ever expect this check to be true it shouldn't result in different behavior and can be compiled out in a build where debugging is disabled.
It's a bit debatable in this case, since that depends on assumptions about callers outside of wined3d.
Yes, that was my reasoning for using WARN. I can change that though. I'm certainly going to move the check to wined3d_texture_update_desc() as you suggest.
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
Am 2015-02-18 um 17:10 schrieb Matteo Bruni:
2015-02-18 16:44 GMT+01:00 Henri Verbeet hverbeet@gmail.com:
It's a bit debatable in this case, since that depends on assumptions about callers outside of wined3d.
Yes, that was my reasoning for using WARN. I can change that though. I'm certainly going to move the check to wined3d_texture_update_desc() as you suggest.
Fwiw, I don't have strong feelings about this, so feel free to do what you think fits best. My thinking is that the entire patch is based on assumptions about the client libraries. If this assumption breaks it's worth making the resulting message more verbose than a WARN.
On 18 February 2015 at 17:25, Stefan Dösinger stefandoesinger@gmail.com wrote:
Fwiw, I don't have strong feelings about this, so feel free to do what you think fits best. My thinking is that the entire patch is based on assumptions about the client libraries.
The conceptual difference is that if we just reject the pitch in wined3d it becomes part of the API, so if e.g. ddraw tries to set an invalid pitch that's ddraw's problem. While if we don't, it's like we're pretending that pitches like that are supported, but we really hope nothing ever uses them, so if something sets an invalid pitch it's wined3d's problem to make it work somehow. The distinction is of course a bit artificial because everything is under our control, but the former seems more proper to me.
If this assumption breaks it's worth making the resulting message more verbose than a WARN.
Possibly, although at least ddraw_surface_create() will already print an ERR if wined3d_texture_update_desc() fails.
-
Henri Verbeet -
Matteo Bruni -
Stefan Dösinger