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

[PATCH 0/13] MR3420: ddraw: Implement Pick() and GetPickRecords().
by Jeffrey Smith (＠whydoubt) 07 Aug '23

07 Aug '23

- The initial development was done by Matthew Wong in 2019. I split his v2 patch into testing and implementation commits, making changes needed to get it to compile. - Some of the feedback given on the v2 patch was addressed by Myah Caron's updated patch versions in 2020, which I split out into a few separate commits. - I also addressed some of the feedback, expanded the tests, and fixed some results based on those tests. _(Following is the commit message from Matthew Wong's v2 patch)_ Implement functions used by some games (notably LEGO Island) for determining which 3D object in a scene was clicked by the mouse cursor. Fighting Steel also uses this function for mouse over. Previous stubs would cause LEGO Island to crash upon any click and Fighting Steel to crash on game start. A patch posted years ago on the bug thread provided the minimum functionality to prevent crashes, but still rendered large portions of the game inaccessible without them implemented correctly. Picking has been implemented by adding a "pick mode" in d3d_execute_buffer_execute() which skips any drawing functions leaving just the vertex processing. Adds click tests for each triangle when in pick mode for creating an array of D3DPICKRECORDs. Add a D3DPICKRECORD array and DWORD counter to d3d_device. These are initiated in d3d_device_init(), allocated/written in d3d_execute_buffer_execute(), and accessed/read in d3d_device1_GetPickRecords(). The counter is used to determine the array size (0 meaning array is not allocated). The array is free'd whenever the data is no longer necessary by d3d_execute_buffer_execute(), d3d_device1_GetPickRecords(), and d3d_device_inner_Release(). Add a compliance test to ddraw1 to test whether certain screen points result in successful picks or not, as well as whether the data returned from GetPickRecords() is valid and correct. Stress testing reveals this patch's Pick() implementation may have slight inaccuracies to the original function; occasionally pixels right on triangle edges result in successful picks when they don't with the original function (and vice versa). It may be some sort of floating point rounding error or other algorithm difference that would be difficult to determine without seeing the original code. In practice, I believe this inaccuracy is so negligible that it won't produce any undesirable results for the user. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=10729 -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3420

6 18

[PATCH v2 0/14] MR3303: Draft: winegstreamer: Implement wg_muxer
by Ziqing Hui (＠zhui) 07 Aug '23

07 Aug '23

Still working in progress. -- v2: temp winegstreamer: Implement wg_muxer. winegstreamer: Introduce array_reserve to unix lib. winegstreamer: Add mpeg4 sink class factory. winegstreamer: Add enum wg_container_type. winegstreamer: Make find_element accept NULL caps. winegstreamer: Add async command handling to media sink. winegstreamer: Add IMFClockStateSink stubs for media sink. winegstreamer: Implement IMFMediaEventGenerator for media sink. winegstreamer: Add stubs for stream sink. winegstreamer: Add stubs for media sink. mf/tests: Test IMFClockStateSink in shutdown state. mf/tests: Test sample processing for MPEG4 media sink. mf/tests: Use IMFMediaEventGenerator interface in event wait helper. https://gitlab.winehq.org/wine/wine/-/merge_requests/3303

3 15

[PATCH v4 0/6] MR3454: user32: Implement UOI_TIMERPROC_EXCEPTION_SUPPRESSION.
by Jinoh Kang (＠iamahuman) 06 Aug '23

06 Aug '23

This is required to avoid silencing (potentially fatal) exceptions from timer procedures. -- v4: win32u: Ignore unhandled info index in NtUserSetObjectInformation. win32u/tests: Add tests for NtUserSetObjectInformation. user32: Implement UOI_TIMERPROC_EXCEPTION_SUPPRESSION. user32/tests: Add tests for UOI_TIMERPROC_EXCEPTION_SUPPRESSION. include: Add definition for UOI_TIMERPROC_EXCEPTION_SUPPRESSION user32/tests: Make test_unicode_wm_char robust against superfluous messages. https://gitlab.winehq.org/wine/wine/-/merge_requests/3454

3 8

Re: [PATCH v2 0/1] MR3447: ntdll/unix: NtTerminateThread should accept NULL handle
by Jinoh Kang (＠iamahuman) 06 Aug '23

06 Aug '23

IMHO this should be made consistent with how `NtTerminateProcess()` handles NULL process handle. There's no special logic for NULL handles in the client[^wineserver] side of `NtTerminateProcess()`:[^NtTerminateProcess] ```c /****************************************************************************** * NtTerminateProcess (NTDLL.@) */ NTSTATUS WINAPI NtTerminateProcess( HANDLE handle, LONG exit_code ) { unsigned int ret; BOOL self; SERVER_START_REQ( terminate_process ) { req->handle = wine_server_obj_handle( handle ); /* <-- no NULL check here */ req->exit_code = exit_code; ret = wine_server_call( req ); self = reply->self; } SERVER_END_REQ; (...snip...) return ret; } ``` Instead, `NtTerminateProcess()` passes the handle to the wineserver[^wineserver], which is actually responsible for interpreting the process handle (and thus checking for NULL handle as well):[^terminate_process] ```c /* terminate a process */ DECL_HANDLER(terminate_process) { struct process *process; if (req->handle) /* <-- NULL check here */ { process = get_process_from_handle( req->handle, PROCESS_TERMINATE ); if (!process) return; } else process = (struct process *)grab_object( current->process ); reply->self = (current->process == process); terminate_process( process, current, req->exit_code ); release_object( process ); } ``` The `terminate_thread` handle should be made to accept NULL handle as well. [^wineserver]: Any Wine app process is a client of the *wineserver*, which emulates the core "kernel" part of the Windows. [^NtTerminateProcess]: https://gitlab.winehq.org/wine/wine/-/blob/f1749b0808c66a0341b77409cc9338ab… [^terminate_process]: https://gitlab.winehq.org/wine/wine/-/blob/f1749b0808c66a0341b77409cc9338ab… -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3447#note_41447

1 0

[PATCH 0/4] MR3517: Fixes for GDI/GDI+ newline measurements.
by Torge Matthies (＠tmatthies) 05 Aug '23

05 Aug '23

-- https://gitlab.winehq.org/wine/wine/-/merge_requests/3517

3 5

Re: [PATCH v3 0/1] MR3491: winegstreamer: Leave media source critical section before unlocking workqueue.
by Zebediah Figura (＠zfigura) 04 Aug '23

04 Aug '23

> Nothing can call PutWorkItem or use the queue after Shutdown has been called. True, although that fact is a little obscure (especially since MFPutWorkItem() is called from a callback. That said, my concern at this point is less about correctness than idiomaticity. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3491#note_41434

1 0

Re: [PATCH v3 0/1] MR3491: winegstreamer: Leave media source critical section before unlocking workqueue.
by Rémi Bernon 04 Aug '23

04 Aug '23

On Fri Aug 4 21:02:44 2023 +0000, Zebediah Figura wrote: > > We could maybe grab the Queue id in the cs, 0 it, and use the local > copy for unlocking. Does that sound better? > That does seem like an improvement; I can't say I see a better way to > solve the problem after looking, at least. We should probably mention in > a comment why we're releasing outside of the lock; it is not obvious at > all from the documentation that MFUnlockWorkQueue() can block. Nothing can call PutWorkItem or use the queue after Shutdown has been called. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3491#note_41432

1 0

[PATCH v2 0/3] MR3428: Avoid relying on linux/ipx.h to define SOL_IPX.
by Jinoh Kang (＠iamahuman) 04 Aug '23

04 Aug '23

Today, Wine fails to build with musl libc and Linux kernel 5.14 uAPI header files. musl libc doesn't supply any definitions for IPX, such as the SOL_IPX macro. However, it still provides linux/ipx.h from Linux uAPI header files if it exists. Linux kernel wouldn't drop linux/ipx.h from uAPI headers until 5.15, although IPX support has already been marked obsolete since 2018. Fix this by not defining HAS_IPX if linux/ipx.h has been included but nothing defines the SOL_IPX macro. Status of IPX support from other libcs are noted below: - bionic: netipx/ipx.h does not exist. linux/ipx.h may or may not exist. Note that sys/socket.h defines SOL_IPX even if linux/ipx.h is missing. - glibc: netipx/ipx.h exists. In this case, Wine assumes IPX support even if the operating system does not support it in runtime. - BSD variants: netipx/ipx.h may or may not exist. linux/ipx.h does not exist. Some BSDs supply SO_DEFAULT_HEADERS instead of SOL_IPX. -- v2: server: Avoid relying on linux/ipx.h to define SOL_IPX. ws2_32: Avoid relying on linux/ipx.h to define SOL_IPX. ntdll: Avoid relying on linux/ipx.h to define SOL_IPX. https://gitlab.winehq.org/wine/wine/-/merge_requests/3428

4 5

Re: [PATCH v3 0/1] MR3491: winegstreamer: Leave media source critical section before unlocking workqueue.
by Zebediah Figura (＠zfigura) 04 Aug '23

04 Aug '23

> We could maybe grab the Queue id in the cs, 0 it, and use the local copy for unlocking. Does that sound better? That does seem like an improvement; I can't say I see a better way to solve the problem after looking, at least. We should probably mention in a comment why we're releasing outside of the lock; it is not obvious at all from the documentation that MFUnlockWorkQueue() can block. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3491#note_41430

1 0

Re: [PATCH v3 0/1] MR3491: winegstreamer: Leave media source critical section before unlocking workqueue.
by Bernhard Kölbl 04 Aug '23

04 Aug '23

On Fri Aug 4 20:16:23 2023 +0000, Zebediah Figura wrote: > > It doesn't matter much if the handle is invalid, as the PutWorkItem > call will just fail. > But the handle could in theory be reallocated for a different queue. > Also, if queues are supposed to be refcounted, it looks quite wrong that > a thread which doesn't have a reference to a handle is accessing it; > that's the sort of thing that generally causes memory errors. > It might be the case that our current implementation is preventing any > of this from being a problem in practice, but it's fragile and > unidiomatic, and it is rather confusing to anyone reading the code; it > looks like an error. We could maybe grab the Queue id in the cs, 0 it, and use the local copy for unlocking. Does that sound better? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3491#note_41426

1 0