[resend PATCH v3 1/4] wined3d: Support AMD alpha to coverage state.
Signed-off-by: Paul Gofman <gofmanp(a)gmail.com> --- v3: - manage the state in state block. dlls/d3d9/tests/visual.c | 2 +- dlls/wined3d/device.c | 28 ++++++++++++++++++++++++++-- dlls/wined3d/stateblock.c | 32 ++++++++++++++++++++++++++++++-- dlls/wined3d/wined3d_private.h | 7 ++++++- include/wine/wined3d.h | 1 + 5 files changed, 64 insertions(+), 6 deletions(-) diff --git a/dlls/d3d9/tests/visual.c b/dlls/d3d9/tests/visual.c index 366862a05a..336d08513d 100644 --- a/dlls/d3d9/tests/visual.c +++ b/dlls/d3d9/tests/visual.c @@ -26680,7 +26680,7 @@ static void test_alpha_to_coverage(void) colour = get_readback_color(&rb, 64, 64); /* Nvidia is probably using some proprietary algorithm for averaging sample colour values. */ - todo_wine ok(color_match(colour, 0x9f404080, 1) || color_match(colour, 0x9f485cbc, 1) /* Nvidia */, + ok(color_match(colour, 0x9f404080, 1) || color_match(colour, 0x9f485cbc, 1) /* Nvidia */, "Got unexpected colour %08x.\n", colour); release_surface_readback(&rb); diff --git a/dlls/wined3d/device.c b/dlls/wined3d/device.c index 8405643741..5636ba7ddf 100644 --- a/dlls/wined3d/device.c +++ b/dlls/wined3d/device.c @@ -529,6 +529,8 @@ void wined3d_device_cleanup(struct wined3d_device *device) if (device->swapchain_count) wined3d_device_uninit_3d(device); + wined3d_blend_state_decref(device->blend_state_atoc_enabled); + wined3d_cs_destroy(device->cs); for (i = 0; i < ARRAY_SIZE(device->multistate_funcs); ++i) @@ -3837,6 +3839,8 @@ void CDECL wined3d_device_apply_stateblock(struct wined3d_device *device, const struct wined3d_d3d_info *d3d_info = &stateblock->device->adapter->d3d_info; const struct wined3d_stateblock_state *state = &stateblock->stateblock_state; unsigned int i, j, count; + BOOL blend_state_set = FALSE; + struct wined3d_color color; TRACE("device %p, stateblock %p.\n", device, stateblock); @@ -3946,15 +3950,24 @@ void CDECL wined3d_device_apply_stateblock(struct wined3d_device *device, { if (i == WINED3D_RS_BLENDFACTOR) { - struct wined3d_color color; + struct wined3d_blend_state *blend_state = stateblock->changed.blend_state + ? state->blend_state : wined3d_device_get_blend_state(device, &color); + wined3d_color_from_d3dcolor(&color, state->rs[i]); - wined3d_device_set_blend_state(device, NULL, &color); + wined3d_device_set_blend_state(device, blend_state, &color); + blend_state_set = TRUE; } else wined3d_device_set_render_state(device, i, state->rs[i]); } } + if (stateblock->changed.blend_state && !blend_state_set) + { + wined3d_device_get_blend_state(device, &color); + wined3d_device_set_blend_state(device, state->blend_state, &color); + } + for (i = 0; i < ARRAY_SIZE(state->texture_states); ++i) { for (j = 0; j < ARRAY_SIZE(state->texture_states[i]); ++j) @@ -5759,12 +5772,23 @@ HRESULT wined3d_device_init(struct wined3d_device *device, struct wined3d *wined struct wined3d_adapter *adapter = wined3d->adapters[adapter_idx]; const struct wined3d_fragment_pipe_ops *fragment_pipeline; const struct wined3d_vertex_pipe_ops *vertex_pipeline; + struct wined3d_blend_state_desc blend_state_desc; unsigned int i; HRESULT hr; if (!wined3d_select_feature_level(adapter, levels, level_count, &device->feature_level)) return E_FAIL; + memset(&blend_state_desc, 0, sizeof(blend_state_desc)); + blend_state_desc.alpha_to_coverage = TRUE; + + if (FAILED(hr = wined3d_blend_state_create(device, &blend_state_desc, + NULL, &wined3d_null_parent_ops, &device->blend_state_atoc_enabled))) + { + ERR("Could not create blend state object.\n"); + return hr; + } + TRACE("Device feature level %s.\n", wined3d_debug_feature_level(device->feature_level)); device->ref = 1; diff --git a/dlls/wined3d/stateblock.c b/dlls/wined3d/stateblock.c index 499d23fbbd..0d3fad0a6d 100644 --- a/dlls/wined3d/stateblock.c +++ b/dlls/wined3d/stateblock.c @@ -209,6 +209,7 @@ static void stateblock_savedstates_set_all(struct wined3d_saved_states *states, states->pixelShader = 1; states->vertexShader = 1; states->scissorRect = 1; + states->blend_state = 1; /* Fixed size arrays */ states->streamSource = 0xffff; @@ -263,6 +264,7 @@ static void stateblock_savedstates_set_vertex(struct wined3d_saved_states *state states->vertexDecl = 1; states->vertexShader = 1; + states->blend_state = 1; for (i = 0; i < ARRAY_SIZE(vertex_states_render); ++i) { @@ -1017,6 +1019,9 @@ void CDECL wined3d_stateblock_capture(struct wined3d_stateblock *stateblock, wined3d_state_record_lights(stateblock->stateblock_state.light_state, state->light_state); + if (stateblock->changed.blend_state) + stateblock->stateblock_state.blend_state = state->blend_state; + TRACE("Capture done.\n"); } @@ -1025,6 +1030,8 @@ void CDECL wined3d_stateblock_apply(const struct wined3d_stateblock *stateblock, { struct wined3d_stateblock_state *state = &device_state->stateblock_state; struct wined3d_device *device = stateblock->device; + BOOL blend_state_set = FALSE; + struct wined3d_color color; unsigned int i; DWORD map; @@ -1121,14 +1128,24 @@ void CDECL wined3d_stateblock_apply(const struct wined3d_stateblock *stateblock, state->rs[rs] = stateblock->stateblock_state.rs[rs]; if (rs == WINED3D_RS_BLENDFACTOR) { - struct wined3d_color color; + struct wined3d_blend_state *blend_state = stateblock->changed.blend_state + ? stateblock->stateblock_state.blend_state + : wined3d_device_get_blend_state(device, &color); + wined3d_color_from_d3dcolor(&color, stateblock->stateblock_state.rs[rs]); - wined3d_device_set_blend_state(device, NULL, &color); + wined3d_device_set_blend_state(device, blend_state, &color); + blend_state_set = TRUE; } else wined3d_device_set_render_state(device, rs, stateblock->stateblock_state.rs[rs]); } + if (stateblock->changed.blend_state && !blend_state_set) + { + wined3d_device_get_blend_state(device, &color); + wined3d_device_set_blend_state(device, stateblock->stateblock_state.blend_state, &color); + } + /* Texture states. */ for (i = 0; i < stateblock->num_contained_tss_states; ++i) { @@ -1426,6 +1443,17 @@ void CDECL wined3d_stateblock_set_render_state(struct wined3d_stateblock *stateb stateblock->stateblock_state.rs[state] = value; stateblock->changed.renderState[state >> 5] |= 1u << (state & 0x1f); + + if (state == WINED3D_RS_POINTSIZE + && (value == WINED3D_ALPHA_TO_COVERAGE_ENABLE || value == WINED3D_ALPHA_TO_COVERAGE_DISABLE)) + { + stateblock->changed.blend_state = 1; + + if (value == WINED3D_ALPHA_TO_COVERAGE_ENABLE) + stateblock->stateblock_state.blend_state = stateblock->device->blend_state_atoc_enabled; + else + stateblock->stateblock_state.blend_state = NULL; + } } void CDECL wined3d_stateblock_set_sampler_state(struct wined3d_stateblock *stateblock, diff --git a/dlls/wined3d/wined3d_private.h b/dlls/wined3d/wined3d_private.h index 0e1e55d52b..f6e5dadf2a 100644 --- a/dlls/wined3d/wined3d_private.h +++ b/dlls/wined3d/wined3d_private.h @@ -76,6 +76,9 @@ #define WINED3D_MAX_DIRTY_REGION_COUNT 7 +#define WINED3D_ALPHA_TO_COVERAGE_ENABLE MAKEFOURCC('A','2','M','1') +#define WINED3D_ALPHA_TO_COVERAGE_DISABLE MAKEFOURCC('A','2','M','0') + struct wined3d_fragment_pipe_ops; struct wined3d_adapter; struct wined3d_context; @@ -3314,6 +3317,7 @@ struct wined3d_device /* Context management */ struct wined3d_context **contexts; UINT context_count; + struct wined3d_blend_state *blend_state_atoc_enabled; }; void wined3d_device_cleanup(struct wined3d_device *device) DECLSPEC_HIDDEN; @@ -3932,7 +3936,8 @@ struct wined3d_saved_states DWORD vertexShader : 1; DWORD scissorRect : 1; DWORD store_stream_offset : 1; - DWORD padding : 4; + DWORD blend_state : 1; + DWORD padding : 3; }; struct StageState { diff --git a/include/wine/wined3d.h b/include/wine/wined3d.h index d313c7aec8..d783e64a26 100644 --- a/include/wine/wined3d.h +++ b/include/wine/wined3d.h @@ -2168,6 +2168,7 @@ struct wined3d_stateblock_state RECT scissor_rect; struct wined3d_light_state *light_state; + struct wined3d_blend_state *blend_state; }; struct wined3d_parent_ops -- 2.24.1
Signed-off-by: Paul Gofman <gofmanp(a)gmail.com> --- v3: - make ATOC state depend on the currently present _ADAPTIVETESS_Y and not on its history as thats what Nvidia does. dlls/d3d9/tests/visual.c | 2 +- dlls/wined3d/state.c | 16 ++++++++++++++-- dlls/wined3d/utils.c | 10 ++++++++++ include/wine/wined3d.h | 1 + 4 files changed, 26 insertions(+), 3 deletions(-) diff --git a/dlls/d3d9/tests/visual.c b/dlls/d3d9/tests/visual.c index 336d08513d..d90f59a498 100644 --- a/dlls/d3d9/tests/visual.c +++ b/dlls/d3d9/tests/visual.c @@ -26604,7 +26604,7 @@ static void test_alpha_to_coverage(void) ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr); if (!adapter_is_amd(&identifier)) { - skip("Alpha to coverage is not supported.\n"); + win_skip("Alpha to coverage is not supported.\n"); refcount = IDirect3DDevice9_Release(device); ok(!refcount, "Device has %u references left.\n", refcount); IDirect3D9_Release(d3d); diff --git a/dlls/wined3d/state.c b/dlls/wined3d/state.c index a4d88e1bfc..ed40ff9b3d 100644 --- a/dlls/wined3d/state.c +++ b/dlls/wined3d/state.c @@ -616,7 +616,7 @@ static void state_blend_factor(struct wined3d_context *context, const struct win static void state_blend_object(struct wined3d_context *context, const struct wined3d_state *state, DWORD state_id) { const struct wined3d_gl_info *gl_info = wined3d_context_gl(context)->gl_info; - BOOL alpha_to_coverage = FALSE; + BOOL alpha_to_coverage; if (!gl_info->supported[ARB_MULTISAMPLE]) return; @@ -626,6 +626,10 @@ static void state_blend_object(struct wined3d_context *context, const struct win struct wined3d_blend_state_desc *desc = &state->blend_state->desc; alpha_to_coverage = desc->alpha_to_coverage; } + else + { + alpha_to_coverage = state->render_states[WINED3D_RS_ADAPTIVETESS_Y] == WINED3DFMT_ATOC; + } if (alpha_to_coverage) gl_info->gl_ops.gl.p_glEnable(GL_SAMPLE_ALPHA_TO_COVERAGE); @@ -1929,6 +1933,14 @@ static void state_nvdb(struct wined3d_context *context, const struct wined3d_sta state_tessellation(context, state, STATE_RENDER(WINED3D_RS_ENABLEADAPTIVETESSELLATION)); } +static void state_nv_atoc(struct wined3d_context *context, const struct wined3d_state *state, DWORD state_id) +{ + if (!isStateDirty(context, STATE_BLEND)) + state_blend_object(context, state, state_id); + + state_tessellation(context, state, STATE_RENDER(WINED3D_RS_ENABLEADAPTIVETESSELLATION)); +} + static void state_wrapu(struct wined3d_context *context, const struct wined3d_state *state, DWORD state_id) { if (state->render_states[WINED3D_RS_WRAPU]) @@ -4647,7 +4659,7 @@ const struct wined3d_state_entry_template misc_state_template[] = { STATE_RENDER(WINED3D_RS_MINTESSELLATIONLEVEL), { STATE_RENDER(WINED3D_RS_ENABLEADAPTIVETESSELLATION),NULL }, WINED3D_GL_EXT_NONE }, { STATE_RENDER(WINED3D_RS_MAXTESSELLATIONLEVEL), { STATE_RENDER(WINED3D_RS_ENABLEADAPTIVETESSELLATION),NULL }, WINED3D_GL_EXT_NONE }, { STATE_RENDER(WINED3D_RS_ADAPTIVETESS_X), { STATE_RENDER(WINED3D_RS_ENABLEADAPTIVETESSELLATION),NULL }, WINED3D_GL_EXT_NONE }, - { STATE_RENDER(WINED3D_RS_ADAPTIVETESS_Y), { STATE_RENDER(WINED3D_RS_ENABLEADAPTIVETESSELLATION),NULL }, WINED3D_GL_EXT_NONE }, + { STATE_RENDER(WINED3D_RS_ADAPTIVETESS_Y), { STATE_RENDER(WINED3D_RS_ADAPTIVETESS_Y), state_nv_atoc }, WINED3D_GL_EXT_NONE }, { STATE_RENDER(WINED3D_RS_ADAPTIVETESS_Z), { STATE_RENDER(WINED3D_RS_ENABLEADAPTIVETESSELLATION),NULL }, WINED3D_GL_EXT_NONE }, { STATE_RENDER(WINED3D_RS_ADAPTIVETESS_W), { STATE_RENDER(WINED3D_RS_ENABLEADAPTIVETESSELLATION),NULL }, WINED3D_GL_EXT_NONE }, { STATE_RENDER(WINED3D_RS_ENABLEADAPTIVETESSELLATION),{ STATE_RENDER(WINED3D_RS_ENABLEADAPTIVETESSELLATION),state_nvdb }, EXT_DEPTH_BOUNDS_TEST }, diff --git a/dlls/wined3d/utils.c b/dlls/wined3d/utils.c index bd239d7659..f696b92214 100644 --- a/dlls/wined3d/utils.c +++ b/dlls/wined3d/utils.c @@ -65,6 +65,7 @@ format_index_remap[] = {WINED3DFMT_R16, WINED3D_FORMAT_FOURCC_BASE + 20}, {WINED3DFMT_AL16, WINED3D_FORMAT_FOURCC_BASE + 21}, {WINED3DFMT_NV12, WINED3D_FORMAT_FOURCC_BASE + 22}, + {WINED3DFMT_ATOC, WINED3D_FORMAT_FOURCC_BASE + 23}, }; #define WINED3D_FORMAT_COUNT (WINED3D_FORMAT_FOURCC_BASE + ARRAY_SIZE(format_index_remap)) @@ -135,6 +136,7 @@ static const struct wined3d_format_channels formats[] = {WINED3DFMT_ATI1N, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0}, {WINED3DFMT_ATI2N, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0}, {WINED3DFMT_NVDB, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}, + {WINED3DFMT_ATOC, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}, {WINED3DFMT_INST, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}, {WINED3DFMT_INTZ, 0, 0, 0, 0, 0, 0, 0, 0, 4, 24, 8}, {WINED3DFMT_RESZ, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}, @@ -339,6 +341,7 @@ static const struct wined3d_format_base_flags format_base_flags[] = {WINED3DFMT_INST, WINED3DFMT_FLAG_EXTENSION}, {WINED3DFMT_NULL, WINED3DFMT_FLAG_EXTENSION}, {WINED3DFMT_NVDB, WINED3DFMT_FLAG_EXTENSION}, + {WINED3DFMT_ATOC, WINED3DFMT_FLAG_EXTENSION}, {WINED3DFMT_RESZ, WINED3DFMT_FLAG_EXTENSION}, }; @@ -3673,6 +3676,12 @@ static void apply_format_fixups(struct wined3d_adapter *adapter, struct wined3d_ format_set_flag(&format->f, WINED3DFMT_FLAG_TEXTURE); } + if (gl_info->supported[ARB_MULTISAMPLE]) + { + format = get_format_gl_internal(adapter, WINED3DFMT_ATOC); + format_set_flag(&format->f, WINED3DFMT_FLAG_TEXTURE); + } + /* RESZ aka AMD DX9-level hack for multisampled depth buffer resolve. You query for RESZ * support by checking for availability of MAKEFOURCC('R','E','S','Z') surfaces with * RENDERTARGET usage. */ @@ -4591,6 +4600,7 @@ const char *debug_d3dformat(enum wined3d_format_id format_id) FMT_TO_STR(WINED3DFMT_R16); FMT_TO_STR(WINED3DFMT_AL16); FMT_TO_STR(WINED3DFMT_NV12); + FMT_TO_STR(WINED3DFMT_ATOC); #undef FMT_TO_STR default: { diff --git a/include/wine/wined3d.h b/include/wine/wined3d.h index d783e64a26..734380b0ad 100644 --- a/include/wine/wined3d.h +++ b/include/wine/wined3d.h @@ -269,6 +269,7 @@ enum wined3d_format_id WINED3DFMT_NV12 = WINEMAKEFOURCC('N','V','1','2'), WINED3DFMT_DF16 = WINEMAKEFOURCC('D','F','1','6'), WINED3DFMT_DF24 = WINEMAKEFOURCC('D','F','2','4'), + WINED3DFMT_ATOC = WINEMAKEFOURCC('A','T','O','C'), WINED3DFMT_FORCE_DWORD = 0xffffffff }; -- 2.24.1
Hi, While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=65027 Your paranoid android. === w1064v1809 (32 bit report) === === w1064v1809_2scr (32 bit report) === === w1064v1809_ar (32 bit report) === === w1064v1809_he (32 bit report) === === w1064v1809_ja (32 bit report) === === w1064v1809_zh_CN (32 bit report) === === w1064v1809 (64 bit report) === === debian10 (32 bit report) === === debian10 (32 bit French report) === === debian10 (32 bit Japanese:Japan report) === === debian10 (32 bit Chinese:China report) === === debian10 (32 bit WoW report) === === debian10 (64 bit WoW report) ===
On Wed, 12 Feb 2020 at 18:24, Paul Gofman <gofmanp(a)gmail.com> wrote:
@@ -616,7 +616,7 @@ static void state_blend_factor(struct wined3d_context *context, const struct win static void state_blend_object(struct wined3d_context *context, const struct wined3d_state *state, DWORD state_id) { const struct wined3d_gl_info *gl_info = wined3d_context_gl(context)->gl_info; - BOOL alpha_to_coverage = FALSE; + BOOL alpha_to_coverage;
if (!gl_info->supported[ARB_MULTISAMPLE]) return; @@ -626,6 +626,10 @@ static void state_blend_object(struct wined3d_context *context, const struct win struct wined3d_blend_state_desc *desc = &state->blend_state->desc; alpha_to_coverage = desc->alpha_to_coverage; } + else + { + alpha_to_coverage = state->render_states[WINED3D_RS_ADAPTIVETESS_Y] == WINED3DFMT_ATOC; + }
if (alpha_to_coverage) gl_info->gl_ops.gl.p_glEnable(GL_SAMPLE_ALPHA_TO_COVERAGE); That works, but it seems tempting to handle the NVIDIA variant in the stateblock as well.
On 2/13/20 17:30, Henri Verbeet wrote:
On Wed, 12 Feb 2020 at 18:24, Paul Gofman <gofmanp(a)gmail.com> wrote:
@@ -616,7 +616,7 @@ static void state_blend_factor(struct wined3d_context *context, const struct win static void state_blend_object(struct wined3d_context *context, const struct wined3d_state *state, DWORD state_id) { const struct wined3d_gl_info *gl_info = wined3d_context_gl(context)->gl_info; - BOOL alpha_to_coverage = FALSE; + BOOL alpha_to_coverage;
if (!gl_info->supported[ARB_MULTISAMPLE]) return; @@ -626,6 +626,10 @@ static void state_blend_object(struct wined3d_context *context, const struct win struct wined3d_blend_state_desc *desc = &state->blend_state->desc; alpha_to_coverage = desc->alpha_to_coverage; } + else + { + alpha_to_coverage = state->render_states[WINED3D_RS_ADAPTIVETESS_Y] == WINED3DFMT_ATOC; + }
if (alpha_to_coverage) gl_info->gl_ops.gl.p_glEnable(GL_SAMPLE_ALPHA_TO_COVERAGE); That works, but it seems tempting to handle the NVIDIA variant in the stateblock as well.
Do you mean bringing in the state trigger behaviour just as for AMD or just move the logic present in this patch to stateblocks somehow? Both variants look less straightforward to me at the moment, as unlike AMD we don't have a pseudo-unique off trigger for Nvidia and it is not immediately clear how to decide when to turn the ATOC state off. For instance, states being set in current stateblock could have never triggered ATOC state but that state can be already set on device previously and the application of stateblock is supposed to turn the state off. I. e., setting the 'changed.blend_state' flag for turning off Nvidia ATOC state becomes cumbersome.
On Thu, 13 Feb 2020 at 18:28, Paul Gofman <gofmanp(a)gmail.com> wrote:
On 2/13/20 17:30, Henri Verbeet wrote:
That works, but it seems tempting to handle the NVIDIA variant in the stateblock as well.
Do you mean bringing in the state trigger behaviour just as for AMD or just move the logic present in this patch to stateblocks somehow? Both variants look less straightforward to me at the moment, as unlike AMD we don't have a pseudo-unique off trigger for Nvidia and it is not immediately clear how to decide when to turn the ATOC state off. For instance, states being set in current stateblock could have never triggered ATOC state but that state can be already set on device previously and the application of stateblock is supposed to turn the state off. I. e., setting the 'changed.blend_state' flag for turning off Nvidia ATOC state becomes cumbersome.
I was thinking of simply taking WINED3D_RS_ADAPTIVETESS_Y into account when setting the blend state in wined3d_device_apply_stateblock(). I.e., some variant of the following: if (changed->blend_state || state_changed(changed->renderState, WINED3D_RS_ADAPTIVETESS_Y)) { blend_state = state->blend_state; if (state->rs[WINED3D_RS_ADAPTIVETESS_Y] == WINED3DFMT_ATOC) blend_state = device->blend_state_atoc_enabled; ... wined3d_device_set_blend_state(..., blend_state, ...); }
On 2/13/20 18:37, Henri Verbeet wrote:
On Thu, 13 Feb 2020 at 18:28, Paul Gofman <gofmanp(a)gmail.com> wrote:
I was thinking of simply taking WINED3D_RS_ADAPTIVETESS_Y into account when setting the blend state in wined3d_device_apply_stateblock(). I.e., some variant of the following:
if (changed->blend_state || state_changed(changed->renderState, WINED3D_RS_ADAPTIVETESS_Y)) { blend_state = state->blend_state; if (state->rs[WINED3D_RS_ADAPTIVETESS_Y] == WINED3DFMT_ATOC) blend_state = device->blend_state_atoc_enabled; ... wined3d_device_set_blend_state(..., blend_state, ...); }
This will reset ATOC state whenever application sets _ADAPTIVETESS_Y, while the application could have set ATOC state AMD way previously (not in the current stateblock, so changed->blend_state is 0 and state->blend_state is NULL). This can probably be worked around by checking the device current blend_state but this way state application will now start combining states with pre-existing on device which looks too twisted. The other probably more straightforward way to solve it would be not to expose both Nvidia and AMD states at the same time but instead make it depend on reported GPU vendor.
On Thu, 13 Feb 2020 at 19:25, Paul Gofman <gofmanp(a)gmail.com> wrote:
On 2/13/20 18:37, Henri Verbeet wrote:
On Thu, 13 Feb 2020 at 18:28, Paul Gofman <gofmanp(a)gmail.com> wrote: I was thinking of simply taking WINED3D_RS_ADAPTIVETESS_Y into account when setting the blend state in wined3d_device_apply_stateblock(). I.e., some variant of the following:
if (changed->blend_state || state_changed(changed->renderState, WINED3D_RS_ADAPTIVETESS_Y)) { blend_state = state->blend_state; if (state->rs[WINED3D_RS_ADAPTIVETESS_Y] == WINED3DFMT_ATOC) blend_state = device->blend_state_atoc_enabled; ... wined3d_device_set_blend_state(..., blend_state, ...); }
This will reset ATOC state whenever application sets _ADAPTIVETESS_Y, while the application could have set ATOC state AMD way previously (not in the current stateblock, so changed->blend_state is 0 and state->blend_state is NULL).
I don't think that can happen for WINED3D_SBT_PRIMARY stateblocks. (I.e., the ones used with wined3d_device_apply_stateblock().) That does mean some loss of generality for wined3d_device_apply_stateblock(), but I don't think that can be maintained once it starts setting state objects anyway. I.e., I think it's fine to assume the stateblock contains complete and consistent state in wined3d_device_apply_stateblock().
On 2/13/20 19:10, Henri Verbeet wrote:
On Thu, 13 Feb 2020 at 19:25, Paul Gofman <gofmanp(a)gmail.com> wrote:
On 2/13/20 18:37, Henri Verbeet wrote:
On Thu, 13 Feb 2020 at 18:28, Paul Gofman <gofmanp(a)gmail.com> wrote: I was thinking of simply taking WINED3D_RS_ADAPTIVETESS_Y into account when setting the blend state in wined3d_device_apply_stateblock(). I.e., some variant of the following:
if (changed->blend_state || state_changed(changed->renderState, WINED3D_RS_ADAPTIVETESS_Y)) { blend_state = state->blend_state; if (state->rs[WINED3D_RS_ADAPTIVETESS_Y] == WINED3DFMT_ATOC) blend_state = device->blend_state_atoc_enabled; ... wined3d_device_set_blend_state(..., blend_state, ...); } This will reset ATOC state whenever application sets _ADAPTIVETESS_Y, while the application could have set ATOC state AMD way previously (not in the current stateblock, so changed->blend_state is 0 and state->blend_state is NULL). I don't think that can happen for WINED3D_SBT_PRIMARY stateblocks. (I.e., the ones used with wined3d_device_apply_stateblock().) That does mean some loss of generality for wined3d_device_apply_stateblock(), but I don't think that can be maintained once it starts setting state objects anyway. I.e., I think it's fine to assume the stateblock contains complete and consistent state in wined3d_device_apply_stateblock().
ATOC state can be set / reset with _SBT_RECORDED stateblocks as well. The logic for this case seems to work now: if the recorded stateblock had an AMD trigger then state block has changed.blend_state and updates it, otherwise it does not touch it. It is encoded not exactly straightforward but looks the same in principle like if we had something like D3DRS_ATOC separate state. I. e., I think we could introduce WINED3D_RS_ATOC and translate _POINTSIZE('A2M1') to _RS_ATOC(TRUE) and _POINTSIZE('A2M0') to _RS_ATOC(FALSE) (I am not suggesting to do so at least because it won't simplify the things in wined3d anyway due to d3d11 blend state object). But maybe it worth to change wined3d_stateblock_set_render_state() and not to record _RS_POINTSIZE in case it is (re)setting ATOC? This would simplify blend state application in wined3d_device_apply_stateblock() and wined3d_device_apply_stateblock(). While d3d9 adaptive tesselation itself which could interfere with AMD ATOC state is barely ever used or supported, my concern was more related to games which like to reset a bunch of random (or all) states now and then just in case. But it is not something I have an application to depend on though.
On 2/13/20 19:41, Paul Gofman wrote:
On 2/13/20 19:10, Henri Verbeet wrote:
On Thu, 13 Feb 2020 at 19:25, Paul Gofman <gofmanp(a)gmail.com> wrote:
On 2/13/20 18:37, Henri Verbeet wrote:
On Thu, 13 Feb 2020 at 18:28, Paul Gofman <gofmanp(a)gmail.com> wrote: I was thinking of simply taking WINED3D_RS_ADAPTIVETESS_Y into account when setting the blend state in wined3d_device_apply_stateblock(). I.e., some variant of the following:
if (changed->blend_state || state_changed(changed->renderState, WINED3D_RS_ADAPTIVETESS_Y)) { blend_state = state->blend_state; if (state->rs[WINED3D_RS_ADAPTIVETESS_Y] == WINED3DFMT_ATOC) blend_state = device->blend_state_atoc_enabled; ... wined3d_device_set_blend_state(..., blend_state, ...); } This will reset ATOC state whenever application sets _ADAPTIVETESS_Y, while the application could have set ATOC state AMD way previously (not in the current stateblock, so changed->blend_state is 0 and state->blend_state is NULL). I don't think that can happen for WINED3D_SBT_PRIMARY stateblocks. (I.e., the ones used with wined3d_device_apply_stateblock().) That does mean some loss of generality for wined3d_device_apply_stateblock(), but I don't think that can be maintained once it starts setting state objects anyway. I.e., I think it's fine to assume the stateblock contains complete and consistent state in wined3d_device_apply_stateblock().
ATOC state can be set / reset with _SBT_RECORDED stateblocks as well. The logic for this case seems to work now: if the recorded stateblock had an AMD trigger then state block has changed.blend_state and updates it, otherwise it does not touch it. It is encoded not exactly straightforward but looks the same in principle like if we had something like D3DRS_ATOC separate state. I. e., I think we could introduce WINED3D_RS_ATOC and translate _POINTSIZE('A2M1') to _RS_ATOC(TRUE) and _POINTSIZE('A2M0') to _RS_ATOC(FALSE) (I am not suggesting to do so at least because it won't simplify the things in wined3d anyway due to d3d11 blend state object). But maybe it worth to change wined3d_stateblock_set_render_state() and not to record _RS_POINTSIZE in case it is (re)setting ATOC? This would simplify blend state application in wined3d_device_apply_stateblock() and wined3d_device_apply_stateblock().
While d3d9 adaptive tesselation itself which could interfere with AMD ATOC state is barely ever used or supported, my concern was more related to games which like to reset a bunch of random (or all) states now and then just in case. But it is not something I have an application to depend on though.
Sorry, please ignore this message. I figured my previous implementation of Nvidia state in state.c is functionally equivalent to what you suggested.
Signed-off-by: Paul Gofman <gofmanp(a)gmail.com> --- dlls/d3d9/tests/visual.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/dlls/d3d9/tests/visual.c b/dlls/d3d9/tests/visual.c index d90f59a498..ccbea1d50a 100644 --- a/dlls/d3d9/tests/visual.c +++ b/dlls/d3d9/tests/visual.c @@ -26560,6 +26560,7 @@ static void test_alpha_to_coverage(void) D3DADAPTER_IDENTIFIER9 identifier; struct surface_readback rb; IDirect3DTexture9 *texture; + IDirect3DStateBlock9 *sb; IDirect3DDevice9 *device; DWORD quality_levels; BOOL nvidia_mode; @@ -26669,6 +26670,9 @@ static void test_alpha_to_coverage(void) ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr); } + hr = IDirect3DDevice9_CreateStateBlock(device, D3DSBT_VERTEXSTATE, &sb); + ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr); + fill_surface(surface, 0x40608000, 0); hr = IDirect3DDevice9_DrawPrimitiveUP(device, D3DPT_TRIANGLESTRIP, 2, quad, sizeof(quad[0])); @@ -26705,6 +26709,23 @@ static void test_alpha_to_coverage(void) ok(colour == 0x40608000, "Got unexpected colour %08x.\n", colour); release_surface_readback(&rb); + hr = IDirect3DDevice9_Clear(device, 0, NULL, D3DCLEAR_TARGET, 0xff2000ff, 0.0f, 0); + ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr); + + IDirect3DStateBlock9_Apply(sb); + IDirect3DStateBlock9_Release(sb); + + hr = IDirect3DDevice9_DrawPrimitiveUP(device, D3DPT_TRIANGLESTRIP, 2, quad, sizeof(quad[0])); + ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr); + + hr = IDirect3DDevice9_StretchRect(device, ms_rt, NULL, rt, NULL, D3DTEXF_POINT); + ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr); + get_rt_readback(rt, &rb); + colour = get_readback_color(&rb, 64, 64); + ok(color_match(colour, 0x9f404080, 1) || color_match(colour, 0x9f485cbc, 1) /* Nvidia */, + "Got unexpected colour %08x.\n", colour); + release_surface_readback(&rb); + hr = IDirect3DDevice9_EndScene(device); ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr); hr = IDirect3DDevice9_Present(device, NULL, NULL, NULL, NULL); -- 2.24.1
Hi, While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=65028 Your paranoid android. === w1064v1809 (32 bit report) === === w1064v1809_2scr (32 bit report) === === w1064v1809_ar (32 bit report) === === w1064v1809_he (32 bit report) === === w1064v1809_ja (32 bit report) === === w1064v1809_zh_CN (32 bit report) === === w1064v1809 (64 bit report) === === debian10 (32 bit report) === === debian10 (32 bit French report) === === debian10 (32 bit Japanese:Japan report) === === debian10 (32 bit Chinese:China report) === === debian10 (32 bit WoW report) === === debian10 (64 bit WoW report) ===
Otherwise if Wine exposes ATOC format the test will always trigger Nvidia path regardless of reported GPU. Signed-off-by: Paul Gofman <gofmanp(a)gmail.com> --- dlls/d3d9/tests/visual.c | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/dlls/d3d9/tests/visual.c b/dlls/d3d9/tests/visual.c index ccbea1d50a..2df39609c7 100644 --- a/dlls/d3d9/tests/visual.c +++ b/dlls/d3d9/tests/visual.c @@ -26591,19 +26591,20 @@ static void test_alpha_to_coverage(void) return; } - if (IDirect3D9_CheckDeviceFormat(d3d, 0, D3DDEVTYPE_HAL, D3DFMT_X8R8G8B8, 0, - D3DRTYPE_SURFACE, MAKEFOURCC('A','T','O','C')) == D3D_OK) + + hr = IDirect3D9_GetAdapterIdentifier(d3d, D3DADAPTER_DEFAULT, 0, &identifier); + ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr); + + if (adapter_is_amd(&identifier)) { - /* ATOC pseudo format is introduced by Nvidia. Some Intel GPUs support - * alpha to coverage the same way as Nvidia. */ - nvidia_mode = TRUE; + nvidia_mode = FALSE; } else { - nvidia_mode = FALSE; - hr = IDirect3D9_GetAdapterIdentifier(d3d, D3DADAPTER_DEFAULT, 0, &identifier); - ok(hr == D3D_OK, "Got unexpected hr %#x.\n", hr); - if (!adapter_is_amd(&identifier)) + /* ATOC pseudo format is introduced by Nvidia. Some Intel GPUs support + * alpha to coverage the same way as Nvidia. */ + if (!IDirect3D9_CheckDeviceFormat(d3d, 0, D3DDEVTYPE_HAL, D3DFMT_X8R8G8B8, 0, + D3DRTYPE_SURFACE, MAKEFOURCC('A','T','O','C')) == D3D_OK) { win_skip("Alpha to coverage is not supported.\n"); refcount = IDirect3DDevice9_Release(device); @@ -26612,6 +26613,7 @@ static void test_alpha_to_coverage(void) DestroyWindow(window); return; } + nvidia_mode = TRUE; } hr = IDirect3DDevice9_CreateRenderTarget(device, 128, 128, -- 2.24.1
I am sorry, this patch has a sort of mistype error. I will resend it as a separate patch as it actually does not interfere with the rest of the series. On 2/12/20 17:53, Paul Gofman wrote:
Otherwise if Wine exposes ATOC format the test will always trigger Nvidia path regardless of reported GPU.
Hi, While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=65029 Your paranoid android. === w1064v1809 (32 bit report) === === w1064v1809_2scr (32 bit report) === === w1064v1809_ar (32 bit report) === === w1064v1809_he (32 bit report) === === w1064v1809_ja (32 bit report) === === w1064v1809_zh_CN (32 bit report) === === w1064v1809 (64 bit report) === === debian10 (32 bit report) === === debian10 (32 bit French report) === === debian10 (32 bit Japanese:Japan report) === === debian10 (32 bit Chinese:China report) === === debian10 (32 bit WoW report) === === debian10 (64 bit WoW report) ===
Hi, While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=65026 Your paranoid android. === w1064v1809 (32 bit report) === === w1064v1809_2scr (32 bit report) === === w1064v1809_ar (32 bit report) === === w1064v1809_he (32 bit report) === === w1064v1809_ja (32 bit report) === === w1064v1809_zh_CN (32 bit report) === === w1064v1809 (64 bit report) === === debian10 (32 bit report) === === debian10 (32 bit French report) === === debian10 (32 bit Japanese:Japan report) === === debian10 (32 bit Chinese:China report) === === debian10 (32 bit WoW report) === === debian10 (64 bit WoW report) ===
On Wed, 12 Feb 2020 at 18:24, Paul Gofman <gofmanp(a)gmail.com> wrote:
@@ -5759,12 +5772,23 @@ HRESULT wined3d_device_init(struct wined3d_device *device, struct wined3d *wined struct wined3d_adapter *adapter = wined3d->adapters[adapter_idx]; const struct wined3d_fragment_pipe_ops *fragment_pipeline; const struct wined3d_vertex_pipe_ops *vertex_pipeline; + struct wined3d_blend_state_desc blend_state_desc; unsigned int i; HRESULT hr;
if (!wined3d_select_feature_level(adapter, levels, level_count, &device->feature_level)) return E_FAIL;
+ memset(&blend_state_desc, 0, sizeof(blend_state_desc)); + blend_state_desc.alpha_to_coverage = TRUE; + + if (FAILED(hr = wined3d_blend_state_create(device, &blend_state_desc, + NULL, &wined3d_null_parent_ops, &device->blend_state_atoc_enabled))) + { + ERR("Could not create blend state object.\n"); + return hr; + } + TRACE("Device feature level %s.\n", wined3d_debug_feature_level(device->feature_level));
device->ref = 1; This leaks the blend state object on error paths.
You also don't reference count the blend state pointer in the wined3d_stateblock_state structure. That's not strictly required because the lifetime of the "blend_state_atoc_enabled" object is otherwise managed by the device, but it's also inconsistent with other objects in the wined3d_stateblock_state structure, which makes it a little ugly.
participants (3)
-
Henri Verbeet -
Marvin -
Paul Gofman