[PATCH 0/5] MR4085: wined3d: Old-GL fixes
From: Matteo Bruni <mbruni(a)codeweavers.com> Fixes: c065b4fe0b3622e0c737a4c7c1c514273ad2d8a7 --- dlls/wined3d/context_gl.c | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/dlls/wined3d/context_gl.c b/dlls/wined3d/context_gl.c index a55365cdf12..b81f14a86ee 100644 --- a/dlls/wined3d/context_gl.c +++ b/dlls/wined3d/context_gl.c @@ -5399,7 +5399,8 @@ void wined3d_context_gl_load_tex_coords(const struct wined3d_context_gl *context gl_info->gl_ops.gl.p_glTexCoordPointer(format_gl->vtx_format, format_gl->vtx_type, e->stride, get_vertex_attrib_pointer(e, state)); gl_info->gl_ops.gl.p_glEnableClientState(GL_TEXTURE_COORD_ARRAY); - wined3d_buffer_validate_user(state->streams[e->stream_idx].buffer); + if (bo) + wined3d_buffer_validate_user(state->streams[e->stream_idx].buffer); } else { @@ -5486,7 +5487,8 @@ static void wined3d_context_gl_load_vertex_data(struct wined3d_context_gl *conte checkGLcall("glVertexPointer(...)"); gl_info->gl_ops.gl.p_glEnableClientState(GL_VERTEX_ARRAY); checkGLcall("glEnableClientState(GL_VERTEX_ARRAY)"); - wined3d_buffer_validate_user(state->streams[e->stream_idx].buffer); + if (bo) + wined3d_buffer_validate_user(state->streams[e->stream_idx].buffer); } /* Normals */ @@ -5509,7 +5511,8 @@ static void wined3d_context_gl_load_vertex_data(struct wined3d_context_gl *conte checkGLcall("glNormalPointer(...)"); gl_info->gl_ops.gl.p_glEnableClientState(GL_NORMAL_ARRAY); checkGLcall("glEnableClientState(GL_NORMAL_ARRAY)"); - wined3d_buffer_validate_user(state->streams[e->stream_idx].buffer); + if (bo) + wined3d_buffer_validate_user(state->streams[e->stream_idx].buffer); } else { @@ -5538,7 +5541,8 @@ static void wined3d_context_gl_load_vertex_data(struct wined3d_context_gl *conte checkGLcall("glColorPointer(4, GL_UNSIGNED_BYTE, ...)"); gl_info->gl_ops.gl.p_glEnableClientState(GL_COLOR_ARRAY); checkGLcall("glEnableClientState(GL_COLOR_ARRAY)"); - wined3d_buffer_validate_user(state->streams[e->stream_idx].buffer); + if (bo) + wined3d_buffer_validate_user(state->streams[e->stream_idx].buffer); } else { @@ -5602,7 +5606,8 @@ static void wined3d_context_gl_load_vertex_data(struct wined3d_context_gl *conte } gl_info->gl_ops.gl.p_glEnableClientState(GL_SECONDARY_COLOR_ARRAY_EXT); checkGLcall("glEnableClientState(GL_SECONDARY_COLOR_ARRAY_EXT)"); - wined3d_buffer_validate_user(state->streams[e->stream_idx].buffer); + if (bo) + wined3d_buffer_validate_user(state->streams[e->stream_idx].buffer); } else { @@ -5703,8 +5708,6 @@ static void wined3d_context_gl_load_numbered_arrays(struct wined3d_context_gl *c format_gl = wined3d_format_gl(element->format); stream = &state->streams[element->stream_idx]; - wined3d_buffer_validate_user(stream->buffer); - if (gl_info->supported[ARB_INSTANCED_ARRAYS]) { @@ -5738,6 +5741,9 @@ static void wined3d_context_gl_load_numbered_arrays(struct wined3d_context_gl *c checkGLcall("glBindBuffer"); current_bo = bo; } + if (bo) + wined3d_buffer_validate_user(stream->buffer); + /* Use the VBO to find out if a vertex buffer exists, not the vb * pointer. vb can point to a user pointer data blob. In that case * current_bo will be 0. If there is a vertex buffer but no vbo we -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4085
From: Matteo Bruni <mbruni(a)codeweavers.com> Fixes: c065b4fe0b3622e0c737a4c7c1c514273ad2d8a7 --- dlls/wined3d/arb_program_shader.c | 12 ++++++++++++ dlls/wined3d/cs.c | 7 +++++-- dlls/wined3d/wined3d_private.h | 1 + 3 files changed, 18 insertions(+), 2 deletions(-) diff --git a/dlls/wined3d/arb_program_shader.c b/dlls/wined3d/arb_program_shader.c index 272a25902d9..59c4c6f2350 100644 --- a/dlls/wined3d/arb_program_shader.c +++ b/dlls/wined3d/arb_program_shader.c @@ -678,6 +678,8 @@ static void shader_arb_load_constants_internal(struct shader_arb_priv *priv, str const struct arb_vs_compiled_shader *gl_shader = priv->compiled_vprog; /* Load DirectX 9 float constants for vertex shader */ + if (!wined3d_cs_prepare_push_constant_buffer(context_gl->c.device, WINED3D_PUSH_CONSTANTS_VS_F)) + return; priv->highest_dirty_vs_const = shader_arb_load_constants_f(vshader, gl_info, GL_VERTEX_PROGRAM_ARB, priv->highest_dirty_vs_const, wined3d_buffer_load_sysmem(device->push_constants[WINED3D_PUSH_CONSTANTS_VS_F], &context_gl->c), @@ -692,6 +694,8 @@ static void shader_arb_load_constants_internal(struct shader_arb_priv *priv, str UINT rt_height = state->fb.render_targets[0]->height; /* Load DirectX 9 float constants for pixel shader */ + if (!wined3d_cs_prepare_push_constant_buffer(context_gl->c.device, WINED3D_PUSH_CONSTANTS_PS_F)) + return; priv->highest_dirty_ps_const = shader_arb_load_constants_f(pshader, gl_info, GL_FRAGMENT_PROGRAM_ARB, priv->highest_dirty_ps_const, wined3d_buffer_load_sysmem(device->push_constants[WINED3D_PUSH_CONSTANTS_PS_F], &context_gl->c), @@ -4433,7 +4437,11 @@ static void find_arb_ps_compile_args(const struct wined3d_state *state, int i; WORD int_skip; + if (!wined3d_cs_prepare_push_constant_buffer(context_gl->c.device, WINED3D_PUSH_CONSTANTS_PS_B)) + return; bool_consts = wined3d_buffer_load_sysmem(device->push_constants[WINED3D_PUSH_CONSTANTS_PS_B], &context_gl->c); + if (!wined3d_cs_prepare_push_constant_buffer(context_gl->c.device, WINED3D_PUSH_CONSTANTS_PS_I)) + return; int_consts = wined3d_buffer_load_sysmem(device->push_constants[WINED3D_PUSH_CONSTANTS_PS_I], &context_gl->c); find_ps_compile_args(state, shader, context_gl->c.stream_info.position_transformed, &args->super, &context_gl->c); @@ -4496,7 +4504,11 @@ static void find_arb_vs_compile_args(const struct wined3d_state *state, int i; WORD int_skip; + if (!wined3d_cs_prepare_push_constant_buffer(context_gl->c.device, WINED3D_PUSH_CONSTANTS_VS_B)) + return; bool_consts = wined3d_buffer_load_sysmem(device->push_constants[WINED3D_PUSH_CONSTANTS_VS_B], &context_gl->c); + if (!wined3d_cs_prepare_push_constant_buffer(context_gl->c.device, WINED3D_PUSH_CONSTANTS_VS_I)) + return; int_consts = wined3d_buffer_load_sysmem(device->push_constants[WINED3D_PUSH_CONSTANTS_VS_I], &context_gl->c); find_vs_compile_args(state, shader, &args->super, &context_gl->c); diff --git a/dlls/wined3d/cs.c b/dlls/wined3d/cs.c index aa011ea7714..ad923e3b827 100644 --- a/dlls/wined3d/cs.c +++ b/dlls/wined3d/cs.c @@ -2129,7 +2129,7 @@ wined3d_cs_push_constant_info[] = [WINED3D_PUSH_CONSTANTS_PS_B] = {sizeof(BOOL), WINED3D_MAX_CONSTS_B, WINED3D_SHADER_CONST_PS_B}, }; -static bool prepare_push_constant_buffer(struct wined3d_device *device, enum wined3d_push_constants type) +bool wined3d_cs_prepare_push_constant_buffer(struct wined3d_device *device, enum wined3d_push_constants type) { const struct push_constant_info *info = &wined3d_cs_push_constant_info[type]; HRESULT hr; @@ -2139,8 +2139,11 @@ static bool prepare_push_constant_buffer(struct wined3d_device *device, enum win .byte_width = info->max_count * info->size, .bind_flags = 0, .access = WINED3D_RESOURCE_ACCESS_CPU | WINED3D_RESOURCE_ACCESS_MAP_R | WINED3D_RESOURCE_ACCESS_MAP_W, + .usage = WINED3DUSAGE_PRIVATE, }; + TRACE("device %p, type %#x.\n", device, type); + if (!device->push_constants[type] && FAILED(hr = wined3d_buffer_create(device, &desc, NULL, NULL, &wined3d_null_parent_ops, &device->push_constants[type]))) { @@ -2193,7 +2196,7 @@ void wined3d_device_context_push_constants(struct wined3d_device_context *contex unsigned int byte_size = count * info->size; struct wined3d_box box; - if (!prepare_push_constant_buffer(context->device, type)) + if (!wined3d_cs_prepare_push_constant_buffer(context->device, type)) return; wined3d_box_set(&box, byte_offset, 0, byte_offset + byte_size, 1, 0, 1); diff --git a/dlls/wined3d/wined3d_private.h b/dlls/wined3d/wined3d_private.h index 0b72bf298e2..e6253a41b39 100644 --- a/dlls/wined3d/wined3d_private.h +++ b/dlls/wined3d/wined3d_private.h @@ -3656,6 +3656,7 @@ void wined3d_cs_init_object(struct wined3d_cs *cs, void (*callback)(void *object), void *object) DECLSPEC_HIDDEN; void wined3d_cs_map_bo_address(struct wined3d_cs *cs, struct wined3d_bo_address *addr, size_t size, unsigned int flags) DECLSPEC_HIDDEN; +bool wined3d_cs_prepare_push_constant_buffer(struct wined3d_device *device, enum wined3d_push_constants type); void wined3d_device_context_set_depth_bounds(struct wined3d_device_context *context, bool enable, float min_depth, float max_depth); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4085
From: Matteo Bruni <mbruni(a)codeweavers.com> In the same vein as 6247bee483b31b1ea0ad49f85f1cffde7433fed2; avoids infinite recursion in the blitter. Triggered by the ddraw tests. --- dlls/wined3d/texture.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/dlls/wined3d/texture.c b/dlls/wined3d/texture.c index 08e95db10f5..a7fbde0f529 100644 --- a/dlls/wined3d/texture.c +++ b/dlls/wined3d/texture.c @@ -5832,8 +5832,17 @@ static bool ffp_blit_supported(enum wined3d_blit_op blit_op, const struct wined3 if (!(dst_resource->bind_flags & WINED3D_BIND_RENDER_TARGET)) { - TRACE("Can only blit to render targets.\n"); - return false; + if (dst_format->id == src_format->id && dst_location == WINED3D_LOCATION_DRAWABLE) + { + if (context->device->shader_backend == &none_shader_backend) + WARN("Claiming !render_target support because of no shader backend.\n"); + return true; + } + else + { + TRACE("Can only blit to render targets.\n"); + return false; + } } return true; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4085
From: Matteo Bruni <mbruni(a)codeweavers.com> --- I swear this patch wrote itself... --- dlls/wined3d/adapter_gl.c | 1 + dlls/wined3d/arb_program_shader.c | 34 ++++++++++++++++++++++++++----- dlls/wined3d/glsl_shader.c | 15 ++------------ dlls/wined3d/wined3d_gl.h | 1 + dlls/wined3d/wined3d_private.h | 11 ++++++++++ 5 files changed, 44 insertions(+), 18 deletions(-) diff --git a/dlls/wined3d/adapter_gl.c b/dlls/wined3d/adapter_gl.c index c3925b7c3d0..bd7bd3a1222 100644 --- a/dlls/wined3d/adapter_gl.c +++ b/dlls/wined3d/adapter_gl.c @@ -84,6 +84,7 @@ static const struct wined3d_extension_map gl_extension_map[] = {"GL_ARB_fragment_coord_conventions", ARB_FRAGMENT_COORD_CONVENTIONS}, {"GL_ARB_fragment_layer_viewport", ARB_FRAGMENT_LAYER_VIEWPORT }, {"GL_ARB_fragment_program", ARB_FRAGMENT_PROGRAM }, + {"GL_ARB_fragment_program_shadow", ARB_FRAGMENT_PROGRAM_SHADOW }, {"GL_ARB_fragment_shader", ARB_FRAGMENT_SHADER }, {"GL_ARB_framebuffer_no_attachments", ARB_FRAMEBUFFER_NO_ATTACHMENTS}, {"GL_ARB_framebuffer_object", ARB_FRAMEBUFFER_OBJECT }, diff --git a/dlls/wined3d/arb_program_shader.c b/dlls/wined3d/arb_program_shader.c index 59c4c6f2350..b61d65346de 100644 --- a/dlls/wined3d/arb_program_shader.c +++ b/dlls/wined3d/arb_program_shader.c @@ -1413,6 +1413,7 @@ static void shader_hw_sample(const struct wined3d_shader_instruction *ins, unsig enum wined3d_shader_resource_type resource_type; struct color_fixup_masks masks; const char *tex_dst = dst_str; + bool shadow_sampler, tex_rect; BOOL np2_fixup = FALSE; const char *tex_type; const char *mod; @@ -1430,18 +1431,35 @@ static void shader_hw_sample(const struct wined3d_shader_instruction *ins, unsig sampler_idx += WINED3D_MAX_FRAGMENT_SAMPLERS; } + shadow_sampler = priv->gl_info->supported[ARB_FRAGMENT_PROGRAM_SHADOW] + && shader_sampler_is_shadow(ins->ctx->shader, &priv->cur_ps_args->super, sampler_idx, sampler_idx); + switch (resource_type) { case WINED3D_SHADER_RESOURCE_TEXTURE_1D: - tex_type = "1D"; + if (shadow_sampler) + tex_type = "SHADOW1D"; + else + tex_type = "1D"; break; case WINED3D_SHADER_RESOURCE_TEXTURE_2D: - if (pshader && priv->cur_ps_args->super.np2_fixup & (1u << sampler_idx) - && priv->gl_info->supported[ARB_TEXTURE_RECTANGLE]) - tex_type = "RECT"; + tex_rect = pshader && priv->cur_ps_args->super.np2_fixup & (1u << sampler_idx) + && priv->gl_info->supported[ARB_TEXTURE_RECTANGLE]; + if (shadow_sampler) + { + if (tex_rect) + tex_type = "SHADOWRECT"; + else + tex_type = "SHADOW2D"; + } else - tex_type = "2D"; + { + if (tex_rect) + tex_type = "RECT"; + else + tex_type = "2D"; + } if (pshader) { @@ -1454,10 +1472,14 @@ static void shader_hw_sample(const struct wined3d_shader_instruction *ins, unsig break; case WINED3D_SHADER_RESOURCE_TEXTURE_3D: + if (shadow_sampler) + FIXME("Unsupported 3D shadow sampler.\n"); tex_type = "3D"; break; case WINED3D_SHADER_RESOURCE_TEXTURE_CUBE: + if (shadow_sampler) + FIXME("Unsupported cube shadow sampler.\n"); tex_type = "CUBE"; break; @@ -3643,6 +3665,8 @@ static GLuint shader_arb_generate_pshader(const struct wined3d_shader *shader, break; } } + if (gl_info->supported[ARB_FRAGMENT_PROGRAM_SHADOW]) + shader_addline(buffer, "OPTION ARB_fragment_program_shadow;\n"); /* For now always declare the temps. At least the Nvidia assembler optimizes completely * unused temps away(but occupies them for the whole shader if they're used once). Always diff --git a/dlls/wined3d/glsl_shader.c b/dlls/wined3d/glsl_shader.c index e381e8df7a4..9ab3f722d31 100644 --- a/dlls/wined3d/glsl_shader.c +++ b/dlls/wined3d/glsl_shader.c @@ -2110,17 +2110,6 @@ static BOOL glsl_is_color_reg_read(const struct wined3d_shader *shader, unsigned return FALSE; } -static BOOL glsl_is_shadow_sampler(const struct wined3d_shader *shader, - const struct ps_compile_args *ps_args, unsigned int resource_idx, unsigned int sampler_idx) -{ - const struct wined3d_shader_version *version = &shader->reg_maps.shader_version; - - if (version->major >= 4) - return shader->reg_maps.sampler_comparison_mode & (1u << sampler_idx); - else - return version->type == WINED3D_SHADER_TYPE_PIXEL && (ps_args->shadow & (1u << resource_idx)); -} - static void shader_glsl_declare_typed_vertex_attribute(struct wined3d_string_buffer *buffer, const struct wined3d_gl_info *gl_info, const char *vector_type, const char *scalar_type, unsigned int index) @@ -2345,7 +2334,7 @@ static void shader_generate_glsl_declarations(const struct wined3d_context_gl *c break; } - shadow_sampler = glsl_is_shadow_sampler(shader, ps_args, entry->resource_idx, entry->sampler_idx); + shadow_sampler = shader_sampler_is_shadow(shader, ps_args, entry->resource_idx, entry->sampler_idx); resource_type = version->type == WINED3D_SHADER_TYPE_PIXEL ? pixelshader_get_resource_type(reg_maps, entry->resource_idx, ps_args->tex_types) : reg_maps->resource_info[entry->resource_idx].type; @@ -3424,7 +3413,7 @@ static void shader_glsl_get_sample_function(const struct wined3d_shader_context enum wined3d_shader_resource_type resource_type; struct shader_glsl_ctx_priv *priv = ctx->backend_data; const struct wined3d_gl_info *gl_info = priv->gl_info; - BOOL shadow = glsl_is_shadow_sampler(ctx->shader, priv->cur_ps_args, resource_idx, sampler_idx); + BOOL shadow = shader_sampler_is_shadow(ctx->shader, priv->cur_ps_args, resource_idx, sampler_idx); BOOL projected = flags & WINED3D_GLSL_SAMPLE_PROJECTED; BOOL texrect = ctx->reg_maps->shader_version.type == WINED3D_SHADER_TYPE_PIXEL && priv->cur_ps_args->np2_fixup & (1u << resource_idx) diff --git a/dlls/wined3d/wined3d_gl.h b/dlls/wined3d/wined3d_gl.h index ceb534a10a4..f5cec2ccdd7 100644 --- a/dlls/wined3d/wined3d_gl.h +++ b/dlls/wined3d/wined3d_gl.h @@ -78,6 +78,7 @@ enum wined3d_gl_extension ARB_FRAGMENT_COORD_CONVENTIONS, ARB_FRAGMENT_LAYER_VIEWPORT, ARB_FRAGMENT_PROGRAM, + ARB_FRAGMENT_PROGRAM_SHADOW, ARB_FRAGMENT_SHADER, ARB_FRAMEBUFFER_NO_ATTACHMENTS, ARB_FRAMEBUFFER_OBJECT, diff --git a/dlls/wined3d/wined3d_private.h b/dlls/wined3d/wined3d_private.h index e6253a41b39..4fb53be659e 100644 --- a/dlls/wined3d/wined3d_private.h +++ b/dlls/wined3d/wined3d_private.h @@ -4382,6 +4382,17 @@ static inline BOOL shader_constant_is_local(const struct wined3d_shader *shader, return FALSE; } +static inline BOOL shader_sampler_is_shadow(const struct wined3d_shader *shader, + const struct ps_compile_args *ps_args, unsigned int resource_idx, unsigned int sampler_idx) +{ + const struct wined3d_shader_version *version = &shader->reg_maps.shader_version; + + if (version->major >= 4) + return shader->reg_maps.sampler_comparison_mode & (1u << sampler_idx); + else + return version->type == WINED3D_SHADER_TYPE_PIXEL && (ps_args->shadow & (1u << resource_idx)); +} + void get_identity_matrix(struct wined3d_matrix *mat) DECLSPEC_HIDDEN; void get_modelview_matrix(const struct wined3d_context *context, const struct wined3d_state *state, unsigned int index, struct wined3d_matrix *mat) DECLSPEC_HIDDEN; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4085
From: Matteo Bruni <mbruni(a)codeweavers.com> More fallout from 2ddb6b66a7cda0bf6aaddc0c6899e35cc92ceee9. --- dlls/wined3d/state.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dlls/wined3d/state.c b/dlls/wined3d/state.c index 6eaec9acb55..30b0adaa135 100644 --- a/dlls/wined3d/state.c +++ b/dlls/wined3d/state.c @@ -3680,7 +3680,7 @@ void clipplane(struct wined3d_context *context, const struct wined3d_state *stat UINT index = state_id - STATE_CLIPPLANE(0); GLdouble plane[4]; - if (isStateDirty(context, STATE_TRANSFORM(WINED3D_TS_VIEW)) || index >= gl_info->limits.user_clip_distances) + if (index >= gl_info->limits.user_clip_distances) return; gl_info->gl_ops.gl.p_glMatrixMode(GL_MODELVIEW); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4085
2/5 doesn't seem right, prepare_push_constant_buffer() is a client side function and can't be called from the CS. Maybe "prepare" is a misleading name and should be fixed. I guess the problem here is that we have a NULL buffer in this case. Instead of creating the buffer anyway I think we should just skip loading constants. I think I ran into that with GLSL and fixed it there, but forgot about arb :-/ Wrt 3/5, if only for the sake of curiosity, what does the recursion stack look like? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4085#note_48608
2/5 doesn't seem right, prepare_push_constant_buffer() is a client side function and can't be called from the CS. Maybe "prepare" is a misleading name and should be fixed.
It works, technically, since I'm adding WINED3DUSAGE_PRIVATE to the buffer (which maybe should be there anyway). But I understand if it's not acceptable.
I guess the problem here is that we have a NULL buffer in this case. Instead of creating the buffer anyway I think we should just skip loading constants.
That's the problem, yes, but it's a bit more subtle. Specifically, the ARB backend accesses int / bool constant data directly (because in general shader generation depends on those values) and it might happen that the shader has no constants at all, so there is no previous call to wined3d_device_context_push_constants(). It's probably not as critical with the GLSL backend but in theory the issue is there as well I think? Skipping constant loading altogether should do the trick though, I'll give that a try. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4085#note_48657
Wrt 3/5, if only for the sake of curiosity, what does the recursion stack look like?
It looks like that infinite recursion only happens with the backbuffer ORM (otherwise the FBO blitter apparently takes care of that particular case - I haven't checked the details). The call chain seems to be: <pre> wined3d_cs_exec_blt_sub_resource() texture2d_blt() raw_blitter_blit() ffp_blitter_blit() surface_cpu_blt() wined3d_texture_load_location(dst_texture,...) raw_blitter_blit() ... </pre> -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4085#note_48658
It works, technically, since I'm adding WINED3DUSAGE_PRIVATE to the buffer (which maybe should be there anyway). But I understand if it's not acceptable.
I don't think creating a PRIVATE resource from the client thread works, but even if it does, the code isn't thread-safe with this change. We should just handle the case where the buffer hasn't been set and fill things with default values. I believe GLSL handles this by virtue of just never getting constant_update_mask, but I could be wrong. I remember fixing this but maybe I submitted from the wrong branch... -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4085#note_48667
participants (3)
-
Matteo Bruni -
Matteo Bruni (＠Mystral)
-
Zebediah Figura (＠zfigura)