Re: [PATCH 4/5] wined3d: Set GL_UNPACK_ALIGNMENT to 1 when uploading surfaces from user memory.
On 10 February 2015 at 20:14, Matteo Bruni <mbruni(a)codeweavers.com> wrote:
gl_info->gl_ops.gl.p_glPixelStorei(GL_UNPACK_ROW_LENGTH, src_pitch / format->byte_count); + if (src_pitch & (surface_alignment - 1)) + gl_info->gl_ops.gl.p_glPixelStorei(GL_UNPACK_ALIGNMENT, 1);
What's a bit awkward about this is that if the pitch isn't a multiple of the pixel byte width it's still not going to work. But I guess this is the best we can do.
2015-02-10 22:05 GMT+01:00 Henri Verbeet <hverbeet(a)gmail.com>:
On 10 February 2015 at 20:14, Matteo Bruni <mbruni(a)codeweavers.com> wrote:
gl_info->gl_ops.gl.p_glPixelStorei(GL_UNPACK_ROW_LENGTH, src_pitch / format->byte_count); + if (src_pitch & (surface_alignment - 1)) + gl_info->gl_ops.gl.p_glPixelStorei(GL_UNPACK_ALIGNMENT, 1);
What's a bit awkward about this is that if the pitch isn't a multiple of the pixel byte width it's still not going to work. But I guess this is the best we can do.
Yes, I think the only 100% foolproof way would be to manually upload row by row in that case. FWIW OpenGL ES 2.0 has only GL_UNPACK_ALIGNMENT (i.e. no GL_UNPACK_ROW_LENGTH or anything else) so we'll need something like that if we ever want to support it. ES 3.0 does have GL_UNPACK_ROW_LENGTH instead so it's in the same spot as desktop GL in that regard. For the time being, I could add a check (maybe inside a WARN_ON() to avoid the overhead in the non-debug case) with a FIXME if the unlucky case happens.
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Am 2015-02-10 um 22:05 schrieb Henri Verbeet:
What's a bit awkward about this is that if the pitch isn't a multiple of the pixel byte width it's still not going to work. But I guess this is the best we can do. Does this ever happen? D3D9 doesn't allow specifying a manual pitch, and ddraw has at most 4 byte pixels and enforces a pitch alignment of 4 according to our tests.
Technically 24-bit RGB8 can have a misaligned pitch, but that isn't supported anywhere. -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBAgAGBQJU2yTzAAoJEN0/YqbEcdMwLhIQAIkLV+BPTLJ2slTNnXHPoVM7 P3TnhzWoHh27bMSoWDU97SeMe4cfRFuSmBPKxaKtKVJJXM6y/3sFQ6297o4Ki/3p hkxZ6uS8unb5VFT3SkTDRjo+F0s6RIfS2qwBzJuCuPhwhDZsjgPkxE2Y8PQhCKb9 9JbEwmIgcgQRqdTx+BEDD4ZmxBqZl3kcRupaLQcmIrmlb/DDgXtuAoWb73YI3E0+ Mlg8R4XM5dCBJnhbVnp1CIgBV5yJx5VnbhOQdSomqe+blqNFh32g//X/Qbh61LBv beR+dt0howlGtBXOE3Rwtv3tnOC2RnpcLPsjHbR4w/RmRGgabKLa1N9X11KaF0ek Oie3fsW0m7sJzfvfZzprpZloXtT/gqcarpU9h1CBSvAx2p6i6RhQRzN0LyU/Cvrh Ckjt7UVJ+OVrAMxIc7vzkSDJ8pb/qvEkmJQYygZeIZaBGrpYk4AtDJJ76W1rxi5f jbrOS+yJSDyJ+bXTrAOw9igWxqVA9MR51stZUjhYu9prhjSd2qrelnu8kGPT+zqr sWdOq5AxdBWYdDzP3m/lQfaSUvsoDkgNcGkITWClKBgCJ+4BdWrhbU4f0ZO+NrGl WCUAoVg8ZAhQilpSnm+yTw2zHeb+rbpqeEMESmFI1pqpRJtRMpH6FC3vMsgNTyKF 7tJNcTfWwJxDKRi5+fn7 =4PUD -----END PGP SIGNATURE-----
On 11 February 2015 at 10:46, Stefan Dösinger <stefandoesinger(a)gmail.com> wrote:
Am 2015-02-10 um 22:05 schrieb Henri Verbeet:
What's a bit awkward about this is that if the pitch isn't a multiple of the pixel byte width it's still not going to work. But I guess this is the best we can do. Does this ever happen? D3D9 doesn't allow specifying a manual pitch, and ddraw has at most 4 byte pixels and enforces a pitch alignment of 4 according to our tests.
Technically 24-bit RGB8 can have a misaligned pitch, but that isn't supported anywhere.
You're probably right. Perhaps we can just explicitly reject pitches that aren't a multiple of the pixel byte width in wined3d_surface_update_desc().
2015-02-11 10:46 GMT+01:00 Stefan Dösinger <stefandoesinger(a)gmail.com>:
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
Am 2015-02-10 um 22:05 schrieb Henri Verbeet:
What's a bit awkward about this is that if the pitch isn't a multiple of the pixel byte width it's still not going to work. But I guess this is the best we can do. Does this ever happen? D3D9 doesn't allow specifying a manual pitch, and ddraw has at most 4 byte pixels and enforces a pitch alignment of 4 according to our tests.
Technically 24-bit RGB8 can have a misaligned pitch, but that isn't supported anywhere.
It looks like that's correct WRT the client APIs but not for formats used internally by wined3d. I tried to set GL_UNPACK_ALIGNMENT to 1 in context_create and that fails volume_v16u16_test in d3d9:visual on Mesa. It fails because we're using GL_RGB16 to emulate D3DFMT_V16U16 when NV_texture_shader isn't supported (and the texture is 1x2x2 which luckily triggers the bug). I think conversions are the only cases where we can currently get pitches non multiple of the pixel byte width and we can fix those by either using a different pitch (we're free to choose it anyway) or avoiding those 3-channels formats altogether. The attached patch takes the former approach. Otherwise I can still go ahead with this patch but it would be a bit cumbersome to have to set UNPACK_ALIGNMENT every time when once per context should be enough (unless I'm missing something).
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Am 2015-02-18 um 14:23 schrieb Matteo Bruni:
It looks like that's correct WRT the client APIs but not for formats used internally by wined3d. I tried to set GL_UNPACK_ALIGNMENT to 1 in context_create and that fails volume_v16u16_test in d3d9:visual on Mesa. It fails because we're using GL_RGB16 to emulate D3DFMT_V16U16 when NV_texture_shader isn't supported (and the texture is 1x2x2 which luckily triggers the bug). I'm aware of some problems with those conversions, and I am working on a test and some fixes right now.
I think conversions are the only cases where we can currently get pitches non multiple of the pixel byte width and we can fix those by either using a different pitch (we're free to choose it anyway) or avoiding those 3-channels formats altogether. The attached patch takes the former approach. The reason why I chose to use the same pitch alignment was because we set GL_UNPACK_ALIGNMENT globally. So once that setting is gone I don't think anything is stopping us from choosing a different pitch.
The V8U8 and V16VU16 converts should be switched to RGBA types (but RGB internals) regardless of that. The drivers don't like non-padded formats either. -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBAgAGBQJU5Ja0AAoJEN0/YqbEcdMwr5wQAILo3M6FH39zKmW5RTASlQbG cx/KHC+P/dnIUtQCoGYQmMzDvHY4XYfY5CsEIF9Rc/1hELQxz4FqMRKk56j6K0AZ 7Vq4zdQFWMbdq+3rbj3YHZrMoWCTBbwc+4Pz08YcaGXRlldJVt5tHS5lisTyUN6Q NZnxQZkXcXX9SZaVgUeynez4XfvMwwHn8wBzd15qtMSyBMechUkAkTcVs9mJB41C B2BMuCuHuMm4MkdfnKIcG5IeyHRcpbrpFRH8Hh/vFFLJSqKYEP6XD2jhAuM3Nyqz ek+uTndNhrrv7ffPMe5YNQvlAjdODhNwZVNIR7EHaIf/3R2fRLG0P4I/zN15Kt0s rFQPlYvcL0/8oCKY3H0NReLh+uTrXmH8E+lallUKmba71OGJPxDLCDIfD0Tgfhlt 3SezJ9i0kBQeb1QAFnChpAzrtNMO1fNrYRQ1My1bJy/Z/WtdE/aF2dsSXU8lz04Y TQyVPuUEf9ZscAtmHjNll3Bnzd9IOs3sdor4xEqQtAgQ9NzhlmDzREgk0AnBD9CQ E3uf8bF5akVrE8STwHZlcWofRDO3ruq8pyOkJh+BBEUdrxaGoTSMlu6Kl/Ry63Ba Gy94/WegrSWx+IJfN1p3igfWFwOhhAP2GRjb8sKS7NKbv3cJJd1T8VMy6e40iMSO nZdH6xu1tmzphMnK1opl =EFgz -----END PGP SIGNATURE-----
participants (3)
-
Henri Verbeet -
Matteo Bruni -
Stefan Dösinger