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@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@gmail.com:
On 10 February 2015 at 20:14, Matteo Bruni mbruni@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.
On 11 February 2015 at 10:46, Stefan Dösinger stefandoesinger@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@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.
-
Henri Verbeet -
Matteo Bruni -
Stefan Dösinger