Here are 5 more patches on top of !4624, mostly along the same lines. I realize it's super late, no need to rush reviewing :slight_smile:
-- v2: d3d9/tests: Test creating a texture on a NULL HWND device. d3d9/tests: Don't create a vertex shader in test_desktop_window() when unsupported. wined3d: Conditionally allow sRGB writes with the 'none' shader backend. wined3d: Conditionally support WINED3D_FRAGMENT_CAP_SRGB_WRITE on the ffp fragment pipe. d3d9/tests: Skip test_sample_attached_rendertarget() without pixel shaders support. wined3d: Don't override texture parameters for COND_NP2 on multisample textures. wined3d: Don't skip FFP projection transform update. d3d9: Don't do instanced draws in DrawPrimitive() and DrawPrimitiveUP(). wined3d: Rename WINED3DUSAGE_PRIVATE to WINED3DUSAGE_CS.
From: Matteo Bruni mbruni@codeweavers.com
Which is really what it means nowadays: resources created by the CS. It has mostly implications WRT thread safety.
In particular, the flag doesn't mean that the resource doesn't participate in memory accounting (that was split into WINED3DUSAGE_VIDMEM_ACCOUNTING by 723cd0a4ae76ec45320290893c7af7f90b668556), nor that it isn't checked for Reset purposes (we use parent == NULL for that). --- dlls/wined3d/arb_program_shader.c | 2 +- dlls/wined3d/glsl_shader.c | 2 +- dlls/wined3d/resource.c | 6 +++--- dlls/wined3d/surface.c | 2 +- dlls/wined3d/texture.c | 6 +++--- include/wine/wined3d.h | 2 +- 6 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/dlls/wined3d/arb_program_shader.c b/dlls/wined3d/arb_program_shader.c index edb9d40cfcf..4e5e3482dfd 100644 --- a/dlls/wined3d/arb_program_shader.c +++ b/dlls/wined3d/arb_program_shader.c @@ -7771,7 +7771,7 @@ static DWORD arbfp_blitter_blit(struct wined3d_blitter *blitter, enum wined3d_bl desc.format = src_texture->resource.format->id; desc.multisample_type = src_texture->resource.multisample_type; desc.multisample_quality = src_texture->resource.multisample_quality; - desc.usage = WINED3DUSAGE_PRIVATE; + desc.usage = WINED3DUSAGE_CS; desc.bind_flags = 0; desc.access = WINED3D_RESOURCE_ACCESS_GPU; desc.width = wined3d_texture_get_level_width(src_texture, src_level); diff --git a/dlls/wined3d/glsl_shader.c b/dlls/wined3d/glsl_shader.c index 933f134830a..fff45bbeba2 100644 --- a/dlls/wined3d/glsl_shader.c +++ b/dlls/wined3d/glsl_shader.c @@ -13265,7 +13265,7 @@ static DWORD glsl_blitter_blit(struct wined3d_blitter *blitter, enum wined3d_bli desc.format = src_texture->resource.format->id; desc.multisample_type = src_texture->resource.multisample_type; desc.multisample_quality = src_texture->resource.multisample_quality; - desc.usage = WINED3DUSAGE_PRIVATE; + desc.usage = WINED3DUSAGE_CS; desc.bind_flags = 0; desc.access = WINED3D_RESOURCE_ACCESS_GPU; desc.width = wined3d_texture_get_level_width(src_texture, src_level); diff --git a/dlls/wined3d/resource.c b/dlls/wined3d/resource.c index c693a0e1321..bd8391ec3b3 100644 --- a/dlls/wined3d/resource.c +++ b/dlls/wined3d/resource.c @@ -35,7 +35,7 @@ static void resource_check_usage(uint32_t usage, unsigned int access) | WINED3DUSAGE_OVERLAY | WINED3DUSAGE_SCRATCH | WINED3DUSAGE_MANAGED - | WINED3DUSAGE_PRIVATE + | WINED3DUSAGE_CS | WINED3DUSAGE_LEGACY_CUBEMAP | ~WINED3DUSAGE_MASK;
@@ -222,7 +222,7 @@ HRESULT resource_init(struct wined3d_resource *resource, struct wined3d_device * adapter_adjust_memory(device->adapter, size); }
- if (!(usage & WINED3DUSAGE_PRIVATE)) + if (!(usage & WINED3DUSAGE_CS)) device_resource_add(device, resource);
return WINED3D_OK; @@ -248,7 +248,7 @@ void resource_cleanup(struct wined3d_resource *resource) adapter_adjust_memory(resource->device->adapter, (INT64)0 - resource->size); }
- if (!(resource->usage & WINED3DUSAGE_PRIVATE)) + if (!(resource->usage & WINED3DUSAGE_CS)) device_resource_released(resource->device, resource);
wined3d_resource_reference(resource); diff --git a/dlls/wined3d/surface.c b/dlls/wined3d/surface.c index 5c783307b71..e413689599d 100644 --- a/dlls/wined3d/surface.c +++ b/dlls/wined3d/surface.c @@ -271,7 +271,7 @@ static struct wined3d_texture *surface_convert_format(struct wined3d_texture *sr desc.format = dst_format->id; desc.multisample_type = WINED3D_MULTISAMPLE_NONE; desc.multisample_quality = 0; - desc.usage = WINED3DUSAGE_SCRATCH | WINED3DUSAGE_PRIVATE; + desc.usage = WINED3DUSAGE_SCRATCH | WINED3DUSAGE_CS; desc.bind_flags = 0; desc.access = WINED3D_RESOURCE_ACCESS_CPU | WINED3D_RESOURCE_ACCESS_MAP_R | WINED3D_RESOURCE_ACCESS_MAP_W; desc.width = wined3d_texture_get_level_width(src_texture, texture_level); diff --git a/dlls/wined3d/texture.c b/dlls/wined3d/texture.c index 7fb06d71dcd..a531998ddd2 100644 --- a/dlls/wined3d/texture.c +++ b/dlls/wined3d/texture.c @@ -418,7 +418,7 @@ static void texture2d_blt_fbo(struct wined3d_device *device, struct wined3d_cont desc.format = resolve_format_id; desc.multisample_type = src_texture->resource.multisample_type; desc.multisample_quality = src_texture->resource.multisample_quality; - desc.usage = WINED3DUSAGE_PRIVATE; + desc.usage = WINED3DUSAGE_CS; desc.bind_flags = 0; desc.access = WINED3D_RESOURCE_ACCESS_GPU; desc.width = wined3d_texture_get_level_width(src_texture, src_level); @@ -457,7 +457,7 @@ static void texture2d_blt_fbo(struct wined3d_device *device, struct wined3d_cont desc.format = resolve_format_id; desc.multisample_type = dst_texture->resource.multisample_type; desc.multisample_quality = dst_texture->resource.multisample_quality; - desc.usage = WINED3DUSAGE_PRIVATE; + desc.usage = WINED3DUSAGE_CS; desc.bind_flags = 0; desc.access = WINED3D_RESOURCE_ACCESS_GPU; desc.width = wined3d_texture_get_level_width(dst_texture, dst_level); @@ -6298,7 +6298,7 @@ static DWORD ffp_blitter_blit(struct wined3d_blitter *blitter, enum wined3d_blit desc.format = src_texture->resource.format->id; desc.multisample_type = src_texture->resource.multisample_type; desc.multisample_quality = src_texture->resource.multisample_quality; - desc.usage = WINED3DUSAGE_PRIVATE; + desc.usage = WINED3DUSAGE_CS; desc.bind_flags = 0; desc.access = WINED3D_RESOURCE_ACCESS_GPU; desc.width = wined3d_texture_get_level_width(src_texture, src_level); diff --git a/include/wine/wined3d.h b/include/wine/wined3d.h index 1336f10ff7a..b3c1b2f8152 100644 --- a/include/wine/wined3d.h +++ b/include/wine/wined3d.h @@ -930,7 +930,7 @@ enum wined3d_memory_segment_group #define WINED3DUSAGE_MASK 0x10007bf0
#define WINED3DUSAGE_SCRATCH 0x00400000 -#define WINED3DUSAGE_PRIVATE 0x00800000 +#define WINED3DUSAGE_CS 0x00800000 #define WINED3DUSAGE_LEGACY_CUBEMAP 0x01000000 #define WINED3DUSAGE_OWNDC 0x02000000 #define WINED3DUSAGE_STATICDECL 0x04000000
From: Matteo Bruni mbruni@codeweavers.com
On Windows this behavior is hardware dependent. Setting instance_count to 0 makes all our d3d versions consistent in this regard and avoids triggering the FIXME() in wined3d:draw_primitive_immediate_mode(). --- dlls/d3d9/device.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/dlls/d3d9/device.c b/dlls/d3d9/device.c index 1267dc8fca1..31dffbf4b60 100644 --- a/dlls/d3d9/device.c +++ b/dlls/d3d9/device.c @@ -3176,7 +3176,7 @@ static HRESULT WINAPI d3d9_device_DrawPrimitive(IDirect3DDevice9Ex *iface, wined3d_primitive_type_from_d3d(primitive_type), 0);
/* Instancing is ignored for non-indexed draws. */ - wined3d_device_context_draw(device->immediate_context, start_vertex, vertex_count, 0, 1); + wined3d_device_context_draw(device->immediate_context, start_vertex, vertex_count, 0, 0);
d3d9_rts_flag_auto_gen_mipmap(device); wined3d_mutex_unlock(); @@ -3274,7 +3274,7 @@ static HRESULT WINAPI d3d9_device_DrawPrimitiveUP(IDirect3DDevice9Ex *iface, wined3d_device_apply_stateblock(device->wined3d_device, device->state);
/* Instancing is ignored for non-indexed draws. */ - wined3d_device_context_draw(device->immediate_context, vb_pos / stride, vtx_count, 0, 1); + wined3d_device_context_draw(device->immediate_context, vb_pos / stride, vtx_count, 0, 0);
wined3d_stateblock_set_stream_source(device->state, 0, NULL, 0, 0); d3d9_rts_flag_auto_gen_mipmap(device);
From: Matteo Bruni mbruni@codeweavers.com
In the same vein as e106bbdd39a5f27bce028ed992033eb0a5635f60. --- dlls/wined3d/ffp_gl.c | 28 ++++++++++------------------ 1 file changed, 10 insertions(+), 18 deletions(-)
diff --git a/dlls/wined3d/ffp_gl.c b/dlls/wined3d/ffp_gl.c index 8a87248de4f..e4c44f77609 100644 --- a/dlls/wined3d/ffp_gl.c +++ b/dlls/wined3d/ffp_gl.c @@ -3528,6 +3528,8 @@ static void transform_projection(struct wined3d_context *context, const struct w const struct wined3d_gl_info *gl_info = wined3d_context_gl(context)->gl_info; struct wined3d_matrix projection;
+ TRACE("context %p, state %p, state_id %lu.\n", context, state, state_id); + gl_info->gl_ops.gl.p_glMatrixMode(GL_PROJECTION); checkGLcall("glMatrixMode(GL_PROJECTION)");
@@ -3558,6 +3560,8 @@ static void vertexdeclaration(struct wined3d_context *context, const struct wine BOOL wasrhw = context->last_was_rhw; unsigned int i;
+ TRACE("context %p, state %p, state_id %lu.\n", context, state, state_id); + transformed = context->stream_info.position_transformed; if (transformed != context->last_was_rhw && !useVertexShaderFunction) updateFog = TRUE; @@ -3574,25 +3578,12 @@ static void vertexdeclaration(struct wined3d_context *context, const struct wine * make sure they're properly set. */ if (!useVertexShaderFunction) { - /* TODO: Move this mainly to the viewport state and only apply when - * the vp has changed or transformed / untransformed was switched. */ if (wasrhw != context->last_was_rhw - && !isStateDirty(context, STATE_TRANSFORM(WINED3D_TS_PROJECTION)) && !isStateDirty(context, STATE_VIEWPORT)) transform_projection(context, state, STATE_TRANSFORM(WINED3D_TS_PROJECTION)); - /* World matrix needs reapplication here only if we're switching between rhw and non-rhw - * mode. - * - * If a vertex shader is used, the world matrix changed and then vertex shader unbound - * this check will fail and the matrix not applied again. This is OK because a simple - * world matrix change reapplies the matrix - These checks here are only to satisfy the - * needs of the vertex declaration. - * - * World and view matrix go into the same gl matrix, so only apply them when neither is - * dirty - */ - if (transformed != wasrhw && !isStateDirty(context, STATE_TRANSFORM(WINED3D_TS_WORLD_MATRIX(0))) - && !isStateDirty(context, STATE_TRANSFORM(WINED3D_TS_VIEW))) + /* World matrix needs reapplication here only if we're switching + * between rhw and non-rhw mode. */ + if (transformed != wasrhw) transform_world(context, state, STATE_TRANSFORM(WINED3D_TS_WORLD_MATRIX(0))); if (!isStateDirty(context, STATE_RENDER(WINED3D_RS_COLORVERTEX))) context_apply_state(context, state, STATE_RENDER(WINED3D_RS_COLORVERTEX)); @@ -3830,8 +3821,9 @@ static void viewport_miscpart_cc(struct wined3d_context *context,
static void viewport_vertexpart(struct wined3d_context *context, const struct wined3d_state *state, DWORD state_id) { - if (!isStateDirty(context, STATE_TRANSFORM(WINED3D_TS_PROJECTION))) - transform_projection(context, state, STATE_TRANSFORM(WINED3D_TS_PROJECTION)); + TRACE("context %p, state %p, state_id %lu.\n", context, state, state_id); + + transform_projection(context, state, STATE_TRANSFORM(WINED3D_TS_PROJECTION)); if (!isStateDirty(context, STATE_RENDER(WINED3D_RS_POINTSCALEENABLE)) && state->render_states[WINED3D_RS_POINTSCALEENABLE]) state_pscale(context, state, STATE_RENDER(WINED3D_RS_POINTSCALEENABLE));
From: Matteo Bruni mbruni@codeweavers.com
Those parameters are not supported on multisample textures in the first place (basically, GL mandates ARB_sampler_objects for those).
The combination COND_NP2 + ARB_texture_multisample is not expected to happen without wined3d configuration overrides. --- dlls/wined3d/texture.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/dlls/wined3d/texture.c b/dlls/wined3d/texture.c index a531998ddd2..2fc6d5c06df 100644 --- a/dlls/wined3d/texture.c +++ b/dlls/wined3d/texture.c @@ -1377,7 +1377,8 @@ GLuint wined3d_texture_gl_prepare_gl_texture(struct wined3d_texture_gl *texture_ gl_info->gl_ops.gl.p_glTexParameteri(target, GL_TEXTURE_WRAP_R, GL_CLAMP_TO_EDGE); }
- if (texture_gl->t.flags & WINED3D_TEXTURE_COND_NP2) + if (texture_gl->t.flags & WINED3D_TEXTURE_COND_NP2 && target != GL_TEXTURE_2D_MULTISAMPLE + && target != GL_TEXTURE_2D_MULTISAMPLE_ARRAY) { /* Conditional non power of two textures use a different clamping * default. If we're using the GL_WINE_normalized_texrect partial
From: Matteo Bruni mbruni@codeweavers.com
--- dlls/wined3d/ffp_gl.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/dlls/wined3d/ffp_gl.c b/dlls/wined3d/ffp_gl.c index e4c44f77609..8af67fc12df 100644 --- a/dlls/wined3d/ffp_gl.c +++ b/dlls/wined3d/ffp_gl.c @@ -4819,6 +4819,7 @@ static const struct wined3d_state_entry_template ffp_fragmentstate_template[] = { STATE_RENDER(WINED3D_RS_FOGVERTEXMODE), { STATE_RENDER(WINED3D_RS_FOGENABLE), NULL }, WINED3D_GL_EXT_NONE }, { STATE_RENDER(WINED3D_RS_FOGSTART), { STATE_RENDER(WINED3D_RS_FOGSTART), state_fogstartend }, WINED3D_GL_EXT_NONE }, { STATE_RENDER(WINED3D_RS_FOGEND), { STATE_RENDER(WINED3D_RS_FOGSTART), NULL }, WINED3D_GL_EXT_NONE }, + { STATE_RENDER(WINED3D_RS_SRGBWRITEENABLE), { STATE_RENDER(WINED3D_RS_SRGBWRITEENABLE), state_srgbwrite }, ARB_FRAMEBUFFER_SRGB }, { STATE_RENDER(WINED3D_RS_SHADEMODE), { STATE_RENDER(WINED3D_RS_SHADEMODE), state_shademode }, WINED3D_GL_EXT_NONE }, { STATE_SAMPLER(0), { STATE_SAMPLER(0), sampler_texdim }, WINED3D_GL_EXT_NONE }, { STATE_SAMPLER(1), { STATE_SAMPLER(1), sampler_texdim }, WINED3D_GL_EXT_NONE }, @@ -4897,6 +4898,7 @@ static void ffp_fragment_get_caps(const struct wined3d_adapter *adapter, struct | WINED3DTEXOPCAPS_SELECTARG1 | WINED3DTEXOPCAPS_SELECTARG2 | WINED3DTEXOPCAPS_DISABLE; + caps->srgb_write = !!gl_info->supported[ARB_FRAMEBUFFER_SRGB];
if (gl_info->supported[ARB_TEXTURE_ENV_COMBINE] || gl_info->supported[EXT_TEXTURE_ENV_COMBINE]
From: Matteo Bruni mbruni@codeweavers.com
--- dlls/d3d9/tests/visual.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)
diff --git a/dlls/d3d9/tests/visual.c b/dlls/d3d9/tests/visual.c index 20df3d29b1b..0d64081f7e0 100644 --- a/dlls/d3d9/tests/visual.c +++ b/dlls/d3d9/tests/visual.c @@ -26877,6 +26877,7 @@ static void test_sample_attached_rendertarget(void) unsigned int color, i; IDirect3D9 *d3d; ULONG refcount; + D3DCAPS9 caps; BOOL is_warp; HWND window; HRESULT hr; @@ -26926,6 +26927,17 @@ static void test_sample_attached_rendertarget(void) return; }
+ hr = IDirect3DDevice9_GetDeviceCaps(device, &caps); + ok(hr == D3D_OK, "Got unexpected hr %#lx.\n", hr); + if (caps.PixelShaderVersion < D3DPS_VERSION(2, 0)) + { + skip("No shader model 2 support, skipping tests.\n"); + IDirect3DDevice9_Release(device); + IDirect3D9_Release(d3d); + DestroyWindow(window); + return; + } + hr = IDirect3DDevice9_CreateQuery(device, D3DQUERYTYPE_EVENT, NULL); if (hr == D3DERR_NOTAVAILABLE) {
From: Matteo Bruni mbruni@codeweavers.com
--- dlls/wined3d/utils.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/dlls/wined3d/utils.c b/dlls/wined3d/utils.c index badd25ddebb..dcbc177d29e 100644 --- a/dlls/wined3d/utils.c +++ b/dlls/wined3d/utils.c @@ -3193,7 +3193,8 @@ static BOOL init_format_texture_info(struct wined3d_adapter *adapter, struct win
adapter->fragment_pipe->get_caps(adapter, &fragment_caps); adapter->shader_backend->shader_get_caps(adapter, &shader_caps); - srgb_write = fragment_caps.srgb_write && (shader_caps.wined3d_caps & WINED3D_SHADER_CAP_SRGB_WRITE); + srgb_write = fragment_caps.srgb_write + && (!shader_caps.ps_version || (shader_caps.wined3d_caps & WINED3D_SHADER_CAP_SRGB_WRITE));
for (i = 0; i < ARRAY_SIZE(format_texture_info); ++i) {
From: Matteo Bruni mbruni@codeweavers.com
--- dlls/d3d9/tests/visual.c | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-)
diff --git a/dlls/d3d9/tests/visual.c b/dlls/d3d9/tests/visual.c index 0d64081f7e0..5169d5da559 100644 --- a/dlls/d3d9/tests/visual.c +++ b/dlls/d3d9/tests/visual.c @@ -26423,6 +26423,7 @@ static void test_desktop_window(void) unsigned int color; IDirect3D9 *d3d; ULONG refcount; + D3DCAPS9 caps; HWND window; HRESULT hr;
@@ -26475,9 +26476,18 @@ static void test_desktop_window(void) device = create_device(d3d, NULL, NULL, TRUE); ok(device != NULL, "Failed to create a D3D device\n");
- hr = IDirect3DDevice9_CreateVertexShader(device, simple_vs, &shader); - ok(SUCCEEDED(hr), "Failed to create vertex shader, hr %#lx.\n", hr); - IDirect3DVertexShader9_Release(shader); + hr = IDirect3DDevice9_GetDeviceCaps(device, &caps); + ok(SUCCEEDED(hr), "Failed to get device caps, hr %#lx.\n", hr); + if (caps.VertexShaderVersion >= D3DVS_VERSION(1, 1)) + { + hr = IDirect3DDevice9_CreateVertexShader(device, simple_vs, &shader); + ok(SUCCEEDED(hr), "Failed to create vertex shader, hr %#lx.\n", hr); + IDirect3DVertexShader9_Release(shader); + } + else + { + skip("Vertex shaders not supported.\n"); + }
IDirect3DDevice9_Release(device);
From: Matteo Bruni mbruni@codeweavers.com
--- dlls/d3d9/tests/visual.c | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/dlls/d3d9/tests/visual.c b/dlls/d3d9/tests/visual.c index 5169d5da559..005b86cdd09 100644 --- a/dlls/d3d9/tests/visual.c +++ b/dlls/d3d9/tests/visual.c @@ -26419,6 +26419,7 @@ static void test_nrm_instruction(void) static void test_desktop_window(void) { IDirect3DVertexShader9 *shader; + IDirect3DTexture9 *texture; IDirect3DDevice9 *device; unsigned int color; IDirect3D9 *d3d; @@ -26476,6 +26477,10 @@ static void test_desktop_window(void) device = create_device(d3d, NULL, NULL, TRUE); ok(device != NULL, "Failed to create a D3D device\n");
+ hr = IDirect3DDevice9_CreateTexture(device, 1, 1, 1, 0, D3DFMT_A8R8G8B8, D3DPOOL_SYSTEMMEM, &texture, NULL); + ok(hr == D3D_OK, "Got unexpected hr %#lx.\n", hr); + IDirect3DTexture9_Release(texture); + hr = IDirect3DDevice9_GetDeviceCaps(device, &caps); ok(SUCCEEDED(hr), "Failed to get device caps, hr %#lx.\n", hr); if (caps.VertexShaderVersion >= D3DVS_VERSION(1, 1))
On Tue Apr 16 16:44:53 2024 +0000, Elizabeth Figura wrote:
So, I let more time go by and now many of the patches in this MR are
arguably moot, since we're going to get rid of the related code altogether; I can drop those.
The other option is to keep them in the MR (and resubmit soon!) so
that the code is in a better shape when it's eventually removed.
Let me know which one you prefer.
Whichever is easier for you :-) Dropping 10/10 is probably best, at least, since clipping emulation will be obsolete now.
For the time being, I dropped 10/10 but left the rest in; I'm open to dropping more, it's no trouble.
It looks like I accepted your suggestion about patch 7/10 back then (i.e. I found a fixup patch in my local branch when I went to rebase), but otherwise there should be no significant changes.
This merge request was approved by Elizabeth Figura.
Regarding patch 4/10, should we just forbid creating multisample NPOT textures in the first place? **Sadly I'm sure nobody has a GPU of that era to test with**...
Sorry for jumping-in. Just for the sake of completeness, I think [Stefan Dösinger](https://gitlab.freedesktop.org/mesa/mesa/-/issues/347) has (or had) some access to old RV530 Radeon hardware. :wink: And when it comes to old GPU hardware Mesa driver development, [Pavel Ondračka](https://gitlab.freedesktop.org/ondracka) is also in 2024 quite active in regard to the r300g Radeon driver. :thumbsup:
This merge request was approved by Jan Sikorski.
-
Clemens Leu (＠lorn10) -
Elizabeth Figura (＠zfigura)
-
Jan Sikorski (＠jsikorski)
-
Matteo Bruni
-
Matteo Bruni (＠Mystral)