The IVideoWindow_put_Visible runs the NtUserShowWindow which may trigger the VideoWindow_NotifyOwnerMessage with a WM_PAINT message and afterwards try to send a WM_ERASEBKND. This will cause the VideoWindow_NotifyOwnerMessage to be stuck waiting on the lock. This causes a deadlock because the IVideoWindow_put_Visible is never able to return, as it is waiting for the WM_ERASEBKND message to go through in the send_erase function, which can only go through once the WM_PAINT has returned.
--
https://gitlab.winehq.org/wine/wine/-/merge_requests/5023
Theoretically, this allows debuggers to tell them apart. https://devblogs.microsoft.com/oldnewthing/20110429-00/?p=10803
But in practice, this is more about cleanliness than anything else; I'm not aware of anyone running SetLastError-aware debuggers in Wine. Feel free to reject if this is a waste of time.
The changeable SetLastError calls were found by this script:
```python
#!/usr/bin/env python3
import os
import re
for (dirpath, dirnames, filenames) in os.walk("."):
for fn in filenames:
if not fn.endswith(".c") and not fn.endswith(".h"):
continue
text = open(dirpath+"/"+fn,"rt").read()
text = text.replace("RtlSetLastWin32Error","SetLastError")
text = text.replace("RtlGetLastWin32Error","GetLastError")
if "SetLastError" not in text:
continue
varname = None
for line in text.split("\n"):
if "GetLastError" in line:
if m := re.search(r"([A-Za-z0-9_]+) *= *GetLastError", line):
prevline = line
varname = m[1]
if line == "}":
varname = None
if varname is not None and "SetLastError" in line:
if m := re.search(r"SetLastError\( *([A-Za-z0-9_]+) *\)", line):
if m[1] == varname:
print(dirpath+"/"+fn, prevline, line)
```
The script also flags https://gitlab.winehq.org/wine/wine/-/blob/master/dlls/mpr/wnet.c#L2811, which is half set and half restore; I chose to left it unchanged.
Idea from https://gitlab.winehq.org/wine/wine/-/merge_requests/5020/diffs#6c845445f68…
--
v3: win32u: Use RtlRestoreLastWin32Error instead of RtlSetLastWin32Error if appropriate
everywhere: Change SetLastError to RestoreLastError if appropriate
https://gitlab.winehq.org/wine/wine/-/merge_requests/5035
Normally I am a strong proponent of adjusting implementations one small step at
a time, but this is a bit of an extreme case.
The current state of UrlCanonicalize() is a prototypical example of "spaghetti
code": the implementation has been repeatedly adjusted to fix a new discovered
edge case, without properly testing the scope of the new logic, with the effect
that the current logic is a scattered mess of conditions that interact in
unpredictable ways.
To be fair, the actual function is much more complicated than one would
anticipate, and the number of new weird edge cases I found while trying to flesh
out existing logic was rather exhausting.
I initially tried to "fix" the existing implementation one step at a time. After
racking up dozens of commits without an end in sight, though, and having to
waste time carefully unravelling the broken code in the right order so as to
avoid temporary failures, I decided instead to just fix everything at once,
effectively rewriting the function from scratch, and this proved to be much more
productive.
Hopefully the huge swath of new tests is enough to prove that this new
implementation really is correct, and has no more spaghetti than what Microsoft
made necessary.
--
v4: shlwapi/tests: Add many more tests for UrlCanonicalize().
kernelbase: Use scheme_is_opaque() in UrlIs().
kernelbase: Reimplement UrlCanonicalize().
kernelbase: Do not canonicalize the relative part in UrlCombine().
kernelbase: Do not use isalnum() with Unicode characters.
https://gitlab.winehq.org/wine/wine/-/merge_requests/4954
This MR implements `NtContinueEx(PCONTEXT, PCONTINUE_OPTIONS)` which was added in Windows 10 20H1.
Also added basic test reusing existing `test_continue()` (included from !4720)
League of Legends game hooks `NtContinue()` and `NtContinueEx()` by putting `jmp` instruction in front of it.
Note that LoL doesn't actually use `NtContinueEx()` itself and game will work fine without it but if game doesn't find `NtContinueEx()` it will permanently ban your account due to detecting "scripting program"
--
v5: ntdll/tests: Implement APC test for NtContinue()
ntdll/tests: Add basic test for NtContinueEx()
ntdll: Implement NtContinueEx()
ntdll/tests: Unify APC test functions
https://gitlab.winehq.org/wine/wine/-/merge_requests/4761
On Tue Feb 6 18:52:35 2024 +0000, Esme Povirk wrote:
> No, that doesn't seem right either, I need to think about this.
Thought about it, I do think it should be ||. We should break if all of the text has been measured, and we should break if the current section doesn't fully fit in to_measure_length.
--
https://gitlab.winehq.org/wine/wine/-/merge_requests/4082#note_60325
On Tue Feb 6 15:01:50 2024 +0000, Santino Mazza wrote:
> I tested GdipMeasureString with different strings that require font
> linking and it behaves correctly. It's strange because I also tested
> with the exact same string the game uses and with the generic
> typographic format, but I still can't get the bug to reproduce.
> Apparently when the game tries to draw that specific string,
> GdipMeasureString returns a bounding box with width 0, which causes
> draw_string_callback to not increase the X position, so it overwrites
> the other word. Again this doesn't happen if I use font_link_get_text_extent_point.
The difference might have to do with scaling? It looks like GdipMeasureString scales the size, and I don't think font_link_get_text_extent_point would.
Come to think of it, it's probably necessary to account for text rotation when in draw_string_callback (an arbitrary affine transform can be applied to the text, although I think at the moment our handling of anything beyond rotation is a bit broken). I think it might be a good idea to add output parameters for dx and dy to draw_driver_string.
--
https://gitlab.winehq.org/wine/wine/-/merge_requests/4082#note_60317