November 2021 - Wine-Devel - list.winehq.org

[PATCH 3/3] wined3d: Avoid accessing the "bo" member of struct wined3d_buffer_gl.

by Henri Verbeet

From: Zebediah Figura <zfigura(a)codeweavers.com> Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> Signed-off-by: Henri Verbeet <hverbeet(a)codeweavers.com> --- v3: - Actually get rid of the cast - Get rid of some wined3d_buffer_gl() calls that are no longer needed now. This supersedes patch 219180. dlls/wined3d/buffer.c | 14 +++++++++----- dlls/wined3d/context_gl.c | 12 ++++++------ dlls/wined3d/state.c | 36 ++++++++++++++++++++++-------------- dlls/wined3d/view.c | 35 +++++++++++++++++++---------------- 4 files changed, 56 insertions(+), 41 deletions(-) diff --git a/dlls/wined3d/buffer.c b/dlls/wined3d/buffer.c index 2bdfff7c1b0..79ee68c177a 100644 --- a/dlls/wined3d/buffer.c +++ b/dlls/wined3d/buffer.c @@ -133,7 +133,9 @@ void wined3d_buffer_invalidate_location(struct wined3d_buffer *buffer, DWORD loc /* Context activation is done by the caller. */ static void wined3d_buffer_gl_bind(struct wined3d_buffer_gl *buffer_gl, struct wined3d_context_gl *context_gl) { - wined3d_context_gl_bind_bo(context_gl, buffer_gl->bo.binding, buffer_gl->bo.id); + const struct wined3d_bo_gl *bo_gl = wined3d_bo_gl(buffer_gl->b.buffer_object); + + wined3d_context_gl_bind_bo(context_gl, bo_gl->binding, bo_gl->id); } static GLenum wined3d_buffer_gl_binding_from_bind_flags(const struct wined3d_gl_info *gl_info, uint32_t bind_flags) @@ -225,7 +227,7 @@ static BOOL wined3d_buffer_gl_create_buffer_object(struct wined3d_buffer_gl *buf return FALSE; } - list_add_head(&buffer_gl->bo.b.users, &buffer_gl->b.bo_user.entry); + list_add_head(&bo->b.users, &buffer_gl->b.bo_user.entry); buffer_gl->b.buffer_object = &bo->b; buffer_invalidate_bo_range(&buffer_gl->b, 0, 0); @@ -1333,6 +1335,7 @@ static void wined3d_buffer_gl_upload_ranges(struct wined3d_buffer *buffer, struc const void *data, unsigned int data_offset, unsigned int range_count, const struct wined3d_range *ranges) { struct wined3d_context_gl *context_gl = wined3d_context_gl(context); + struct wined3d_bo_gl *bo_gl = wined3d_bo_gl(buffer->buffer_object); struct wined3d_buffer_gl *buffer_gl = wined3d_buffer_gl(buffer); const struct wined3d_gl_info *gl_info = context_gl->gl_info; const struct wined3d_range *range; @@ -1345,10 +1348,10 @@ static void wined3d_buffer_gl_upload_ranges(struct wined3d_buffer *buffer, struc while (range_count--) { range = &ranges[range_count]; - GL_EXTCALL(glBufferSubData(buffer_gl->bo.binding, + GL_EXTCALL(glBufferSubData(bo_gl->binding, range->offset, range->size, (BYTE *)data + range->offset - data_offset)); } - wined3d_context_gl_reference_bo(context_gl, &buffer_gl->bo); + wined3d_context_gl_reference_bo(context_gl, bo_gl); checkGLcall("buffer upload"); } @@ -1357,6 +1360,7 @@ static void wined3d_buffer_gl_download_ranges(struct wined3d_buffer *buffer, str void *data, unsigned int data_offset, unsigned int range_count, const struct wined3d_range *ranges) { struct wined3d_context_gl *context_gl = wined3d_context_gl(context); + struct wined3d_bo_gl *bo_gl = wined3d_bo_gl(buffer->buffer_object); struct wined3d_buffer_gl *buffer_gl = wined3d_buffer_gl(buffer); const struct wined3d_gl_info *gl_info = context_gl->gl_info; const struct wined3d_range *range; @@ -1369,7 +1373,7 @@ static void wined3d_buffer_gl_download_ranges(struct wined3d_buffer *buffer, str while (range_count--) { range = &ranges[range_count]; - GL_EXTCALL(glGetBufferSubData(buffer_gl->bo.binding, + GL_EXTCALL(glGetBufferSubData(bo_gl->binding, range->offset, range->size, (BYTE *)data + range->offset - data_offset)); } checkGLcall("buffer download"); diff --git a/dlls/wined3d/context_gl.c b/dlls/wined3d/context_gl.c index d63b928ca70..210d8e0b925 100644 --- a/dlls/wined3d/context_gl.c +++ b/dlls/wined3d/context_gl.c @@ -4335,12 +4335,12 @@ void dispatch_compute(struct wined3d_device *device, const struct wined3d_state if (parameters->indirect) { const struct wined3d_indirect_dispatch_parameters *indirect = &parameters->u.indirect; - struct wined3d_buffer_gl *buffer_gl = wined3d_buffer_gl(indirect->buffer); + struct wined3d_bo_gl *bo_gl = wined3d_bo_gl(indirect->buffer->buffer_object); - GL_EXTCALL(glBindBuffer(GL_DISPATCH_INDIRECT_BUFFER, buffer_gl->bo.id)); + GL_EXTCALL(glBindBuffer(GL_DISPATCH_INDIRECT_BUFFER, bo_gl->id)); GL_EXTCALL(glDispatchComputeIndirect((GLintptr)indirect->offset)); GL_EXTCALL(glBindBuffer(GL_DISPATCH_INDIRECT_BUFFER, 0)); - wined3d_context_gl_reference_bo(context_gl, &buffer_gl->bo); + wined3d_context_gl_reference_bo(context_gl, bo_gl); } else { @@ -4721,8 +4721,8 @@ static void draw_primitive_immediate_mode(struct wined3d_context_gl *context_gl, static void wined3d_context_gl_draw_indirect(struct wined3d_context_gl *context_gl, const struct wined3d_state *state, const struct wined3d_indirect_draw_parameters *parameters, unsigned int idx_size) { + struct wined3d_bo_gl *bo_gl = wined3d_bo_gl(parameters->buffer->buffer_object); GLenum gl_primitive_type = gl_primitive_type_from_d3d(state->primitive_type); - struct wined3d_buffer_gl *buffer_gl = wined3d_buffer_gl(parameters->buffer); const struct wined3d_gl_info *gl_info = context_gl->gl_info; const void *offset; @@ -4732,7 +4732,7 @@ static void wined3d_context_gl_draw_indirect(struct wined3d_context_gl *context_ return; } - GL_EXTCALL(glBindBuffer(GL_DRAW_INDIRECT_BUFFER, buffer_gl->bo.id)); + GL_EXTCALL(glBindBuffer(GL_DRAW_INDIRECT_BUFFER, bo_gl->id)); offset = (void *)(GLintptr)parameters->offset; if (idx_size) @@ -4748,7 +4748,7 @@ static void wined3d_context_gl_draw_indirect(struct wined3d_context_gl *context_ } GL_EXTCALL(glBindBuffer(GL_DRAW_INDIRECT_BUFFER, 0)); - wined3d_context_gl_reference_bo(context_gl, &buffer_gl->bo); + wined3d_context_gl_reference_bo(context_gl, bo_gl); checkGLcall("draw indirect"); } diff --git a/dlls/wined3d/state.c b/dlls/wined3d/state.c index 5e41f681c75..21b5ba1d032 100644 --- a/dlls/wined3d/state.c +++ b/dlls/wined3d/state.c @@ -4465,7 +4465,7 @@ static void indexbuffer(struct wined3d_context *context, const struct wined3d_st { const struct wined3d_gl_info *gl_info = wined3d_context_gl(context)->gl_info; const struct wined3d_stream_info *stream_info = &context->stream_info; - struct wined3d_buffer_gl *buffer_gl; + struct wined3d_buffer *buffer; if (!state->index_buffer || !stream_info->all_vbo) { @@ -4473,9 +4473,16 @@ static void indexbuffer(struct wined3d_context *context, const struct wined3d_st return; } - buffer_gl = wined3d_buffer_gl(state->index_buffer); - GL_EXTCALL(glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, buffer_gl->bo.id)); - buffer_gl->b.bo_user.valid = true; + buffer = state->index_buffer; + if (buffer->buffer_object) + { + GL_EXTCALL(glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, wined3d_bo_gl(buffer->buffer_object)->id)); + buffer->bo_user.valid = true; + } + else + { + GL_EXTCALL(glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, 0)); + } } static void depth_clip(const struct wined3d_rasterizer_state *r, const struct wined3d_gl_info *gl_info) @@ -4568,7 +4575,7 @@ static void state_cb(struct wined3d_context *context, const struct wined3d_state { const struct wined3d_gl_info *gl_info = wined3d_context_gl(context)->gl_info; enum wined3d_shader_type shader_type; - struct wined3d_buffer_gl *buffer_gl; + struct wined3d_buffer *buffer; unsigned int i, base, count; TRACE("context %p, state %p, state_id %#x.\n", context, state, state_id); @@ -4589,10 +4596,10 @@ static void state_cb(struct wined3d_context *context, const struct wined3d_state continue; } - buffer_gl = wined3d_buffer_gl(buffer_state->buffer); - GL_EXTCALL(glBindBufferRange(GL_UNIFORM_BUFFER, base + i, buffer_gl->bo.id, - buffer_state->offset, buffer_state->size)); - buffer_gl->b.bo_user.valid = true; + buffer = buffer_state->buffer; + GL_EXTCALL(glBindBufferRange(GL_UNIFORM_BUFFER, base + i, + wined3d_bo_gl(buffer->buffer_object)->id, buffer_state->offset, buffer_state->size)); + buffer->bo_user.valid = true; } checkGLcall("bind constant buffers"); } @@ -4642,7 +4649,7 @@ static void state_so(struct wined3d_context *context, const struct wined3d_state { struct wined3d_context_gl *context_gl = wined3d_context_gl(context); const struct wined3d_gl_info *gl_info = context_gl->gl_info; - struct wined3d_buffer_gl *buffer_gl; + struct wined3d_buffer *buffer; unsigned int offset, size, i; TRACE("context %p, state %p, state_id %#x.\n", context, state, state_id); @@ -4657,16 +4664,17 @@ static void state_so(struct wined3d_context *context, const struct wined3d_state continue; } - buffer_gl = wined3d_buffer_gl(state->stream_output[i].buffer); + buffer = state->stream_output[i].buffer; offset = state->stream_output[i].offset; if (offset == ~0u) { FIXME("Appending to stream output buffers not implemented.\n"); offset = 0; } - size = buffer_gl->b.resource.size - offset; - GL_EXTCALL(glBindBufferRange(GL_TRANSFORM_FEEDBACK_BUFFER, i, buffer_gl->bo.id, offset, size)); - buffer_gl->b.bo_user.valid = true; + size = buffer->resource.size - offset; + GL_EXTCALL(glBindBufferRange(GL_TRANSFORM_FEEDBACK_BUFFER, i, + wined3d_bo_gl(buffer->buffer_object)->id, offset, size)); + buffer->bo_user.valid = true; } checkGLcall("bind transform feedback buffers"); } diff --git a/dlls/wined3d/view.c b/dlls/wined3d/view.c index 8746d9168c2..cbcfed4a21e 100644 --- a/dlls/wined3d/view.c +++ b/dlls/wined3d/view.c @@ -245,10 +245,11 @@ static void create_texture_view(struct wined3d_gl_view *view, GLenum view_target } static void create_buffer_texture(struct wined3d_gl_view *view, struct wined3d_context_gl *context_gl, - struct wined3d_buffer_gl *buffer_gl, const struct wined3d_format_gl *view_format_gl, + struct wined3d_buffer *buffer, const struct wined3d_format_gl *view_format_gl, unsigned int offset, unsigned int size) { const struct wined3d_gl_info *gl_info = context_gl->gl_info; + const struct wined3d_bo_gl *bo_gl; if (!gl_info->supported[ARB_TEXTURE_BUFFER_OBJECT]) { @@ -263,7 +264,8 @@ static void create_buffer_texture(struct wined3d_gl_view *view, struct wined3d_c return; } - wined3d_buffer_load_location(&buffer_gl->b, &context_gl->c, WINED3D_LOCATION_BUFFER); + wined3d_buffer_load_location(buffer, &context_gl->c, WINED3D_LOCATION_BUFFER); + bo_gl = wined3d_bo_gl(buffer->buffer_object); view->target = GL_TEXTURE_BUFFER; if (!view->name) @@ -272,13 +274,13 @@ static void create_buffer_texture(struct wined3d_gl_view *view, struct wined3d_c wined3d_context_gl_bind_texture(context_gl, GL_TEXTURE_BUFFER, view->name); if (gl_info->supported[ARB_TEXTURE_BUFFER_RANGE]) { - GL_EXTCALL(glTexBufferRange(GL_TEXTURE_BUFFER, view_format_gl->internal, buffer_gl->bo.id, offset, size)); + GL_EXTCALL(glTexBufferRange(GL_TEXTURE_BUFFER, view_format_gl->internal, bo_gl->id, offset, size)); } else { - if (offset || size != buffer_gl->b.resource.size) + if (offset || size != buffer->resource.size) FIXME("OpenGL implementation does not support ARB_texture_buffer_range.\n"); - GL_EXTCALL(glTexBuffer(GL_TEXTURE_BUFFER, view_format_gl->internal, buffer_gl->bo.id)); + GL_EXTCALL(glTexBuffer(GL_TEXTURE_BUFFER, view_format_gl->internal, bo_gl->id)); } checkGLcall("Create buffer texture"); @@ -309,8 +311,7 @@ static void create_buffer_view(struct wined3d_gl_view *view, struct wined3d_cont unsigned int offset, size; get_buffer_view_range(buffer, desc, view_format, &offset, &size); - create_buffer_texture(view, wined3d_context_gl(context), - wined3d_buffer_gl(buffer), wined3d_format_gl(view_format), offset, size); + create_buffer_texture(view, wined3d_context_gl(context), buffer, wined3d_format_gl(view_format), offset, size); } static void wined3d_view_invalidate_location(struct wined3d_resource *resource, @@ -976,7 +977,7 @@ static void wined3d_shader_resource_view_gl_cs_init(void *object) context = context_acquire(resource->device, NULL, 0); create_buffer_view(&view_gl->gl_view, context, desc, buffer, view_format); view_gl->bo_user.valid = true; - list_add_head(&wined3d_buffer_gl(buffer)->bo.b.users, &view_gl->bo_user.entry); + list_add_head(&buffer->buffer_object->users, &view_gl->bo_user.entry); context_release(context); } else @@ -1480,8 +1481,9 @@ void wined3d_unordered_access_view_gl_clear(struct wined3d_unordered_access_view { const struct wined3d_gl_info *gl_info = context_gl->gl_info; const struct wined3d_format_gl *format_gl; - struct wined3d_buffer_gl *buffer_gl; struct wined3d_resource *resource; + struct wined3d_buffer *buffer; + struct wined3d_bo_gl *bo_gl; unsigned int offset, size; resource = view_gl->v.resource; @@ -1587,15 +1589,16 @@ void wined3d_unordered_access_view_gl_clear(struct wined3d_unordered_access_view return; } - buffer_gl = wined3d_buffer_gl(buffer_from_resource(resource)); - wined3d_buffer_load_location(&buffer_gl->b, &context_gl->c, WINED3D_LOCATION_BUFFER); + buffer = buffer_from_resource(resource); + wined3d_buffer_load_location(buffer, &context_gl->c, WINED3D_LOCATION_BUFFER); wined3d_unordered_access_view_invalidate_location(&view_gl->v, ~WINED3D_LOCATION_BUFFER); - get_buffer_view_range(&buffer_gl->b, &view_gl->v.desc, &format_gl->f, &offset, &size); - wined3d_context_gl_bind_bo(context_gl, buffer_gl->bo.binding, buffer_gl->bo.id); - GL_EXTCALL(glClearBufferSubData(buffer_gl->bo.binding, format_gl->internal, + bo_gl = wined3d_bo_gl(buffer->buffer_object); + get_buffer_view_range(buffer, &view_gl->v.desc, &format_gl->f, &offset, &size); + wined3d_context_gl_bind_bo(context_gl, bo_gl->binding, bo_gl->id); + GL_EXTCALL(glClearBufferSubData(bo_gl->binding, format_gl->internal, offset, size, format_gl->format, format_gl->type, clear_value)); - wined3d_context_gl_reference_bo(context_gl, &buffer_gl->bo); + wined3d_context_gl_reference_bo(context_gl, bo_gl); checkGLcall("clear unordered access view"); } @@ -1662,7 +1665,7 @@ static void wined3d_unordered_access_view_gl_cs_init(void *object) context_gl = wined3d_context_gl(context_acquire(resource->device, NULL, 0)); create_buffer_view(&view_gl->gl_view, &context_gl->c, desc, buffer, view_gl->v.format); view_gl->bo_user.valid = true; - list_add_head(&wined3d_buffer_gl(buffer)->bo.b.users, &view_gl->bo_user.entry); + list_add_head(&buffer->buffer_object->users, &view_gl->bo_user.entry); if (desc->flags & (WINED3D_VIEW_BUFFER_COUNTER | WINED3D_VIEW_BUFFER_APPEND)) { struct wined3d_bo_gl *bo = &view_gl->counter_bo; -- 2.20.1

Nov. 10, 2021

[PATCH 2/3] wined3d: Introduce wined3d_context_gl_reference_buffer().

by Henri Verbeet

From: Zebediah Figura <zfigura(a)codeweavers.com> Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> Signed-off-by: Henri Verbeet <hverbeet(a)codeweavers.com> --- Split from patch 219180. dlls/wined3d/context_gl.c | 62 ++++++++++++++++------------------ dlls/wined3d/wined3d_private.h | 11 ++++++ 2 files changed, 41 insertions(+), 32 deletions(-) diff --git a/dlls/wined3d/context_gl.c b/dlls/wined3d/context_gl.c index 09c871efcd4..d63b928ca70 100644 --- a/dlls/wined3d/context_gl.c +++ b/dlls/wined3d/context_gl.c @@ -3796,8 +3796,8 @@ static void context_gl_load_shader_resources(struct wined3d_context_gl *context_ struct wined3d_shader_sampler_map_entry *entry; struct wined3d_shader_resource_view_gl *srv_gl; struct wined3d_shader_resource_view *view; - struct wined3d_buffer_gl *buffer_gl; struct wined3d_shader *shader; + struct wined3d_buffer *buffer; unsigned int i, j; for (i = 0; i < WINED3D_SHADER_TYPE_COUNT; ++i) @@ -3813,10 +3813,10 @@ static void context_gl_load_shader_resources(struct wined3d_context_gl *context_ if (!state->cb[i][j].buffer) continue; - buffer_gl = wined3d_buffer_gl(state->cb[i][j].buffer); - wined3d_buffer_load(&buffer_gl->b, &context_gl->c, state); - wined3d_context_gl_reference_bo(context_gl, &buffer_gl->bo); - if (!buffer_gl->b.bo_user.valid) + buffer = state->cb[i][j].buffer; + wined3d_buffer_load(buffer, &context_gl->c, state); + wined3d_context_gl_reference_buffer(context_gl, buffer); + if (!buffer->bo_user.valid) device_invalidate_state(context_gl->c.device, STATE_CONSTANT_BUFFER(i)); } @@ -3829,9 +3829,9 @@ static void context_gl_load_shader_resources(struct wined3d_context_gl *context_ if (view->resource->type == WINED3D_RTYPE_BUFFER) { - buffer_gl = wined3d_buffer_gl(buffer_from_resource(view->resource)); - wined3d_buffer_load(&buffer_gl->b, &context_gl->c, state); - wined3d_context_gl_reference_bo(context_gl, &buffer_gl->bo); + buffer = buffer_from_resource(view->resource); + wined3d_buffer_load(buffer, &context_gl->c, state); + wined3d_context_gl_reference_buffer(context_gl, buffer); srv_gl = wined3d_shader_resource_view_gl(view); if (!srv_gl->bo_user.valid) @@ -3850,8 +3850,8 @@ static void context_gl_load_unordered_access_resources(struct wined3d_context_gl { struct wined3d_unordered_access_view_gl *uav_gl; struct wined3d_unordered_access_view *view; - struct wined3d_buffer_gl *buffer_gl; struct wined3d_texture *texture; + struct wined3d_buffer *buffer; unsigned int i; context_gl->c.uses_uavs = 0; @@ -3866,10 +3866,10 @@ static void context_gl_load_unordered_access_resources(struct wined3d_context_gl if (view->resource->type == WINED3D_RTYPE_BUFFER) { - buffer_gl = wined3d_buffer_gl(buffer_from_resource(view->resource)); - wined3d_buffer_load_location(&buffer_gl->b, &context_gl->c, WINED3D_LOCATION_BUFFER); + buffer = buffer_from_resource(view->resource); + wined3d_buffer_load_location(buffer, &context_gl->c, WINED3D_LOCATION_BUFFER); wined3d_unordered_access_view_invalidate_location(view, ~WINED3D_LOCATION_BUFFER); - wined3d_context_gl_reference_bo(context_gl, &buffer_gl->bo); + wined3d_context_gl_reference_buffer(context_gl, buffer); uav_gl = wined3d_unordered_access_view_gl(view); if (!uav_gl->bo_user.valid) @@ -3889,20 +3889,18 @@ static void context_gl_load_unordered_access_resources(struct wined3d_context_gl static void context_gl_load_stream_output_buffers(struct wined3d_context_gl *context_gl, const struct wined3d_state *state) { + struct wined3d_buffer *buffer; unsigned int i; for (i = 0; i < ARRAY_SIZE(state->stream_output); ++i) { - struct wined3d_buffer_gl *buffer_gl; - - if (!state->stream_output[i].buffer) + if (!(buffer = state->stream_output[i].buffer)) continue; - buffer_gl = wined3d_buffer_gl(state->stream_output[i].buffer); - wined3d_buffer_load(&buffer_gl->b, &context_gl->c, state); - wined3d_buffer_invalidate_location(&buffer_gl->b, ~WINED3D_LOCATION_BUFFER); - wined3d_context_gl_reference_bo(context_gl, &buffer_gl->bo); - if (!buffer_gl->b.bo_user.valid) + wined3d_buffer_load(buffer, &context_gl->c, state); + wined3d_buffer_invalidate_location(buffer, ~WINED3D_LOCATION_BUFFER); + wined3d_context_gl_reference_buffer(context_gl, buffer); + if (!buffer->bo_user.valid) device_invalidate_state(context_gl->c.device, STATE_STREAM_OUTPUT); } } @@ -3954,15 +3952,15 @@ static BOOL context_apply_draw_state(struct wined3d_context *context, while (map) { const struct wined3d_stream_info_element *e; - struct wined3d_buffer_gl *buffer_gl; + struct wined3d_buffer *buffer; e = &context->stream_info.elements[wined3d_bit_scan(&map)]; - buffer_gl = wined3d_buffer_gl(state->streams[e->stream_idx].buffer); + buffer = state->streams[e->stream_idx].buffer; - if (!buffer_gl->b.bo_user.valid) + if (!buffer->bo_user.valid) device_invalidate_state(device, STATE_STREAMSRC); else - wined3d_buffer_load(&buffer_gl->b, context, state); + wined3d_buffer_load(buffer, context, state); } /* Loading the buffers above may have invalidated the stream info. */ if (wined3d_context_is_graphics_state_dirty(context, STATE_STREAMSRC)) @@ -3972,28 +3970,28 @@ static BOOL context_apply_draw_state(struct wined3d_context *context, while (map) { const struct wined3d_stream_info_element *e; - struct wined3d_buffer_gl *buffer_gl; + struct wined3d_buffer *buffer; e = &context->stream_info.elements[wined3d_bit_scan(&map)]; - buffer_gl = wined3d_buffer_gl(state->streams[e->stream_idx].buffer); + buffer = state->streams[e->stream_idx].buffer; - wined3d_context_gl_reference_bo(context_gl, &buffer_gl->bo); + wined3d_context_gl_reference_buffer(context_gl, buffer); } if (indexed && state->index_buffer) { - struct wined3d_buffer_gl *buffer_gl = wined3d_buffer_gl(state->index_buffer); + struct wined3d_buffer *buffer = state->index_buffer; if (context->stream_info.all_vbo) { - wined3d_buffer_load(&buffer_gl->b, context, state); - if (!buffer_gl->b.bo_user.valid) + wined3d_buffer_load(buffer, context, state); + if (!buffer->bo_user.valid) device_invalidate_state(device, STATE_INDEXBUFFER); - wined3d_context_gl_reference_bo(context_gl, &buffer_gl->bo); + wined3d_context_gl_reference_buffer(context_gl, buffer); } else { - wined3d_buffer_load_sysmem(&buffer_gl->b, context); + wined3d_buffer_load_sysmem(buffer, context); } } diff --git a/dlls/wined3d/wined3d_private.h b/dlls/wined3d/wined3d_private.h index 2880a5c6b90..60702bc9777 100644 --- a/dlls/wined3d/wined3d_private.h +++ b/dlls/wined3d/wined3d_private.h @@ -1612,6 +1612,11 @@ struct wined3d_bo_gl uint64_t command_fence_id; }; +static inline struct wined3d_bo_gl *wined3d_bo_gl(struct wined3d_bo *bo) +{ + return CONTAINING_RECORD(bo, struct wined3d_bo_gl, b); +} + static inline GLuint wined3d_bo_gl_id(uintptr_t bo) { return bo ? ((struct wined3d_bo_gl *)bo)->id : 0; @@ -6581,6 +6586,12 @@ static inline void wined3d_context_gl_reference_bo(struct wined3d_context_gl *co bo_gl->command_fence_id = device_gl->current_fence_id; } +static inline void wined3d_context_gl_reference_buffer(struct wined3d_context_gl *context_gl, + struct wined3d_buffer *buffer) +{ + wined3d_context_gl_reference_bo(context_gl, wined3d_bo_gl(buffer->buffer_object)); +} + static inline bool wined3d_map_persistent(void) { return sizeof(void *) >= sizeof(uint64_t); -- 2.20.1

Nov. 10, 2021

[PATCH 1/3] ddraw: Use wined3d_bit_scan() in compute_sphere_visibility().

by Henri Verbeet

Rémi Bernon reports an unspecified issue with gcc 11 related to compute_sphere_visibility() expecting 12 input planes, but d3d_device3_ComputeSphereVisibility() only providing 6. The actual number of planes required depends on the "enabled_planes" mask. This patch should make the code better reflect that, but I do not have a gcc 11 setup to verify it resolves the issue there. Signed-off-by: Henri Verbeet <hverbeet(a)codeweavers.com> --- dlls/ddraw/device.c | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/dlls/ddraw/device.c b/dlls/ddraw/device.c index 8afbab0e58d..6e8f109173b 100644 --- a/dlls/ddraw/device.c +++ b/dlls/ddraw/device.c @@ -4638,17 +4638,20 @@ static void prepare_clip_space_planes(struct d3d_device *device, struct wined3d_ plane[5].w = m._44 - m._43; } -static void compute_sphere_visibility(struct wined3d_vec4 plane[12], DWORD enabled_planes, BOOL equality, - D3DVECTOR *centers, D3DVALUE *radii, DWORD sphere_count, DWORD *return_values) +static void compute_sphere_visibility(const struct wined3d_vec4 *planes, DWORD enabled_planes, BOOL equality, + const D3DVECTOR *centres, const D3DVALUE *radii, unsigned int sphere_count, DWORD *return_values) { - UINT i, j; + unsigned int mask, i, j; + memset(return_values, 0, sphere_count * sizeof(*return_values)); for (i = 0; i < sphere_count; ++i) { - return_values[i] = 0; - for (j = 0; j < 12; ++j) - if (enabled_planes & 1u << j) - return_values[i] |= in_plane(j, plane[j], centers[i], radii[i], equality); + mask = enabled_planes; + while (mask) + { + j = wined3d_bit_scan(&mask); + return_values[i] |= in_plane(j, planes[j], centres[i], radii[i], equality); + } } } -- 2.20.1

Nov. 10, 2021

Re: [PATCH vkd3d 5/5] vkd3d-shader/hlsl: Write SM4 sample instructions.

by Henri Verbeet

On Tue, 9 Nov 2021 at 04:56, Zebediah Figura <zfigura(a)codeweavers.com> wrote: > case HLSL_RESOURCE_SAMPLE: > - hlsl_fixme(ctx, load->node.loc, "Resource sample instruction."); > + if (!load->sampler.var) > + hlsl_fixme(ctx, load->node.loc, "SM4 combined sample expression.\n"); Is the extra newline intentional here?

Nov. 10, 2021

Re: [PATCH vkd3d 2/5] vkd3d-shader/hlsl: Perform a copy propagation pass.

by Giovanni Mascellani

Hi, On 09/11/21 23:21, Zebediah Figura wrote: > Overall this patch looks a lot better than I was afraid of. It's a lot > of code that's intimidating to review, but once you ignore the rbtree > boilerplate it's simple enough and seems about in line with what I > expect. There's quite a few things that I think can be simplified, but > the basic structure seems sound, so nice work. Great, that's good! Thanks for your review, I think most if not all of the points you wrote should be easy to fix. > The CF part is probably going to be a lot trickier, but fortunately I > think that this patch alone is the one that really matters. Frankly, if > we were to take this patch, and then a second patch that does copy-prop > on interior CF blocks with a fresh copy_propagation_state(), we might > even cover enough that it's not even worrying about CF... Mmh, note that you have to handle control flow in some way or another, you cannot just ignore it. This patch doesn't know about control flow, but the only way it can do it is by bailing out at the first control flow instruction in the program. It's not enough to ignore the control flow block and resume processing after it, because the inner block might invalidate some of your knowledge about the program state (in 3/5 patch this is done by copy_propagation_invalidate_from_block). >> +/* struct copy_propagation_state represents the accumulated knowledge >> + * of the copy propagation pass while it scans through the code. Field >> + * "variables" is a tree whose elements have type struct >> + * copy_propagation_varible, and represent each of the variables the >> + * pass has already encountered (except those with special semantics, >> + * which are ignored). For each variable, the array "values" (whose >> + * length is the register size of the variable) represent which node >> + * and which index inside that node (i.e., which of the at most four >> + * entries of a vector) provided that value last time. Field "node" >> + * can be NULL, meaning that the pass was not able to statically >> + * determine the node. >> + */ > > This comment feels a bit too low-level? I dunno, comments like this are > hard to review, but my inclination is to describe what you're doing at a > high level, along the lines of "we track the last known value of each > component of each variable", and let the code itself explain the details. Personally I would have loved to meet comments like mine in other areas of Wine: knowing what data read or written by an algorithm are meant to represent makes it much easier to understand why the algorithm does what it does. I agree that this is not the most complicated case around, but I thought this would be helpful for someone not already knowing my code. OTOH I understand that also a high-level description is useful: would you consider it satisfying to have both? >> +static int copy_propagation_variable_compare(const void *key, const >> struct rb_entry *entry) >> +{ >> + struct copy_propagation_variable *variable = >> RB_ENTRY_VALUE(entry, struct copy_propagation_variable, entry); >> + uintptr_t key_int = (uintptr_t)key, entry_int = >> (uintptr_t)variable->var; >> + >> + if (key_int < entry_int) >> + return -1; >> + else if (key_int > entry_int) >> + return 1; >> + else >> + return 0; > > Could we just modify the rbtree implementation to use uintptr_t instead, > and then do a direct subtraction? I am not sure of what you mean: the pointer in struct copy_propagation_variable is used as a pointer, it would not be very logical to store it as an integer. Also, do you get the right think if you subtract two unsigned integers? For one thing the result is unsigned, but even if you cast it to signed you don't get an order, do you? Even if you could, you would get a shorter code, but I am not sure it would be a more legible one. >> +static struct copy_propagation_variable >> *copy_propagation_get_variable(struct hlsl_ctx *ctx, >> + struct copy_propagation_state *state, struct hlsl_ir_var *var) >> +{ >> + struct rb_entry *entry = rb_get(&state->variables, var); >> + struct copy_propagation_variable *variable; >> + int res; >> + >> + if (entry) >> + return RB_ENTRY_VALUE(entry, struct >> copy_propagation_variable, entry); >> + >> + variable = hlsl_alloc(ctx, sizeof(*variable)); >> + if (!variable) >> + return NULL; >> + >> + variable->var = var; >> + variable->values = hlsl_alloc(ctx, sizeof(*variable->values) * >> var->data_type->reg_size); >> + if (!variable->values) >> + { >> + vkd3d_free(variable); >> + return NULL; >> + } >> + >> + res = rb_put(&state->variables, var, &variable->entry); >> + assert(!res); > > Although this is a bit awkward because most of the function is only > relevant for stores, not loads, and I think it would actually be better > to reflect that in the code, one way or another (so we can do less effort). Do less effort at what? It's true that when loading I could return NULL when the variable has never been seen before (meaning that we don't know anything about that variable, so the load cannot be removed) instead of creating an "empty" variable placeholder and return that one, but I don't really see an advantage doing that. It would seem more effort rather than less to me, because there are more possible branches to check. (BTW, that would also mean that the variable is read before being written, which is something that shouldn't normally happen, but I don't think it's sensible to count on that) >> + if (TRACE_ON()) >> + { >> + char buf[32]; >> + if (!node) >> + sprintf(buf, "(nil)"); > > Can that happen? Not yet, but it will as soon as loops and conditionals enter the scene, because they can cause the last store to a variable to become unknown (at compilation time). Now, I know that usually we don't like to have dead code around. I think this case could be accepted, since this code is just a line long and becomes alive at 3/5. Also, I find it easier to review this piece of code at once, instead of first reviewing a part, checking that the missing case will never happen and then review the missing case and checking that it fits well in the already-reviewed part. But if you really don't like it, I can defer the NULL case to 3/5. >> + else if (node->index) >> + sprintf(buf, "@%u", node->index); >> + else >> + sprintf(buf, "%p", node); > > Can that happen? Definitely. Arguably, it might be the common case, given that the earlier optimization passes (including this pass itself, given that it might be executed more than once) might synthesize a lot of nodes, which won't have an index until compute_liveness is executed. >> + TRACE("variable %s[%d] is written by %p[%d]\n", >> variable->var->name, offset + index, buf, index); > > This trace doesn't really match the usual format, but more saliently, > the hardcoded buffer is ugly. Assuming the other two cases really can > happen, I'd rather see individual traces spelled out. > > Same thing below. Ok. As usual, I found find that uglier, but I can live with that. BTW, having the traces is not a condicio sine qua non for me. They were useful for me to develop the patch, and I left them because I figured that they might also be useful in the future, but we can happily get rid of them. >> + struct hlsl_deref *src = &load->src; >> + struct hlsl_ir_var *var = src->var; >> + struct hlsl_ir_swizzle *swizzle; >> + DWORD s; >> + >> + if (var->is_input_semantic || var->is_output_semantic || >> var->is_uniform) >> + return false; > > For input semantics and uniforms: yeah, but do we need to? The important > question is "can we reconstruct a store for this variable", and this > should already be false. > > For output semantics, we should never get here in the first place. I think you're right. I had put this check initially because I was concentrated on other aspects of the algorithm, but it's true that it is overly cautious. >> + >> + if (type->type != HLSL_CLASS_SCALAR && type->type != >> HLSL_CLASS_VECTOR) >> + return false; >> + >> + offset = hlsl_offset_from_deref(src); > > The problem with hlsl_offset_from_deref(), and the reason I probably > should have fought against 62b25bc52b in its current form, is that it > not only doesn't deal with non-constant offsets, but doesn't really give > the caller a way to bail either. We should probably be returning bool > from it, and then aborting here. I agree. Actually, the first version of my patch set had a variant of hlsl_offset_from_deref that acknowledged the possibility that it could not be possible to statically determine the offset of a deref. This condition is difficult to handle at code generation time, but it is not particularly problematic here: at load time just ignore the load, and at store time invalidate the whole variable. I can totally fix hlsl_offset_from_deref so that this happens. (speaking of constant expressions in the code, I am a bit bothered that evaluate_array_dimension is not able to figure out the value of constant expressions that are not immediate constants, and unfortunately fixing that is not as easy as running a fold_constants pass; I don't in mind a clean solution for that that doesn't require re-implementing constant folding there; that's another matter, of course) >> + >> + variable = copy_propagation_get_variable(ctx, state, var); >> + if (!variable) >> + return false; >> + >> + new_node = copy_propagation_reconstruct_node(variable, offset, >> type->dimx, indices); >> + >> + if (TRACE_ON()) >> + { >> + char buf[32]; >> + if (!new_node) >> + sprintf(buf, "(nil)"); > > Is this useful to trace? As before, it is a matter of tastes, I believe. I can drop it, just as I can drop the traces above. >> + else if (new_node->index) >> + sprintf(buf, "@%u", new_node->index); >> + else >> + sprintf(buf, "%p", new_node); > > Can this happen? Yes, just as above. >> + variable = copy_propagation_get_variable(ctx, state, var); >> + if (!variable) >> + return false; >> + >> + copy_propagation_set_value(variable, hlsl_offset_from_deref(lhs), >> store->writemask, store->rhs.node); >> + >> + return false; >> +} > > Shouldn't this function just return void? It's a matter of philosophy. If you ask me, the switch in copy_propagation_recursive should be rather oblivious of what the various helpers actually do, it shouldn't have an insight that copy_propagation_store never returns true. For its point of view, any helper can potentially modify the code. Though I can change to void if you prefer. >> +static bool copy_propagation_recursive(struct hlsl_ctx *ctx, struct >> hlsl_block *block, >> + struct copy_propagation_state *state) So, no comment for a function called "recursive" which is not (yet) actually recursive? :-P >> @@ -1354,7 +1601,12 @@ int hlsl_emit_dxbc(struct hlsl_ctx *ctx, struct >> hlsl_ir_function_decl *entry_fun >> progress |= transform_ir(ctx, split_struct_copies, body, NULL); >> } >> while (progress); >> - while (transform_ir(ctx, fold_constants, body, NULL)); >> + do >> + { >> + progress = transform_ir(ctx, fold_constants, body, NULL); >> + progress |= copy_propagation_pass(ctx, body); > > This probably isn't worth examining, but I'm curious why constant > folding is useful after copy-prop. Copy propagation might enable some additional steps of constant folding, if a constant is first stored, than loaded and then an operation is carried on it. OTOH, constant folding can enable some additional step of copy propagation if it makes an offset statically visible. Thanks, Giovanni.

Nov. 10, 2021

[PATCH 7/7] dinput: Fix DIEFF_CARTESIAN to DIEFF_SPHERICAL conversion.

by Rémi Bernon

Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/dinput/joystick_hid.c | 10 ++++++++-- dlls/dinput8/tests/hid.c | 3 --- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/dlls/dinput/joystick_hid.c b/dlls/dinput/joystick_hid.c index a9716b20ab5..5a3c46e2d9a 100644 --- a/dlls/dinput/joystick_hid.c +++ b/dlls/dinput/joystick_hid.c @@ -2019,13 +2019,19 @@ static BOOL get_parameters_object_ofs( struct hid_joystick *impl, struct hid_val static void convert_directions_to_spherical( const DIEFFECT *in, DIEFFECT *out ) { - DWORD i, direction_flags = DIEFF_CARTESIAN | DIEFF_POLAR | DIEFF_SPHERICAL; + DWORD i, j, direction_flags = DIEFF_CARTESIAN | DIEFF_POLAR | DIEFF_SPHERICAL; + double tmp; switch (in->dwFlags & direction_flags) { case DIEFF_CARTESIAN: for (i = 1; i < in->cAxes; ++i) - out->rglDirection[i - 1] = atan2( in->rglDirection[i], in->rglDirection[0] ); + { + tmp = in->rglDirection[0]; + for (j = 1; j < i; ++j) tmp = sqrt( tmp * tmp + in->rglDirection[j] * in->rglDirection[j] ); + tmp = atan2( in->rglDirection[i], tmp ); + out->rglDirection[i - 1] = tmp * 18000 / M_PI; + } out->rglDirection[in->cAxes - 1] = 0; out->cAxes = in->cAxes; break; diff --git a/dlls/dinput8/tests/hid.c b/dlls/dinput8/tests/hid.c index 5f89f13146a..66e8c7c0b0d 100644 --- a/dlls/dinput8/tests/hid.c +++ b/dlls/dinput8/tests/hid.c @@ -6085,7 +6085,6 @@ static void test_periodic_effect( IDirectInputDevice8W *device, HANDLE file, DWO } else { - todo_wine ok( desc.rglDirection[0] == 6343, "got rglDirection[0] %d expected %d\n", desc.rglDirection[0], 6343 ); if (i == 2) @@ -6097,7 +6096,6 @@ static void test_periodic_effect( IDirectInputDevice8W *device, HANDLE file, DWO } else { - todo_wine ok( desc.rglDirection[1] == 5330, "got rglDirection[1] %d expected %d\n", desc.rglDirection[1], 5330 ); ok( desc.rglDirection[2] == 0, "got rglDirection[2] %d expected %d\n", @@ -6141,7 +6139,6 @@ static void test_periodic_effect( IDirectInputDevice8W *device, HANDLE file, DWO { ok( hr == DI_OK, "GetParameters returned %#x\n", hr ); ok( desc.cAxes == i, "got cAxes %u expected 2\n", desc.cAxes ); - todo_wine ok( desc.rglDirection[0] == 15343, "got rglDirection[0] %d expected %d\n", desc.rglDirection[0], 15343 ); ok( desc.rglDirection[1] == 0, "got rglDirection[1] %d expected %d\n", desc.rglDirection[1], 0 ); -- 2.33.1

Nov. 10, 2021

[PATCH 6/7] dinput: Only convert DIEFFECT directions to spherical when needed.

by Rémi Bernon

Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/dinput/joystick_hid.c | 182 +++++++++++++++++++++++++++---------- dlls/dinput8/tests/hid.c | 7 -- 2 files changed, 134 insertions(+), 55 deletions(-) diff --git a/dlls/dinput/joystick_hid.c b/dlls/dinput/joystick_hid.c index cec02ccd74a..a9716b20ab5 100644 --- a/dlls/dinput/joystick_hid.c +++ b/dlls/dinput/joystick_hid.c @@ -2017,6 +2017,127 @@ static BOOL get_parameters_object_ofs( struct hid_joystick *impl, struct hid_val return DIENUM_STOP; } +static void convert_directions_to_spherical( const DIEFFECT *in, DIEFFECT *out ) +{ + DWORD i, direction_flags = DIEFF_CARTESIAN | DIEFF_POLAR | DIEFF_SPHERICAL; + + switch (in->dwFlags & direction_flags) + { + case DIEFF_CARTESIAN: + for (i = 1; i < in->cAxes; ++i) + out->rglDirection[i - 1] = atan2( in->rglDirection[i], in->rglDirection[0] ); + out->rglDirection[in->cAxes - 1] = 0; + out->cAxes = in->cAxes; + break; + case DIEFF_POLAR: + out->rglDirection[0] = (in->rglDirection[0] % 36000) - 9000; + if (out->rglDirection[0] < 0) out->rglDirection[0] += 36000; + for (i = 1; i < in->cAxes; ++i) out->rglDirection[i] = 0; + out->cAxes = in->cAxes; + break; + case DIEFF_SPHERICAL: + for (i = 0; i < in->cAxes; ++i) + { + out->rglDirection[i] = in->rglDirection[i] % 36000; + if (out->rglDirection[i] < 0) out->rglDirection[i] += 36000; + } + out->cAxes = in->cAxes; + break; + } +} + +static void convert_directions_from_spherical( const DIEFFECT *in, DIEFFECT *out ) +{ + DWORD i, j, direction_flags = DIEFF_CARTESIAN | DIEFF_POLAR | DIEFF_SPHERICAL; + LONG tmp; + + switch (out->dwFlags & direction_flags) + { + case DIEFF_CARTESIAN: + out->rglDirection[0] = 10000; + for (i = 1; i <= in->cAxes; ++i) + { + tmp = cos( in->rglDirection[i - 1] * M_PI / 18000 ) * 10000; + for (j = 0; j < i; ++j) + out->rglDirection[j] = round( out->rglDirection[j] * tmp / 10000.0 ); + out->rglDirection[i] = sin( in->rglDirection[i - 1] * M_PI / 18000 ) * 10000; + } + out->cAxes = in->cAxes; + break; + case DIEFF_POLAR: + out->rglDirection[0] = (in->rglDirection[0] + 9000) % 36000; + if (out->rglDirection[0] < 0) out->rglDirection[0] += 36000; + out->rglDirection[1] = 0; + out->cAxes = 2; + break; + case DIEFF_SPHERICAL: + for (i = 0; i < in->cAxes; ++i) + { + out->rglDirection[i] = in->rglDirection[i] % 36000; + if (out->rglDirection[i] < 0) out->rglDirection[i] += 36000; + } + out->cAxes = in->cAxes; + break; + } +} + +static void convert_directions( const DIEFFECT *in, DIEFFECT *out ) +{ + DWORD direction_flags = DIEFF_CARTESIAN | DIEFF_POLAR | DIEFF_SPHERICAL; + LONG directions[6] = {0}; + DIEFFECT spherical = {.rglDirection = directions}; + + switch (in->dwFlags & direction_flags) + { + case DIEFF_CARTESIAN: + switch (out->dwFlags & direction_flags) + { + case DIEFF_CARTESIAN: + memcpy( out->rglDirection, in->rglDirection, in->cAxes * sizeof(LONG) ); + out->cAxes = in->cAxes; + break; + case DIEFF_POLAR: + convert_directions_to_spherical( in, &spherical ); + convert_directions_from_spherical( &spherical, out ); + break; + case DIEFF_SPHERICAL: + convert_directions_to_spherical( in, out ); + break; + } + break; + + case DIEFF_POLAR: + switch (out->dwFlags & direction_flags) + { + case DIEFF_POLAR: + memcpy( out->rglDirection, in->rglDirection, in->cAxes * sizeof(LONG) ); + out->cAxes = in->cAxes; + break; + case DIEFF_CARTESIAN: + convert_directions_to_spherical( in, &spherical ); + convert_directions_from_spherical( &spherical, out ); + break; + case DIEFF_SPHERICAL: + convert_directions_to_spherical( in, out ); + break; + } + break; + + case DIEFF_SPHERICAL: + switch (out->dwFlags & direction_flags) + { + case DIEFF_POLAR: + case DIEFF_CARTESIAN: + convert_directions_from_spherical( in, out ); + break; + case DIEFF_SPHERICAL: + convert_directions_to_spherical( in, out ); + break; + } + break; + } +} + static HRESULT WINAPI hid_joystick_effect_GetParameters( IDirectInputEffect *iface, DIEFFECT *params, DWORD flags ) { DIPROPHEADER filter = @@ -2026,8 +2147,7 @@ static HRESULT WINAPI hid_joystick_effect_GetParameters( IDirectInputEffect *ifa .dwHow = DIPH_BYUSAGE, }; struct hid_joystick_effect *impl = impl_from_IDirectInputEffect( iface ); - ULONG i, j, count, capacity, object_flags, direction_flags; - LONG tmp, directions[6] = {0}; + ULONG i, count, capacity, object_flags, direction_flags; BOOL ret; TRACE( "iface %p, params %p, flags %#x.\n", iface, params, flags ); @@ -2067,27 +2187,9 @@ static HRESULT WINAPI hid_joystick_effect_GetParameters( IDirectInputEffect *ifa if ((direction_flags & DIEFF_POLAR) && count != 2) return DIERR_INVALIDPARAM; if (capacity < params->cAxes) return DIERR_MOREDATA; - if (direction_flags & DIEFF_SPHERICAL) - memcpy( directions, impl->params.rglDirection, count * sizeof(LONG) ); - else if (direction_flags & DIEFF_POLAR) - { - directions[0] = (impl->params.rglDirection[0] + 9000) % 36000; - if (directions[0] < 0) directions[0] += 36000; - } - else if (direction_flags & DIEFF_CARTESIAN) - { - directions[0] = 10000; - for (i = 1; i <= count; ++i) - { - tmp = cos( impl->params.rglDirection[i - 1] * M_PI / 18000 ) * 10000; - for (j = 0; j < i; ++j) directions[j] = round( directions[j] * tmp / 10000.0 ); - directions[i] = sin( impl->params.rglDirection[i - 1] * M_PI / 18000 ) * 10000; - } - } - if (!count) params->rglDirection = NULL; else if (!params->rglDirection) return DIERR_INVALIDPARAM; - else memcpy( params->rglDirection, directions, count * sizeof(LONG) ); + else convert_directions( &impl->params, params ); } if (flags & DIEP_TYPESPECIFICPARAMS) @@ -2182,7 +2284,6 @@ static HRESULT WINAPI hid_joystick_effect_SetParameters( IDirectInputEffect *ifa }; struct hid_joystick_effect *impl = impl_from_IDirectInputEffect( iface ); ULONG i, count, old_value, object_flags, direction_flags; - LONG directions[6] = {0}; HRESULT hr; BOOL ret; @@ -2226,31 +2327,11 @@ static HRESULT WINAPI hid_joystick_effect_SetParameters( IDirectInputEffect *ifa if ((direction_flags & DIEFF_POLAR) && count != 2) return DIERR_INVALIDPARAM; if ((direction_flags & DIEFF_CARTESIAN) && params->cAxes != count) return DIERR_INVALIDPARAM; - if (!count) memset( directions, 0, sizeof(directions) ); - else if (direction_flags & DIEFF_POLAR) - { - directions[0] = (params->rglDirection[0] % 36000) - 9000; - if (directions[0] < 0) directions[0] += 36000; - for (i = 1; i < count; ++i) directions[i] = 0; - } - else if (direction_flags & DIEFF_CARTESIAN) - { - for (i = 1; i < count; ++i) - directions[i - 1] = atan2( params->rglDirection[i], params->rglDirection[0] ); - directions[count - 1] = 0; - } - else - { - for (i = 0; i < count; ++i) - { - directions[i] = params->rglDirection[i] % 36000; - if (directions[i] < 0) directions[i] += 36000; - } - } - - if (memcmp( impl->params.rglDirection, directions, count * sizeof(LONG) )) + impl->params.dwFlags &= ~(DIEFF_CARTESIAN | DIEFF_POLAR | DIEFF_SPHERICAL); + impl->params.dwFlags |= direction_flags; + if (memcmp( impl->params.rglDirection, params->rglDirection, count * sizeof(LONG) )) impl->modified = TRUE; - memcpy( impl->params.rglDirection, directions, count * sizeof(LONG) ); + memcpy( impl->params.rglDirection, params->rglDirection, count * sizeof(LONG) ); } if (flags & DIEP_TYPESPECIFICPARAMS) @@ -2514,7 +2595,9 @@ static HRESULT WINAPI hid_joystick_effect_Download( IDirectInputEffect *iface ) ULONG report_len = impl->joystick->caps.OutputReportByteLength; HANDLE device = impl->joystick->device; struct hid_value_caps *caps; + LONG directions[4] = {0}; DWORD i, tmp, count; + DIEFFECT spherical; NTSTATUS status; USAGE usage; HRESULT hr; @@ -2659,10 +2742,13 @@ static HRESULT WINAPI hid_joystick_effect_Download( IDirectInputEffect *iface ) impl->joystick->preparsed, impl->effect_update_buf, report_len ); if (status != HIDP_STATUS_SUCCESS) WARN( "HidP_SetUsages returned %#x\n", status ); + spherical.rglDirection = directions; + convert_directions_to_spherical( &impl->params, &spherical ); + if (!effect_update->direction_count) WARN( "no PID effect direction caps found\n" ); - else for (i = 0; i < impl->params.cAxes - 1; ++i) + else for (i = 0; i < spherical.cAxes - 1; ++i) { - tmp = impl->directions[i] + (i == 0 ? 9000 : 0); + tmp = directions[i] + (i == 0 ? 9000 : 0); caps = effect_update->direction_caps[effect_update->direction_count - i - 1]; set_parameter_value( impl, impl->effect_update_buf, caps, tmp % 36000 ); } diff --git a/dlls/dinput8/tests/hid.c b/dlls/dinput8/tests/hid.c index 5b01ea3a68a..5f89f13146a 100644 --- a/dlls/dinput8/tests/hid.c +++ b/dlls/dinput8/tests/hid.c @@ -6114,26 +6114,19 @@ static void test_periodic_effect( IDirectInputDevice8W *device, HANDLE file, DWO hr = IDirectInputEffect_GetParameters( effect, &desc, DIEP_DIRECTION ); ok( hr == DI_OK, "GetParameters returned %#x\n", hr ); ok( desc.cAxes == i, "got cAxes %u expected 2\n", desc.cAxes ); - todo_wine ok( desc.rglDirection[0] == 1000, "got rglDirection[0] %d expected %d\n", desc.rglDirection[0], 1000 ); if (i == 1) ok( desc.rglDirection[1] == 0xcdcdcdcd, "got rglDirection[1] %d expected %d\n", desc.rglDirection[1], 0xcdcdcdcd ); else - { - todo_wine ok( desc.rglDirection[1] == 2000, "got rglDirection[1] %d expected %d\n", desc.rglDirection[1], 2000 ); - } if (i <= 2) ok( desc.rglDirection[2] == 0xcdcdcdcd, "got rglDirection[2] %d expected %d\n", desc.rglDirection[2], 0xcdcdcdcd ); else - { - todo_wine ok( desc.rglDirection[2] == 3000, "got rglDirection[2] %d expected %d\n", desc.rglDirection[2], 3000 ); - } desc.dwFlags = DIEFF_POLAR; desc.cAxes = 1; -- 2.33.1

Nov. 10, 2021

[PATCH 5/7] dinput: Check for DIEFF_POLAR axes count before capacity.

by Rémi Bernon

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=51922 Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/dinput/joystick_hid.c | 4 ++-- dlls/dinput8/tests/hid.c | 6 +----- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/dlls/dinput/joystick_hid.c b/dlls/dinput/joystick_hid.c index dd77f557e3e..cec02ccd74a 100644 --- a/dlls/dinput/joystick_hid.c +++ b/dlls/dinput/joystick_hid.c @@ -2063,14 +2063,14 @@ static HRESULT WINAPI hid_joystick_effect_GetParameters( IDirectInputEffect *ifa if (!direction_flags) return DIERR_INVALIDPARAM; count = params->cAxes = impl->params.cAxes; - if (capacity < params->cAxes) return DIERR_MOREDATA; if (!count) params->dwFlags &= ~(DIEFF_CARTESIAN | DIEFF_POLAR | DIEFF_SPHERICAL); + if ((direction_flags & DIEFF_POLAR) && count != 2) return DIERR_INVALIDPARAM; + if (capacity < params->cAxes) return DIERR_MOREDATA; if (direction_flags & DIEFF_SPHERICAL) memcpy( directions, impl->params.rglDirection, count * sizeof(LONG) ); else if (direction_flags & DIEFF_POLAR) { - if (count != 2) return DIERR_INVALIDPARAM; directions[0] = (impl->params.rglDirection[0] + 9000) % 36000; if (directions[0] < 0) directions[0] += 36000; } diff --git a/dlls/dinput8/tests/hid.c b/dlls/dinput8/tests/hid.c index 54109593a18..5b01ea3a68a 100644 --- a/dlls/dinput8/tests/hid.c +++ b/dlls/dinput8/tests/hid.c @@ -6138,11 +6138,7 @@ static void test_periodic_effect( IDirectInputDevice8W *device, HANDLE file, DWO desc.dwFlags = DIEFF_POLAR; desc.cAxes = 1; hr = IDirectInputEffect_GetParameters( effect, &desc, DIEP_DIRECTION ); - if (i != 2) - { - todo_wine_if( i == 3 ) - ok( hr == DIERR_INVALIDPARAM, "GetParameters returned %#x\n", hr ); - } + if (i != 2) ok( hr == DIERR_INVALIDPARAM, "GetParameters returned %#x\n", hr ); else ok( hr == DIERR_MOREDATA, "GetParameters returned %#x\n", hr ); desc.cAxes = 3; memset( desc.rglDirection, 0xcd, 3 * sizeof(LONG) ); -- 2.33.1

Nov. 10, 2021

[PATCH 4/7] dinput: Allow DIEFF_CARTESIAN directions when cAxes matches.

by Rémi Bernon

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=51922 Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/dinput/joystick_hid.c | 2 +- dlls/dinput8/tests/hid.c | 2 -- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/dlls/dinput/joystick_hid.c b/dlls/dinput/joystick_hid.c index dfaf6ae83ff..dd77f557e3e 100644 --- a/dlls/dinput/joystick_hid.c +++ b/dlls/dinput/joystick_hid.c @@ -2224,7 +2224,7 @@ static HRESULT WINAPI hid_joystick_effect_SetParameters( IDirectInputEffect *ifa count = impl->params.cAxes; if (params->cAxes < count) return DIERR_INVALIDPARAM; if ((direction_flags & DIEFF_POLAR) && count != 2) return DIERR_INVALIDPARAM; - if ((direction_flags & DIEFF_CARTESIAN) && count < 2) return DIERR_INVALIDPARAM; + if ((direction_flags & DIEFF_CARTESIAN) && params->cAxes != count) return DIERR_INVALIDPARAM; if (!count) memset( directions, 0, sizeof(directions) ); else if (direction_flags & DIEFF_POLAR) diff --git a/dlls/dinput8/tests/hid.c b/dlls/dinput8/tests/hid.c index 4c1d73112cd..54109593a18 100644 --- a/dlls/dinput8/tests/hid.c +++ b/dlls/dinput8/tests/hid.c @@ -6061,11 +6061,9 @@ static void test_periodic_effect( IDirectInputDevice8W *device, HANDLE file, DWO desc.rglDirection[1] = 2000; desc.rglDirection[2] = 3000; hr = IDirectInputEffect_SetParameters( effect, &desc, DIEP_DIRECTION | DIEP_NODOWNLOAD ); - todo_wine_if( i == 2 ) ok( hr == DIERR_INVALIDPARAM, "SetParameters returned %#x\n", hr ); desc.cAxes = i; hr = IDirectInputEffect_SetParameters( effect, &desc, DIEP_DIRECTION | DIEP_NODOWNLOAD ); - todo_wine_if( i == 1 ) ok( hr == DI_DOWNLOADSKIPPED, "SetParameters returned %#x\n", hr ); desc.dwFlags = DIEFF_SPHERICAL; -- 2.33.1

Nov. 10, 2021

[PATCH 3/7] dinput8/tests: Add more force-feedback effect direction tests.

by Rémi Bernon

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=51922 Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/dinput8/tests/hid.c | 131 ++++++++++++++++++++++++++++++++++++++- 1 file changed, 130 insertions(+), 1 deletion(-) diff --git a/dlls/dinput8/tests/hid.c b/dlls/dinput8/tests/hid.c index 59111db7f3a..4c1d73112cd 100644 --- a/dlls/dinput8/tests/hid.c +++ b/dlls/dinput8/tests/hid.c @@ -5476,7 +5476,7 @@ static void test_periodic_effect( IDirectInputDevice8W *device, HANDLE file, DWO LONG directions[4] = {0}; DIEFFECT desc = {0}; DWORD axes[4] = {0}; - ULONG ref, flags; + ULONG i, ref, flags; HRESULT hr; GUID guid; @@ -6037,6 +6037,135 @@ static void test_periodic_effect( IDirectInputDevice8W *device, HANDLE file, DWO ref = IDirectInputEffect_Release( effect ); ok( ref == 0, "Release returned %d\n", ref ); + + for (i = 1; i < 4; i++) + { + winetest_push_context( "%u axes", i ); + hr = IDirectInputDevice8_CreateEffect( device, &GUID_Sine, NULL, &effect, NULL ); + ok( hr == DI_OK, "CreateEffect returned %#x\n", hr ); + + desc.dwFlags = DIEFF_OBJECTIDS; + desc.cAxes = i; + desc.rgdwAxes[0] = DIDFT_ABSAXIS | DIDFT_MAKEINSTANCE( 2 ) | DIDFT_FFACTUATOR; + desc.rgdwAxes[1] = DIDFT_ABSAXIS | DIDFT_MAKEINSTANCE( 0 ) | DIDFT_FFACTUATOR; + desc.rgdwAxes[2] = DIDFT_ABSAXIS | DIDFT_MAKEINSTANCE( 1 ) | DIDFT_FFACTUATOR; + desc.rglDirection[0] = 0; + desc.rglDirection[1] = 0; + desc.rglDirection[2] = 0; + hr = IDirectInputEffect_SetParameters( effect, &desc, DIEP_AXES | DIEP_NODOWNLOAD ); + ok( hr == DI_DOWNLOADSKIPPED, "SetParameters returned %#x\n", hr ); + + desc.dwFlags = DIEFF_CARTESIAN; + desc.cAxes = i == 3 ? 2 : 3; + desc.rglDirection[0] = 1000; + desc.rglDirection[1] = 2000; + desc.rglDirection[2] = 3000; + hr = IDirectInputEffect_SetParameters( effect, &desc, DIEP_DIRECTION | DIEP_NODOWNLOAD ); + todo_wine_if( i == 2 ) + ok( hr == DIERR_INVALIDPARAM, "SetParameters returned %#x\n", hr ); + desc.cAxes = i; + hr = IDirectInputEffect_SetParameters( effect, &desc, DIEP_DIRECTION | DIEP_NODOWNLOAD ); + todo_wine_if( i == 1 ) + ok( hr == DI_DOWNLOADSKIPPED, "SetParameters returned %#x\n", hr ); + + desc.dwFlags = DIEFF_SPHERICAL; + desc.cAxes = i; + hr = IDirectInputEffect_GetParameters( effect, &desc, DIEP_DIRECTION ); + ok( hr == DI_OK, "GetParameters returned %#x\n", hr ); + desc.cAxes = 3; + memset( desc.rglDirection, 0xcd, 3 * sizeof(LONG) ); + hr = IDirectInputEffect_GetParameters( effect, &desc, DIEP_DIRECTION ); + ok( hr == DI_OK, "GetParameters returned %#x\n", hr ); + ok( desc.cAxes == i, "got cAxes %u expected 2\n", desc.cAxes ); + if (i == 1) + { + ok( desc.rglDirection[0] == 0, "got rglDirection[0] %d expected %d\n", desc.rglDirection[0], 0 ); + ok( desc.rglDirection[1] == 0xcdcdcdcd, "got rglDirection[1] %d expected %d\n", + desc.rglDirection[1], 0xcdcdcdcd ); + ok( desc.rglDirection[2] == 0xcdcdcdcd, "got rglDirection[2] %d expected %d\n", + desc.rglDirection[2], 0xcdcdcdcd ); + } + else + { + todo_wine + ok( desc.rglDirection[0] == 6343, "got rglDirection[0] %d expected %d\n", + desc.rglDirection[0], 6343 ); + if (i == 2) + { + ok( desc.rglDirection[1] == 0, "got rglDirection[1] %d expected %d\n", + desc.rglDirection[1], 0 ); + ok( desc.rglDirection[2] == 0xcdcdcdcd, "got rglDirection[2] %d expected %d\n", + desc.rglDirection[2], 0xcdcdcdcd ); + } + else + { + todo_wine + ok( desc.rglDirection[1] == 5330, "got rglDirection[1] %d expected %d\n", + desc.rglDirection[1], 5330 ); + ok( desc.rglDirection[2] == 0, "got rglDirection[2] %d expected %d\n", + desc.rglDirection[2], 0 ); + } + } + + desc.dwFlags = DIEFF_CARTESIAN; + desc.cAxes = i; + hr = IDirectInputEffect_GetParameters( effect, &desc, DIEP_DIRECTION ); + ok( hr == DI_OK, "GetParameters returned %#x\n", hr ); + desc.cAxes = 3; + memset( desc.rglDirection, 0xcd, 3 * sizeof(LONG) ); + hr = IDirectInputEffect_GetParameters( effect, &desc, DIEP_DIRECTION ); + ok( hr == DI_OK, "GetParameters returned %#x\n", hr ); + ok( desc.cAxes == i, "got cAxes %u expected 2\n", desc.cAxes ); + todo_wine + ok( desc.rglDirection[0] == 1000, "got rglDirection[0] %d expected %d\n", desc.rglDirection[0], 1000 ); + if (i == 1) + ok( desc.rglDirection[1] == 0xcdcdcdcd, "got rglDirection[1] %d expected %d\n", + desc.rglDirection[1], 0xcdcdcdcd ); + else + { + todo_wine + ok( desc.rglDirection[1] == 2000, "got rglDirection[1] %d expected %d\n", + desc.rglDirection[1], 2000 ); + } + if (i <= 2) + ok( desc.rglDirection[2] == 0xcdcdcdcd, "got rglDirection[2] %d expected %d\n", + desc.rglDirection[2], 0xcdcdcdcd ); + else + { + todo_wine + ok( desc.rglDirection[2] == 3000, "got rglDirection[2] %d expected %d\n", + desc.rglDirection[2], 3000 ); + } + + desc.dwFlags = DIEFF_POLAR; + desc.cAxes = 1; + hr = IDirectInputEffect_GetParameters( effect, &desc, DIEP_DIRECTION ); + if (i != 2) + { + todo_wine_if( i == 3 ) + ok( hr == DIERR_INVALIDPARAM, "GetParameters returned %#x\n", hr ); + } + else ok( hr == DIERR_MOREDATA, "GetParameters returned %#x\n", hr ); + desc.cAxes = 3; + memset( desc.rglDirection, 0xcd, 3 * sizeof(LONG) ); + hr = IDirectInputEffect_GetParameters( effect, &desc, DIEP_DIRECTION ); + if (i != 2) ok( hr == DIERR_INVALIDPARAM, "GetParameters returned %#x\n", hr ); + else + { + ok( hr == DI_OK, "GetParameters returned %#x\n", hr ); + ok( desc.cAxes == i, "got cAxes %u expected 2\n", desc.cAxes ); + todo_wine + ok( desc.rglDirection[0] == 15343, "got rglDirection[0] %d expected %d\n", + desc.rglDirection[0], 15343 ); + ok( desc.rglDirection[1] == 0, "got rglDirection[1] %d expected %d\n", desc.rglDirection[1], 0 ); + ok( desc.rglDirection[2] == 0xcdcdcdcd, "got rglDirection[2] %d expected %d\n", + desc.rglDirection[2], 0xcdcdcdcd ); + } + + ref = IDirectInputEffect_Release( effect ); + ok( ref == 0, "Release returned %d\n", ref ); + winetest_pop_context(); + } } static void test_condition_effect( IDirectInputDevice8W *device, HANDLE file, DWORD version ) -- 2.33.1

Nov. 10, 2021