April 2023 - Wine-gitlab - list.winehq.org

Re: [PATCH v3 0/1] MR499: server: Allow cancelling alerted asyncs.
by Jinoh Kang (＠iamahuman) 29 Apr '23

29 Apr '23

On Tue Apr 18 19:25:54 2023 +0000, Zebediah Figura wrote: > > I don't think so. `async_terminate()` is currently no-op if the async > has already been terminated with a status other than `STATUS_ALERTED`, > and the patch maintains this invariant. > Right, that's the bit I failed to think through. > > It is possible to introduce a new boolean flag, but the flag cannot > replace `terminate_status`. `terminate_status` can be any arbitrary > value passed to the third parameter of `async_set_timeout`. > Ah, and I missed that that was a possibility, although, in my defense, > it's not stated in the patch subject either. I.e. this patch doesn't > just fix the race where an alerted async is canceled, it also fixes the > similar race where it times out. > So yes, I think the patch makes sense as is. Maybe a comment to yet > again overdocument asyncs wouldn't hurt? I easily see myself looking at > code like this and having to ask myself, "when exactly is > terminate_status set?" and the answer is "if (and only if?) the async is > canceled or times out while it's alerted", which is nice to have > documented and not have to puzzle through later. To reduce complexity, I've tried refactoring the state management. Currently, the async has the following two state flags that deal with the state of the client that originally requested the async: - `signaled`: indicates whether the async object is signaled; the client may wait on the async handle before returning from the corresponding wine system call. - Valid transition: 0 -> 1. - This flag is set if only if any of the following conditions are true: - Given that `!async->blocking`, - `async_wake_obj(async)` has been called when `async->unknown_status = 0`, or - `async_handoff(async, ...)` has been called with `get_error() != STATUS_ALERTED && (async->pending || !NT_ERROR(get_error()) && (get_error() != STATUS_PENDING || async->iosb->status != STATUS_PENDING)`. - (Regardless of `async->blocking`) `async_set_result(async, status, ...)` has been called with `async->terminated && (!async->alerted || status != STATUS_PENDING)`. - Note that this flag exhibits slightly different behavior depending on `async->blocking`. - If `async->blocking`, it tries to block the requestor until the async operation has completed. - If `!async->blocking`, it tries to signal as soon as the initial status is known, _unless_ the wait handle is being closed, in which case it doesn't bother to touch the flag at all. - If signaled and a wait on the async object is satisfied, the requestor function should have performed the synchronous completion sequence (either directly or via APC\_ASYNC\_IO) before the wait is satisfied, unless `async->pending`. - `direct_result`: from Wine codebase, `a flag if we're passing result directly from request instead of APC`. - Valid transition: 1 -> 0 (after set to 1 from `create_request_async`). - Whenever async state is settled, the flag is set if all of the following conditions are true: - The async was created via create_request_async(), and - async_set_unknown_status() has not been called, and - The async handle has never been waited on (wait consumes the flag), and - Any of: - async_handoff() has not been called, or - From async_handoff(), the async is terminated or alerted synchronously. - The flag has the following effect: - For each async_terminate() call, it will send APC_ASYNC_IO if and only if !direct_result. - For each async handle wait satisfaction, it will call async_set_result() if and only if direct_result. We can observe the following from above: - The flags are defined in terms of what the flags *do* rather than what the value of the flags *mean*. It doesn't help that the transitions for each flag are quite scattered and sparse. - Their behaviors are dependent on the client-side async implementation, so reasoning about them requires verifying how the requestor side behaves. - Depending on other part of the async object state, one or more flags may be left unobserved and thus ignored, effectivly becoming a "don't care" bit. This increases complexity and hinders maintenance in my opinion. In particular, it's difficult to determine precisely *when* the flag is or should be set or unset. I'd propose that the constituent conditions of each flag be refactored into separate state variables first. Paradoxically, it may even help combine two or more binary flags into an enum as the semantics become more simpler. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/499#note_31579

1 0

[PATCH 0/1] MR2330: xmllite/writer: implement prefix autogeneration
by David Kahurani (＠kahurani) 29 Apr '23

29 Apr '23

Looks good to me but would love some opinion(s) Signed-off-by: David Kahurani <k.kahurani(a)gmail.com> -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2330

3 7

Re: [PATCH v14 0/4] MR2669: wine{alsa,coreaudio,oss,pulse}: Move AudioSessionControl into mmdevapi.
by Davide Beatrici (＠davidebeatrici) 29 Apr '23

29 Apr '23

Davide Beatrici (@davidebeatrici) commented about dlls/mmdevapi/session.c: > +#include <winternl.h> > + > +#include <wine/debug.h> > +#include <wine/unixlib.h> > + > +#include "mmdevdrv.h" > +#include "unixlib.h" > + > +#define NULL_PTR_ERR MAKE_HRESULT(SEVERITY_ERROR, FACILITY_WIN32, RPC_X_NULL_REF_POINTER) > + > +WINE_DEFAULT_DEBUG_CHANNEL(mmdevapi); > + > +extern void sessions_lock(void) DECLSPEC_HIDDEN; > +extern void sessions_unlock(void) DECLSPEC_HIDDEN; > + > +extern const IAudioClient3Vtbl AudioClient3_Vtbl; Unused. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2669#note_31574

1 0

[PATCH 0/1] MR189: tests: Add a test for arrays with an expression as the index.
by Ethan Lee (＠flibitijibibo) 29 Apr '23

29 Apr '23

See https://bugs.winehq.org/show_bug.cgi?id=54832 I'm starting to see this particular FIXME in quite a few games (Escape Goat 2, Murder Miners, and Little Racers STREET to name a few), and since I'm not sure how to fix this I figured I could at least provide a test for someone that knows more SM4 than me :P -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/189

2 1

Re: [PATCH v4 0/2] MR188: vkd3d-shader/tpf: Add support for emitting sample_l instructions
by Francisco Casas (＠fcasas) 28 Apr '23

28 Apr '23

I just realized one more thing. I think we are not properly following the specification for both sample_l and sample_b. In the **Restrictions** section of the documentation for sample_b it is mentioned that the last src register must use a single component selector if it is not a scalar immediate. This "select_component" is also ilustrated in the sample_l format: ``` sample_l[_aoffimmi(u,v,w)] dest[.mask], srcAddress[.swizzle], srcResource[.swizzle], srcSampler, srcLOD.select_component ``` Currently, we are not respecting that since sm4_src_from_node() sets ``` src->swizzle_type = VKD3D_SM4_SWIZZLE_VEC4; ``` by default. It should be VKD3D_SM4_SWIZZLE_SCALAR in this case. If I am not mistaken, a simple: ``` instr.srcs[3].swizzle_type = VKD3D_SM4_SWIZZLE_SCALAR; ``` after calling sm4_src_from_node() should do. The disassembly should show a single component in the swizzle of the last src register. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/188#note_31568

1 0

Re: [PATCH v4 0/2] MR188: vkd3d-shader/tpf: Add support for emitting sample_l instructions
by Ethan Lee (＠flibitijibibo) 28 Apr '23

28 Apr '23

On Fri Apr 28 21:50:24 2023 +0000, Francisco Casas wrote: > Currently, there doesn't exist a `[test todo]` Section for the > shader_runner. > This only seems to work (in this incomplete stage) because the > shader_runner is handling the section the same as the previous one, > `[pixel shader todo]`. > The proper way to write this part is: > ``` > [test] > todo draw quad > todo probe all rgba (0.25, 0, 0.25, 0) > ``` > and remove the `todo`s in the next commit. Done! -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/188#note_31566

1 0

[PATCH v3 0/2] MR188: vkd3d-shader/tpf: Add support for emitting sample_l instructions
by Ethan Lee (＠flibitijibibo) 28 Apr '23

28 Apr '23

>From what I can tell, the recent work on SampleBias/SampleLevel did all of the work for us, we just need to take the same framework and include the case for the `sample_l` instruction. Tested with some shaders from the native Linux version of Little Racers STREET. -- v3: vkd3d-shader/tpf: Add support for emitting sample_l instructions tests: Add a test for SampleLevel() function. https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/188

3 4

[PATCH v2 0/1] MR188: vkd3d-shader/tpf: Add support for emitting sample_l instructions
by Ethan Lee (＠flibitijibibo) 28 Apr '23

28 Apr '23

>From what I can tell, the recent work on SampleBias/SampleLevel did all of the work for us, we just need to take the same framework and include the case for the `sample_l` instruction. Tested with some shaders from the native Linux version of Little Racers STREET. -- v2: vkd3d-shader/tpf: Add support for emitting sample_l instructions https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/188

2 2

[PATCH 0/1] MR188: vkd3d-shader/tpf: Add support for emitting sample_l instructions
by Ethan Lee (＠flibitijibibo) 28 Apr '23

28 Apr '23

>From what I can tell, the recent work on SampleBias/SampleLevel did all of the work for us, we just need to take the same framework and include the case for the `sample_l` instruction. Tested with some shaders from the native Linux version of Little Racers STREET. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/188

2 1

[PATCH v6 0/2] MR184: vkd3d-shader/hlsl: Add support for SampleGrad() method
by Ethan Lee (＠flibitijibibo) 28 Apr '23

28 Apr '23

~~This one's marked as a draft, as there seems to be a blocker with the method parameters.~~ ~~The first commit totally works, _if_ the ddx/ddy parameters are literals - they do _not_ work when passing a variable of any kind. The test comes from tests/d3d12.c, so I'm mostly just trying to migrate that to the HLSL test suite, but it currently hits an assert before we get to the resource load (which does eventually work) and I'm not sure what's causing it:~~ ``` vkd3d-compiler: libs/vkd3d-shader/tpf.c:3190: sm4_register_from_node: Assertion `instr->reg.allocated' failed. ``` ~~Seems like it's surprised when we try to load from the constant buffer maybe?~~ Fixed! -- v6: tests: Add a basic compilation test for SampleGrad() method. vkd3d-shader/hlsl: Add support for SampleGrad() method https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/184

2 2