[PATCH 0/3] MR1014: wined3d: fixing a couple of integral types related issues.
Fixes inconsistent function (or function pointer) declation and implementation. (favoring non long types). Fixes errouneous format specifier in printf-like function for arguments of type DWORD_PTR (or SIZE_T or equiv). This serie could be a starting point for enabling long type compilation in wined3d. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1014
From: Eric Pouech <eric.pouech(a)gmail.com> Signed-off-by: Eric Pouech <eric.pouech(a)gmail.com> --- dlls/wined3d/wined3d_private.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dlls/wined3d/wined3d_private.h b/dlls/wined3d/wined3d_private.h index 5217ee7d221..b02bf1bef36 100644 --- a/dlls/wined3d/wined3d_private.h +++ b/dlls/wined3d/wined3d_private.h @@ -2939,7 +2939,7 @@ void context_gl_apply_texture_draw_state(struct wined3d_context_gl *context_gl, void context_gl_resource_released(struct wined3d_device *device, GLuint name, BOOL rb_namespace) DECLSPEC_HIDDEN; void context_invalidate_compute_state(struct wined3d_context *context, DWORD state_id) DECLSPEC_HIDDEN; -void context_invalidate_state(struct wined3d_context *context, DWORD state_id) DECLSPEC_HIDDEN; +void context_invalidate_state(struct wined3d_context *context, unsigned int state_id) DECLSPEC_HIDDEN; void context_resource_released(const struct wined3d_device *device, struct wined3d_resource *resource) DECLSPEC_HIDDEN; void context_restore(struct wined3d_context *context, struct wined3d_texture *texture, unsigned int sub_resource_idx) DECLSPEC_HIDDEN; @@ -4055,7 +4055,7 @@ LRESULT device_process_message(struct wined3d_device *device, HWND window, BOOL UINT message, WPARAM wparam, LPARAM lparam, WNDPROC proc) DECLSPEC_HIDDEN; void device_resource_add(struct wined3d_device *device, struct wined3d_resource *resource) DECLSPEC_HIDDEN; void device_resource_released(struct wined3d_device *device, struct wined3d_resource *resource) DECLSPEC_HIDDEN; -void device_invalidate_state(const struct wined3d_device *device, DWORD state) DECLSPEC_HIDDEN; +void device_invalidate_state(const struct wined3d_device *device, unsigned int state) DECLSPEC_HIDDEN; HRESULT wined3d_device_set_implicit_swapchain(struct wined3d_device *device, struct wined3d_swapchain *swapchain) DECLSPEC_HIDDEN; void wined3d_device_uninit_3d(struct wined3d_device *device) DECLSPEC_HIDDEN; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1014
From: Eric Pouech <eric.pouech(a)gmail.com> wined3d_resource_ops.resource_sub_resource_map() Signed-off-by: Eric Pouech <eric.pouech(a)gmail.com> --- dlls/wined3d/texture.c | 2 +- dlls/wined3d/wined3d_private.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/dlls/wined3d/texture.c b/dlls/wined3d/texture.c index a065d0ae54f..c76a50ea6ae 100644 --- a/dlls/wined3d/texture.c +++ b/dlls/wined3d/texture.c @@ -3689,7 +3689,7 @@ static void texture_resource_sub_resource_get_map_pitch(struct wined3d_resource } static HRESULT texture_resource_sub_resource_map(struct wined3d_resource *resource, unsigned int sub_resource_idx, - void **map_ptr, const struct wined3d_box *box, DWORD flags) + void **map_ptr, const struct wined3d_box *box, uint32_t flags) { struct wined3d_texture_sub_resource *sub_resource; struct wined3d_device *device = resource->device; diff --git a/dlls/wined3d/wined3d_private.h b/dlls/wined3d/wined3d_private.h index b02bf1bef36..b7d3b579ed2 100644 --- a/dlls/wined3d/wined3d_private.h +++ b/dlls/wined3d/wined3d_private.h @@ -4393,7 +4393,7 @@ struct wined3d_resource_ops void (*resource_sub_resource_get_map_pitch)(struct wined3d_resource *resource, unsigned int sub_resource_idx, unsigned int *row_pitch, unsigned int *slice_pitch); HRESULT (*resource_sub_resource_map)(struct wined3d_resource *resource, unsigned int sub_resource_idx, - void **map_ptr, const struct wined3d_box *box, DWORD flags); + void **map_ptr, const struct wined3d_box *box, uint32_t flags); HRESULT (*resource_sub_resource_unmap)(struct wined3d_resource *resource, unsigned int sub_resource_idx); }; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1014
From: Eric Pouech <eric.pouech(a)gmail.com> Signed-off-by: Eric Pouech <eric.pouech(a)gmail.com> --- dlls/wined3d/buffer.c | 2 +- dlls/wined3d/context_vk.c | 4 ++-- dlls/wined3d/device.c | 2 +- dlls/wined3d/shader_sm4.c | 6 +++--- dlls/wined3d/wined3d_main.c | 2 +- 5 files changed, 8 insertions(+), 8 deletions(-) diff --git a/dlls/wined3d/buffer.c b/dlls/wined3d/buffer.c index 3a2263d109f..1a953f8e218 100644 --- a/dlls/wined3d/buffer.c +++ b/dlls/wined3d/buffer.c @@ -282,7 +282,7 @@ static BOOL buffer_process_converted_attribute(struct wined3d_buffer *buffer, DWORD_PTR idx = (data + i) % buffer->stride; if (buffer->conversion_map[idx] != conversion_type) { - TRACE("Byte %lu in vertex changed:\n", idx); + TRACE("Byte %Iu in vertex changed:\n", idx); TRACE(" It was type %#x, is %#x now.\n", buffer->conversion_map[idx], conversion_type); ret = TRUE; buffer->conversion_map[idx] = conversion_type; diff --git a/dlls/wined3d/context_vk.c b/dlls/wined3d/context_vk.c index d47f7cf5d1a..13700c6dfb9 100644 --- a/dlls/wined3d/context_vk.c +++ b/dlls/wined3d/context_vk.c @@ -781,7 +781,7 @@ static void wined3d_bo_slab_vk_free_slice(struct wined3d_bo_slab_vk *slab, struct wined3d_bo_slab_vk_key key; struct wine_rb_entry *entry; - TRACE("slab %p, idx %lu, context_vk %p.\n", slab, idx, context_vk); + TRACE("slab %p, idx %Iu, context_vk %p.\n", slab, idx, context_vk); wined3d_device_vk_allocator_lock(device_vk); @@ -819,7 +819,7 @@ static void wined3d_context_vk_destroy_bo_slab_slice(struct wined3d_context_vk * if (!(o = wined3d_context_vk_get_retired_object_vk(context_vk))) { - ERR("Leaking slab %p, slice %#lx.\n", slab, idx); + ERR("Leaking slab %p, slice %#Ix.\n", slab, idx); return; } diff --git a/dlls/wined3d/device.c b/dlls/wined3d/device.c index e214c47f071..d1c18177d62 100644 --- a/dlls/wined3d/device.c +++ b/dlls/wined3d/device.c @@ -1112,7 +1112,7 @@ bool wined3d_device_gl_create_bo(struct wined3d_device_gl *device_gl, struct win GLsizeiptr buffer_offset = 0; GLuint id = 0; - TRACE("device_gl %p, context_gl %p, size %lu, binding %#x, usage %#x, coherent %#x, flags %#x, bo %p.\n", + TRACE("device_gl %p, context_gl %p, size %Iu, binding %#x, usage %#x, coherent %#x, flags %#x, bo %p.\n", device_gl, context_gl, size, binding, usage, coherent, flags, bo); if (gl_info->supported[ARB_BUFFER_STORAGE]) diff --git a/dlls/wined3d/shader_sm4.c b/dlls/wined3d/shader_sm4.c index 73001d90833..62aba18a235 100644 --- a/dlls/wined3d/shader_sm4.c +++ b/dlls/wined3d/shader_sm4.c @@ -1717,7 +1717,7 @@ static void shader_sm4_read_instruction(void *data, const DWORD **ptr, struct wi } if (!len || remaining < len) { - WARN("Read invalid length %u (remaining %lu).\n", len, remaining); + WARN("Read invalid length %u (remaining %Iu).\n", len, remaining); goto fail; } --len; @@ -1920,7 +1920,7 @@ static HRESULT parse_dxbc(const char *data, SIZE_T data_size, if (chunk_offset >= data_size || !require_space(chunk_offset, 2, sizeof(DWORD), data_size)) { - WARN("Invalid chunk offset %#x (data size %#lx).\n", chunk_offset, data_size); + WARN("Invalid chunk offset %#x (data size %#Ix).\n", chunk_offset, data_size); return E_FAIL; } @@ -1931,7 +1931,7 @@ static HRESULT parse_dxbc(const char *data, SIZE_T data_size, if (!require_space(chunk_ptr - data, 1, chunk_size, data_size)) { - WARN("Invalid chunk size %#x (data size %#lx, chunk offset %#x).\n", + WARN("Invalid chunk size %#x (data size %#Ix, chunk offset %#x).\n", chunk_size, data_size, chunk_offset); return E_FAIL; } diff --git a/dlls/wined3d/wined3d_main.c b/dlls/wined3d/wined3d_main.c index 4b718746b6a..2e4dda2c473 100644 --- a/dlls/wined3d/wined3d_main.c +++ b/dlls/wined3d/wined3d_main.c @@ -630,7 +630,7 @@ static LRESULT CALLBACK wined3d_wndproc(HWND window, UINT message, WPARAM wparam { if (filter && message != WM_DISPLAYCHANGE) { - TRACE("Filtering message: window %p, message %#x, wparam %#lx, lparam %#lx.\n", + TRACE("Filtering message: window %p, message %#x, wparam %#Ix, lparam %#Ix.\n", window, message, wparam, lparam); if (unicode) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1014
On 10/7/22 04:57, eric pouech (@epo) wrote:
This serie could be a starting point for enabling long type compilation in wined3d.
Maybe, although I think it may make sense to avoid tying wined3d to a certain compiler target where possible, given its frequency of use as a porting tool. Along the same lines, it's not unlikely that we'll move parts of it to vkd3d, and I'm not sure that we're going to end up using LLP64 there.
This merge request was approved by Zebediah Figura. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1014
On Mon Oct 10 00:33:29 2022 +0000, **** wrote: > Zebediah Figura replied on the mailing list: > ``` > On 10/7/22 04:57, eric pouech (@epo) wrote: > > This serie could be a starting point for enabling long type > compilation in wined3d. > Maybe, although I think it may make sense to avoid tying wined3d to a > certain compiler target where possible, given its frequency of use as a > porting tool. Along the same lines, it's not unlikely that we'll move > parts of it to vkd3d, and I'm not sure that we're going to end up using > LLP64 there. > ``` Does it mean you'd rather wait for that migration to happen before considering migating to long types, or you'd favor long type migration by reducing at max the trace/printf changes (so that it doesn't change twice when considering the two migrations). If the second case, note that: - it means changing types so that they are the same in lp64 and llp64 (mainly no longer using dword & long, but int:s), example: moving all flags variable / parameters from DWORD to unsigned - but some plzces will could still require trace changes (eg NSTATUS or GetLastError where I think it s better to keep the windows type for clarity) - it's not very clear what would be preferred between using unsigned vs. uint32_t vs UINT. Current code uses the three forms but couldn't find out a pattern on when to use which. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1014#note_10112
On 10/10/22 05:13, eric pouech (@epo) wrote:
Does it mean you'd rather wait for that migration to happen before considering migating to long types, or you'd favor long type migration by reducing at max the trace/printf changes (so that it doesn't change twice when considering the two migrations). If the second case, note that: - it means changing types so that they are the same in lp64 and llp64 (mainly no longer using dword & long, but int:s), example: moving all flags variable / parameters from DWORD to unsigned - but some plzces will could still require trace changes (eg NSTATUS or GetLastError where I think it s better to keep the windows type for clarity)
I think it would potentially make sense to get rid of long-typed variables in wined3d, though this gets a bit awkward, since e.g. DWORD is propagated in some places from the public interface, and HRESULT is relatively semantically charged. Mostly at this point I'd just advocate inaction.
- it's not very clear what would be preferred between using unsigned vs. uint32_t vs UINT. Current code uses the three forms but couldn't find out a pattern on when to use which.
UINT should probably be replaced with unsigned int. uint32_t would, I think, be preferred when something does specifically need to be 32 bits (e.g. I would prefer it to DWORD for shader token parsing).
On Mon Oct 10 18:13:11 2022 +0000, **** wrote:
Zebediah Figura replied on the mailing list: ``` On 10/10/22 05:13, eric pouech (@epo) wrote:
Does it mean you'd rather wait for that migration to happen before considering migating to long types, or you'd favor long type migration by reducing at max the trace/printf changes (so that it doesn't change twice when considering the two migrations). If the second case, note that: - it means changing types so that they are the same in lp64 and llp64 (mainly no longer using dword & long, but int:s), example: moving all flags variable / parameters from DWORD to unsigned - but some plzces will could still require trace changes (eg NSTATUS or GetLastError where I think it s better to keep the windows type for clarity) I think it would potentially make sense to get rid of long-typed variables in wined3d, though this gets a bit awkward, since e.g. DWORD is propagated in some places from the public interface, and HRESULT is relatively semantically charged. Mostly at this point I'd just advocate inaction. - it's not very clear what would be preferred between using unsigned vs. uint32_t vs UINT. Current code uses the three forms but couldn't find out a pattern on when to use which. UINT should probably be replaced with unsigned int. uint32_t would, I think, be preferred when something does specifically need to be 32 bits (e.g. I would prefer it to DWORD for shader token parsing).
I had started writing a reply to this, but Zeb answered before I finished it. Anyway:
it's not very clear what would be preferred between using unsigned vs. uint32_t vs UINT. Current code uses the three forms but couldn't find out a pattern on when to use which.
It's not consistent because there's quite some amount of older code inside wined3d, but in principle: * Array sizes and similar quantities like resource map ranges that in principle are limited by the address space of the process use `size_t`. * Explicitly 64-bit variables use `uint64_t`. There aren't very many of these; mostly things like Vulkan command buffer IDs or fence IDs. * Fixed-width variables like flags, masks, bitmaps, etc. use `uint32_t`. * Signed types are rare; in general integer variables are unsigned unless they can legitimately take on negative values. * Loop counter types are generally dictated by the thing they're iterating over; typically these should be `size_t`. * General integer types use `unsigned int`. Typically these are things we pass from D3D to OpenGL or Vulkan in one form or another. * We generally avoid Win32 types like `DWORD`/`UINT`/`ULONG`/`WORD`/`BYTE` in new code, preferring the standard C89/C99 types where possible. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1014#note_10151
participants (5)
-
Eric Pouech -
eric pouech (＠epo)
-
Henri Verbeet (＠hverbeet)
-
Zebediah Figura -
Zebediah Figura (＠zfigura)