https://bugs.winehq.org/show_bug.cgi?id=44410
Bug ID: 44410 Summary: UltraISO setup uses wrong char widths in path edit box Product: Wine Version: unspecified Hardware: x86-64 OS: Linux Status: UNCONFIRMED Severity: normal Priority: P2 Component: usp10 Assignee: wine-bugs@winehq.org Reporter: thomas.faber@reactos.org Regression SHA1: fb44feb62fd8b49edd27921bc6bc3682d14c1254 Distribution: ---
Created attachment 60335 --> https://bugs.winehq.org/attachment.cgi?id=60335 Screenshot
The path edit box in UltraISO's setup shows a broken font display since fb44feb62fd (usp10: Re-use script caches for the same font). It's most clearly visible with the first backslash, which has too much white space on its left (see screenshot).
I've tracked it down to the fact that calls to ScriptPlaceOpenType are made with psa->fNoGlyphIndex values of both TRUE and FALSE, leading to the widths[] array being populated with both character and glyph indexes. Whichever is requested first will be written to the cache and then used for both. As far as I understand, this issue has existed before the guilty commit, it is just easier to hit now due to the cache re-use. It would seem a fix should either use two separate caches (one indexed by character, one by glyph), or consistently use one of the two as the index to widths[], and possibly keep a char<->glyph lookup table in addition.
Related ReactOS bug: https://jira.reactos.org/browse/CORE-14226
https://bugs.winehq.org/show_bug.cgi?id=44410
--- Comment #1 from Thomas Faber thomas.faber@reactos.org --- Created attachment 60336 --> https://bugs.winehq.org/attachment.cgi?id=60336 Hacky patch
Attached patch is a quick hack that makes the issue go away. This is suboptimal because we now duplicate caching for chars vs glyphs. Instead, the already existing glyphs cache (ScriptCache::page) should be used to look up the correct index into a singular widths cache if necessary.
https://bugs.winehq.org/show_bug.cgi?id=44410
Fabian Maurer dark.shadow4@web.de changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |dark.shadow4@web.de
--- Comment #2 from Fabian Maurer dark.shadow4@web.de --- Can you provide a download link?
https://bugs.winehq.org/show_bug.cgi?id=44410
Thomas Faber thomas.faber@reactos.org changed:
What |Removed |Added ---------------------------------------------------------------------------- URL| |http://dw.ezbsys.net/uiso9_ | |pe.exe
--- Comment #3 from Thomas Faber thomas.faber@reactos.org --- Oops, yeah I missed that. Added, thanks.
https://bugs.winehq.org/show_bug.cgi?id=44410
Fabian Maurer dark.shadow4@web.de changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|UNCONFIRMED |NEW Ever confirmed|0 |1 Keywords| |download, patch
--- Comment #4 from Fabian Maurer dark.shadow4@web.de --- Thanks, confirming. Do you plan on fixing that issue yourself?
https://bugs.winehq.org/show_bug.cgi?id=44410
--- Comment #5 from Thomas Faber thomas.faber@reactos.org --- Yeah I'm looking at that. The fix itself should be simple, but it'll require a bunch of char vs glyph tests that I'll need to spend some time on.
https://bugs.winehq.org/show_bug.cgi?id=44410
Thomas Faber thomas.faber@reactos.org changed:
What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.winehq.org/sho | |w_bug.cgi?id=44558
https://bugs.winehq.org/show_bug.cgi?id=44410
Gijs Vermeulen gijsvrm@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Keywords| |regression
https://bugs.winehq.org/show_bug.cgi?id=44410
Gijs Vermeulen gijsvrm@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |andrey.goosev@gmail.com
--- Comment #6 from Gijs Vermeulen gijsvrm@gmail.com --- *** Bug 44558 has been marked as a duplicate of this bug. ***
https://bugs.winehq.org/show_bug.cgi?id=44410
Gijs Vermeulen gijsvrm@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Summary|UltraISO setup uses wrong |Multiple setups use wrong |char widths in path edit |char widths in path edit |box |box (UltraISO, some GOG | |installers)
https://bugs.winehq.org/show_bug.cgi?id=44410
Gijs Vermeulen gijsvrm@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |gyebro69@gmail.com See Also|https://bugs.winehq.org/sho | |w_bug.cgi?id=44558 |
https://bugs.winehq.org/show_bug.cgi?id=44410
zzzzzyzz@hacari.org changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |zzzzzyzz@hacari.org
https://bugs.winehq.org/show_bug.cgi?id=44410
--- Comment #7 from Nikolay Sivov bunglehead@gmail.com --- (In reply to Thomas Faber from comment #5)
Yeah I'm looking at that. The fix itself should be simple, but it'll require a bunch of char vs glyph tests that I'll need to spend some time on.
Any news on that? I think we should either skip cache on fNoGlyphIndex, or do GetGlyphIndicesW() on every call with fNoGlyphIndex.
How did you plan to test this? Let me know if you don't have time for this, it's something we need to get fixed for 4.0 I think.
https://bugs.winehq.org/show_bug.cgi?id=44410
Nikolay Sivov bunglehead@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Version|unspecified |3.1
https://bugs.winehq.org/show_bug.cgi?id=44410
--- Comment #8 from Thomas Faber thomas.faber@reactos.org --- Created attachment 62997 --> https://bugs.winehq.org/attachment.cgi?id=62997 Patch with some tests and related fixes
Yeah, as you may have guessed I got sidetracked, and currently don't have time for this.
My idea of the correct fix was to simply use get_cache_glyph when appropriate (either in get_cache_glyph_widths or its callers), i.e. make the cache store glyph widths, and translate from chars where needed.
My approach to testing this was to try to validate Wine's implementation by comparing the results from ScriptGetGlyphABCWidth with GetCharABCWidthsW/GetCharABCWidthsI, as in the patch I'm attaching. I seem to recall that the test as-is already showed a difference between the broken and fixed width caches, but I haven't reduced it down to make a concise test case yet. There's also a lot of variance introduced by choice of font, and I'm not sure I was done playing with that.
Hope that helps. Thanks.
https://bugs.winehq.org/show_bug.cgi?id=44410
Nikolay Sivov bunglehead@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Resolution|--- |FIXED Fixed by SHA1| |8d018d8d1dadf9d731690567b9c | |98d5a9eae37db Status|NEW |RESOLVED
--- Comment #9 from Nikolay Sivov bunglehead@gmail.com --- Marking fixed, 8d018d8d1dadf9d731690567b9c98d5a9eae37db.
https://bugs.winehq.org/show_bug.cgi?id=44410
Alexandre Julliard julliard@winehq.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|RESOLVED |CLOSED
--- Comment #10 from Alexandre Julliard julliard@winehq.org --- Closing bugs fixed in 4.0-rc3.
https://bugs.winehq.org/show_bug.cgi?id=44410
Jactry Zeng jactry92@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.winehq.org/sho | |w_bug.cgi?id=46678