[PATCH v2 0/4] MR4888: wined3d: Avoid assuming the GL backend in (what can be) common code.
These functions should be usable for Vulkan as well. -- v2: wined3d: Collapse some trivially nested ifs into a single condition. https://gitlab.winehq.org/wine/wine/-/merge_requests/4888
From: Zebediah Figura <zfigura(a)codeweavers.com> --- dlls/wined3d/shader.c | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/dlls/wined3d/shader.c b/dlls/wined3d/shader.c index 1f0fe13402c..611402ba9a7 100644 --- a/dlls/wined3d/shader.c +++ b/dlls/wined3d/shader.c @@ -26,7 +26,6 @@ #include <string.h> #include "wined3d_private.h" -#include "wined3d_gl.h" WINE_DEFAULT_DEBUG_CHANNEL(d3d_shader); @@ -2841,6 +2840,8 @@ void find_ps_compile_args(const struct wined3d_state *state, const struct wined3 { for (i = 0; i < shader->limits->sampler; ++i) { + enum wined3d_shader_tex_types type = WINED3D_SHADER_TEX_2D; + if (!shader->reg_maps.resource_info[i].type) continue; @@ -2851,20 +2852,24 @@ void find_ps_compile_args(const struct wined3d_state *state, const struct wined3 continue; texture = texture_from_resource(view->resource); - switch (wined3d_texture_gl(texture)->target) + switch (texture->resource.type) { - /* RECT textures are distinguished from 2D textures via np2_fixup */ - default: + case WINED3D_RTYPE_NONE: + case WINED3D_RTYPE_BUFFER: + case WINED3D_RTYPE_TEXTURE_1D: + assert(0); + + case WINED3D_RTYPE_TEXTURE_2D: + if (texture->resource.usage & WINED3DUSAGE_LEGACY_CUBEMAP) + type = WINED3D_SHADER_TEX_CUBE; break; - case GL_TEXTURE_3D: - args->tex_types |= WINED3D_SHADER_TEX_3D << i * WINED3D_PSARGS_TEXTYPE_SHIFT; - break; - - case GL_TEXTURE_CUBE_MAP_ARB: - args->tex_types |= WINED3D_SHADER_TEX_CUBE << i * WINED3D_PSARGS_TEXTYPE_SHIFT; + case WINED3D_RTYPE_TEXTURE_3D: + type = WINED3D_SHADER_TEX_3D; break; } + + args->tex_types |= (type << (i * WINED3D_PSARGS_TEXTYPE_SHIFT)); } } else if (shader->reg_maps.shader_version.major <= 3) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4888
From: Zebediah Figura <zfigura(a)codeweavers.com> --- dlls/wined3d/utils.c | 23 +---------------------- 1 file changed, 1 insertion(+), 22 deletions(-) diff --git a/dlls/wined3d/utils.c b/dlls/wined3d/utils.c index a46a4c7257d..cda31374c93 100644 --- a/dlls/wined3d/utils.c +++ b/dlls/wined3d/utils.c @@ -6437,30 +6437,9 @@ void wined3d_ffp_get_fs_settings(const struct wined3d_context *context, const st else settings->op[i].color_fixup = texture->resource.format->color_fixup; if (ignore_textype) - { settings->op[i].tex_type = WINED3D_GL_RES_TYPE_TEX_1D; - } else - { - switch (wined3d_texture_gl(texture)->target) - { - case GL_TEXTURE_1D: - settings->op[i].tex_type = WINED3D_GL_RES_TYPE_TEX_1D; - break; - case GL_TEXTURE_2D: - settings->op[i].tex_type = WINED3D_GL_RES_TYPE_TEX_2D; - break; - case GL_TEXTURE_3D: - settings->op[i].tex_type = WINED3D_GL_RES_TYPE_TEX_3D; - break; - case GL_TEXTURE_CUBE_MAP_ARB: - settings->op[i].tex_type = WINED3D_GL_RES_TYPE_TEX_CUBE; - break; - case GL_TEXTURE_RECTANGLE_ARB: - settings->op[i].tex_type = WINED3D_GL_RES_TYPE_TEX_RECT; - break; - } - } + settings->op[i].tex_type = texture->resource.gl_type; } else { settings->op[i].color_fixup = COLOR_FIXUP_IDENTITY; settings->op[i].tex_type = WINED3D_GL_RES_TYPE_TEX_1D; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4888
From: Zebediah Figura <zfigura(a)codeweavers.com> This check was introduced in 4640be8dc834. At that point the only way for that condition to be false was for a cube map texture. It's not clear why cube maps are relevant here; we have no tests for this, but it seems most expedient to just preserve the intent of the check. --- dlls/wined3d/utils.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/dlls/wined3d/utils.c b/dlls/wined3d/utils.c index cda31374c93..a0b1fded9f5 100644 --- a/dlls/wined3d/utils.c +++ b/dlls/wined3d/utils.c @@ -6479,11 +6479,7 @@ void wined3d_ffp_get_fs_settings(const struct wined3d_context *context, const st if (!i && texture && state->render_states[WINED3D_RS_COLORKEYENABLE]) { - GLenum texture_dimensions; - - texture_dimensions = wined3d_texture_gl(texture)->target; - - if (texture_dimensions == GL_TEXTURE_2D || texture_dimensions == GL_TEXTURE_RECTANGLE_ARB) + if (!(texture->resource.usage & WINED3DUSAGE_LEGACY_CUBEMAP)) { if (texture->async.color_key_flags & WINED3D_CKEY_SRC_BLT && !texture->resource.format->alpha_size) { -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4888
From: Zebediah Figura <zfigura(a)codeweavers.com> --- dlls/wined3d/utils.c | 54 +++++++++++++++++++++++--------------------- 1 file changed, 28 insertions(+), 26 deletions(-) diff --git a/dlls/wined3d/utils.c b/dlls/wined3d/utils.c index a0b1fded9f5..24bbad0f6fa 100644 --- a/dlls/wined3d/utils.c +++ b/dlls/wined3d/utils.c @@ -6477,35 +6477,37 @@ void wined3d_ffp_get_fs_settings(const struct wined3d_context *context, const st aarg0 = (args[aop] & ARG0) ? state->texture_states[i][WINED3D_TSS_ALPHA_ARG0] : ARG_UNUSED; } - if (!i && texture && state->render_states[WINED3D_RS_COLORKEYENABLE]) + if (!i && state->render_states[WINED3D_RS_COLORKEYENABLE] + && texture && !(texture->resource.usage & WINED3DUSAGE_LEGACY_CUBEMAP) + && (texture->async.color_key_flags & WINED3D_CKEY_SRC_BLT) && !texture->resource.format->alpha_size) { - if (!(texture->resource.usage & WINED3DUSAGE_LEGACY_CUBEMAP)) + if (aop == WINED3D_TOP_DISABLE) { - if (texture->async.color_key_flags & WINED3D_CKEY_SRC_BLT && !texture->resource.format->alpha_size) + aarg1 = WINED3DTA_TEXTURE; + aop = WINED3D_TOP_SELECT_ARG1; + } + else if (aop == WINED3D_TOP_SELECT_ARG1 && aarg1 != WINED3DTA_TEXTURE) + { + if (state->blend_state && state->blend_state->desc.rt[0].enable) { - if (aop == WINED3D_TOP_DISABLE) - { - aarg1 = WINED3DTA_TEXTURE; - aop = WINED3D_TOP_SELECT_ARG1; - } - else if (aop == WINED3D_TOP_SELECT_ARG1 && aarg1 != WINED3DTA_TEXTURE) - { - if (state->blend_state && state->blend_state->desc.rt[0].enable) - { - aarg2 = WINED3DTA_TEXTURE; - aop = WINED3D_TOP_MODULATE; - } - else aarg1 = WINED3DTA_TEXTURE; - } - else if (aop == WINED3D_TOP_SELECT_ARG2 && aarg2 != WINED3DTA_TEXTURE) - { - if (state->blend_state && state->blend_state->desc.rt[0].enable) - { - aarg1 = WINED3DTA_TEXTURE; - aop = WINED3D_TOP_MODULATE; - } - else aarg2 = WINED3DTA_TEXTURE; - } + aarg2 = WINED3DTA_TEXTURE; + aop = WINED3D_TOP_MODULATE; + } + else + { + aarg1 = WINED3DTA_TEXTURE; + } + } + else if (aop == WINED3D_TOP_SELECT_ARG2 && aarg2 != WINED3DTA_TEXTURE) + { + if (state->blend_state && state->blend_state->desc.rt[0].enable) + { + aarg1 = WINED3DTA_TEXTURE; + aop = WINED3D_TOP_MODULATE; + } + else + { + aarg2 = WINED3DTA_TEXTURE; } } } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4888
Note that constructs like can often be rewritten as without readability being reduced.
True. I might do that in a separate patch just so the diff is clear.
Added now. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4888#note_58543
It reads to me like it's specifically checking for a 2D texture instead of "not a cube texture", but the distinction may be somewhat moot: WINED3D_CKEY_SRC_BLT implies ddraw, and that means we either have a cube texture or a 2D texture.
Yeah, it could be unnecessarily defensive coding as well. It's not really obvious to me why cube textures or 2D textures would matter for the purposes of GL, though. \[I don't suppose you, or someone else more knowledgeable about early GL, has an idea?\]
It may not be about GL. One thing worth pointing out about wined3d from that time (see e.g. "git show 4640be8d:dlls/wined3d/wined3d_private.h") is that IWineD3DTextureImpl and IWineD3DCubeTextureImpl were separate structures. It may simply be a matter of colour-keyed cube textures being too obscure to be worth the effort without having an application using them. And that's assuming those are supported by ddraw at all; they may not be, IIRC there were some restrictions on colour-keying. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4888#note_58606
This merge request was approved by Jan Sikorski. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4888
participants (4)
-
Henri Verbeet (＠hverbeet) -
Jan Sikorski (＠jsikorski)
-
Zebediah Figura
-
Zebediah Figura (＠zfigura)