June 2024 - Wine-GitLab - list.winehq.org

Re: [PATCH v8 0/11] MR5444: jscript: Do not allocate dispex_prop_t for indexed props.
by Gabriel Ivăncescu June 6, 2024

June 6, 2024

On Thu Jun 6 19:05:56 2024 +0000, Jacek Caban wrote: > > In such case, instead of failing, it would call the other function > that was looked up, since it matches in IID and DISPID. It won't crash, > but I don't think it would be quite correct, either. > That's the whole point, the reason it's safe and I don't see why you > think it's incorrect. BTW, IID is intentionally abstract in my proposal; > a bit closer to the actual meaning would be to use something like > prototype ID once we have those (interface ID is just already available > and close enough). > > Continuing the above, IMO the following would make it the most robust: > we look it up as mentioned from the func_info_t's dispid and IID, and > then compare to see if it's the same `func_info_t` pointer. > Sure, but it can't compare `func_info_t`s by a pointer, it would need to > compare IID or something like that anyway. So in your proposal, the > opaque pointer serves as (ID,IID) carrier, nothing else. > > BTW, is there a reason you renamed `IWineJSDispatchHost` to just > `IJSDispatchHost` when it's Wine's own interface? I thought it's better > to avoid ambiguity with a native interface, especially if we're to > extend it (obviously we can't touch native interfaces), makes it a lot > more obvious. > It's all Wine code, I don't see how putting "wine" in random places > makes it any cleaner, but it's not a strong preference and I won't > insist either way. Well, I expect them to behave like JS functions. If it were a JS function, it would check to see if the object type matches, and then do its thing. If you think of it like this, it wouldn't make much sense for it to use another object type's function. As for Wine prefix, we typically have that already for our own custom interfaces in mshtml, and even though it's all Wine code, it makes it clearer for someone unfamiliar to understand that the interface is not something from Windows—so they can find its documentation/code in Wine code with grep instead of a Google search, and can freely change them around, etc. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5444#note_72606

1 0

Re: [PATCH v8 0/11] MR5444: jscript: Do not allocate dispex_prop_t for indexed props.
by Jacek Caban (＠jacek) June 6, 2024

June 6, 2024

> In such case, instead of failing, it would call the other function that was looked up, since it matches in IID and DISPID. It won't crash, but I don't think it would be quite correct, either. That's the whole point, the reason it's safe and I don't see why you think it's incorrect. BTW, IID is intentionally abstract in my proposal; a bit closer to the actual meaning would be to use something like prototype ID once we have those (interface ID is just already available and close enough). > Continuing the above, IMO the following would make it the most robust: we look it up as mentioned from the func_info_t's dispid and IID, and then compare to see if it's the same `func_info_t` pointer. Sure, but it can't compare `func_info_t`s by a pointer, it would need to compare IID or something like that anyway. So in your proposal, the opaque pointer serves as (ID,IID) carrier, nothing else. > BTW, is there a reason you renamed `IWineJSDispatchHost` to just `IJSDispatchHost` when it's Wine's own interface? I thought it's better to avoid ambiguity with a native interface, especially if we're to extend it (obviously we can't touch native interfaces), makes it a lot more obvious. It's all Wine code, I don't see how putting "wine" in random places makes it any cleaner, but it's not a strong preference and I won't insist either way. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5444#note_72605

1 0

[PATCH v2 0/12] MR5796: kernelbase, ntdll: Implement DOS wildcards matching in NtQueryDirectoryFile and use that in FindFirstFile().
by Paul Gofman (＠gofman) June 6, 2024

June 6, 2024

-- v2: kernelbase: Preprocess wildcarded mask and pass it with NtQueryDirectoryFile(). https://gitlab.winehq.org/wine/wine/-/merge_requests/5796

3 14

[PATCH v36 0/2] MR5752: msvcrt: [PR53960] Fix ucrt has different struct layout than msvcrt
by cqwrteur (＠trcrsired) June 6, 2024

June 6, 2024

UCRT's FILE struct and fd struct have different layouts, and their locks are different. Implementation in Wine does not match UCRT in Windows, causing ABI issues. see: https://bugs.winehq.org/show_bug.cgi?id=53960 -- v36: ucrtbase: Fix incorrect layout of FILE. ucrtbase: Add FILE structure tests. https://gitlab.winehq.org/wine/wine/-/merge_requests/5752

3 3

Re: [PATCH v8 0/11] MR5444: jscript: Do not allocate dispex_prop_t for indexed props.
by Gabriel Ivăncescu June 6, 2024

June 6, 2024

On Thu Jun 6 15:55:43 2024 +0000, Gabriel Ivăncescu wrote: > Actually you don't even need to query IID anymore. Just look up the > function by dispid, and then check if it's same func_info_t, that's enough. BTW, is there a reason you renamed `IWineJSDispatchHost` to just `IJSDispatchHost` when it's Wine's own interface? I thought it's better to avoid ambiguity with a native interface, especially if we're to extend it (obviously we can't touch native interfaces), makes it a lot more obvious. And to add to what I said above, what I meant with "retrieving a potentially different function" is mostly theoretical, because as you said current mshtml likely has only one struct layout per interface. **But** if we were to assume this is not the case (addressing the original concern), then two objects could have same IID but different struct layouts and different functions. In such case, instead of failing, it would call the other function that was looked up, since it matches in IID and DISPID. It won't crash, but I don't think it would be quite correct, either. And it still makes the code simpler anyway. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5444#note_72601

1 0

[PATCH v3 0/7] MR5732: winegstreamer: Get rid of the wg_transform internal output_format.
by Rémi Bernon June 6, 2024

June 6, 2024

-- v3: winegstreamer: Request the new transform output format explicitly. winegstreamer: Use a caps to store the desired output format. winegstreamer: Only report format changes when frontend supports it. winegstreamer: Rename allow_size_change to allow_format_change. winegstreamer: Enforce default stride presence in the video processor. winegstreamer: Enforce default stride value in the video decoder. winegstreamer: Allow to clear video decoder input/output types. https://gitlab.winehq.org/wine/wine/-/merge_requests/5732

3 10

[PATCH v3 0/7] MR5717: Improve dmo_wrapper_sink_Receive.
by Ziqing Hui (＠zhui) June 6, 2024

June 6, 2024

This reduce the chance of buffer allocating issue for Kirikiri2 games(eg. Super Naughty Maid) when the game is trying to play videos. -- v3: qasf/dmowrapper: Sync Stop() and Receive() for dmo wrapper filter. qasf/dmowrapper: Return VFW_E_WRONG_STATE in dmo_wrapper_sink_Receive. qasf/dmowrapper: Allocate output samples before calling ProcessInput(). qasf/dmowrapper: Return failure in dmo_wrapper_sink_Receive if samples allocating fails. qasf/dmowrapper: Introduce release_output_samples. qasf/dmowrapper: Introduce get_output_samples. qasf/tests: Add more tests for dmo_wrapper_sink_Receive. https://gitlab.winehq.org/wine/wine/-/merge_requests/5717

3 8

Re: [PATCH v8 0/11] MR5444: jscript: Do not allocate dispex_prop_t for indexed props.
by Gabriel Ivăncescu June 6, 2024

June 6, 2024

On Thu Jun 6 15:51:53 2024 +0000, Gabriel Ivăncescu wrote: > > But then you assume that all objects implementing the interface will > use the same struct layout for that. I can't think of an example inside > MSHTML where it's currently not the case, but that's not generally a > promise that QI gives. > In theory, there's a similar concern when looking up the function via > IID and DISPID, specifically, that it might find a different function. > In practice as you said it probably doesn't happen but… > > So what's the point of it then? Avoiding one argument in a call? > Continuing the above, IMO the following would make it the most robust: > we look it up as mentioned from the func_info_t's dispid and IID, and > then compare to see if it's the same `func_info_t` pointer. If not, > return E_UNEXPECTED since it's the wrong function. And yes, one less > argument to keep track of, but also one less thing to keep track of (in > the ProxyFunction for instance, and everywhere else). > > I think that you misinterprets your observations. Here is a test > proving that currentStyle should have a setter that throws an error > (unless used for modifiable style object): > Okay, thanks for the test, I guess there's more at play here indeed. Actually you don't even need to query IID anymore. Just look up the function by dispid, and then check if it's same func_info_t, that's enough. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5444#note_72595

1 0

[PATCH v3 0/3] MR5799: winex11: Fix some incorrect usage of NtGdiDdDDICreateDCFromMemory.
by Rémi Bernon June 6, 2024

June 6, 2024

Didn't notice that it was failing until I did the same for winemac with https://gitlab.winehq.org/wine/wine/-/merge_requests/5798. It was silently falling back to copies while winemac can't do that. -- v3: gdi.exe16: Fix some incorrect usage of NtGdiDdDDICreateDCFromMemory. winex11: Fix some incorrect usage of NtGdiDdDDICreateDCFromMemory. winex11: Wrap more window surface formats with NtGdiDdDDICreateDCFromMemory. https://gitlab.winehq.org/wine/wine/-/merge_requests/5799

1 3

Re: [PATCH v8 0/11] MR5444: jscript: Do not allocate dispex_prop_t for indexed props.
by Gabriel Ivăncescu June 6, 2024

June 6, 2024

> But then you assume that all objects implementing the interface will use the same struct layout for that. I can't think of an example inside MSHTML where it's currently not the case, but that's not generally a promise that QI gives. In theory, there's a similar concern when looking up the function via IID and DISPID, specifically, that it might find a different function. In practice as you said it probably doesn't happen but… > So what's the point of it then? Avoiding one argument in a call? Continuing the above, IMO the following would make it the most robust: we look it up as mentioned from the func_info_t's dispid and IID, and then compare to see if it's the same `func_info_t` pointer. If not, return E_UNEXPECTED since it's the wrong function. And yes, one less argument to keep track of, but also one less thing to keep track of (in the ProxyFunction for instance, and everywhere else). > I think that you misinterprets your observations. Here is a test proving that currentStyle should have a setter that throws an error (unless used for modifiable style object): Okay, thanks for the test, I guess there's more at play here indeed. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5444#note_72591

1 0