http://bugs.winehq.org/show_bug.cgi?id=30115
Bug #: 30115 Summary: Perfect World: Skills and items icons messed up (regression) Product: Wine Version: 1.3.26 Platform: x86 OS/Version: Linux Status: UNCONFIRMED Severity: normal Priority: P2 Component: directx-d3d AssignedTo: wine-bugs@winehq.org ReportedBy: mooroon2@mail.ru CC: hverbeet@gmail.com Classification: Unclassified Regression SHA1: a84275075d4a20a9a7f42eda01585d34f31b56d4
It is a well-known regression that had been reported ages ago to APPDB, but it seems that no one had took care to proceed with the regression testing so far. The problem manifests itself as the all in-game icons representing items inside the inventory or skills in hotbars/learning trees/e.t.c. - are "messed up". "Messed up" in this case means that, for example, for an item "sword" you would got displayed anything else other than "sword" icon.
This problem is actual with latest Wine (1.4), and seems to affect all the localized versions of PW out there: I've seen complaints about this bug affecting PW International on APPDB PWI page, I've seen complaints about messed up icons on Chinese forums, e.t.c. The regression testing I had done had been held using localized Russian version of PW client (by Mail.Ru, downloaded from pw.mail.ru). Unfortunately I can't repeat the test using international version as IPs belonging to AS located in Russia are region-restricted from registration/playing on PWI servers.
This regression is a bit tricky and it was a bit hard to cut the things down to a single commit id. Problem with straightforward bisecting is as follows: the game works OK up to but not including commit a8427507. Starting with commit a8427507 and up to but not including commit 156d24c6 the game refuses to start hanging early during startup process and displaying so-called "red traffic light" bugreporting dialog box. From commit 156d24c6 onward the game starts up but with icons messed up.
Checking out commit 156d24c6, and then reverting commit a8427507 using "git show a8427507 | git apply -R --" fixes an issue making icons to be displayed as they expected to be. Checking out any revision in-between a8427507..156d24c6 and trying to compile and run the game with is "as is" results in game hang on startup. Checking out any revision in-between a8427507..156d24c6, reverting commit a8427507 and compiling wine results in the normal game startup and correct skills/items icons.
Just for completeness, here is "git bisect" results: =================================================================
First commit where the game stops working: [lexa2@lx2linux wine-git]$ git bisect bad a84275075d4a20a9a7f42eda01585d34f31b56d4 is the first bad commit commit a84275075d4a20a9a7f42eda01585d34f31b56d4 Author: Henri Verbeet hverbeet@codeweavers.com Date: Thu Aug 4 19:53:27 2011 +0200
wined3d: Implement wined3d_surface_bltfast() on top of wined3d_surface_blt().
:040000 040000 1ee4ae032b4da38d6ba772665b4c57f55756111a a069811143e3a52a9c6172d8bdd089f2914aac77 M dlls
=================================================================
First commit where the game starts working: [lexa2@lx2linux wine-git]$ git bisect good 156d24c63466479e72811ce0bddef1d5ec754998 is the first bad commit commit 156d24c63466479e72811ce0bddef1d5ec754998 Author: Henri Verbeet hverbeet@codeweavers.com Date: Sun Aug 21 21:34:51 2011 +0200
wined3d: Add compressed surface support to surface_cpu_blt().
:040000 040000 d8022e36543d1aaa2b3c7cb51e2918b1ded5853c 00af3b1b397df5c13b3c84b1dc6f53c6493229be M dlls
=================================================================
Finally, system specs: 32bit linux, kernel 2.6.38.8-32.i686.PAE, nVIDIA drivers 295.17, GeForce GTX 550 Ti 1024MB, 6Gb of DDR2 RAM, AMD Phenom II X4 955 CPU, gcc 4.4.5 20101112 (Red Hat 4.4.5-2).
P.S. Henri, in case you won't be able to reproduce this problem on your rig - I would capture and post here any logs you would request from me. Thanks in advance for spending your time fixing this regression.
http://bugs.winehq.org/show_bug.cgi?id=30115
Alexey Loukianov mooroon2@mail.ru changed:
What |Removed |Added ---------------------------------------------------------------------------- Keywords| |regression
http://bugs.winehq.org/show_bug.cgi?id=30115
Alexey Loukianov mooroon2@mail.ru changed:
What |Removed |Added ---------------------------------------------------------------------------- URL| |http://pwi.perfectworld.com | |/
http://bugs.winehq.org/show_bug.cgi?id=30115
Dmitry Timoshkov dmitry@baikal.ru changed:
What |Removed |Added ---------------------------------------------------------------------------- Summary|Perfect World: Skills and |Perfect World: Skills and |items icons messed up |items icons messed up |(regression) |
http://bugs.winehq.org/show_bug.cgi?id=30115
--- Comment #1 from Alexey Loukianov mooroon2@mail.ru 2012-03-09 08:59:29 CST --- Well, I had tracked this bug up to the differences in blit operation implementation for compressed textures introduced by commits 156d24c6 and a8427507. Problematic icons are stored (and blited) as surfaces in WINED3DFMT_DXT1 format. Prior to commit 156d24c6 blits for such surfaces had been handled in surface_cpu_bltfast(). After commit 156d24c6 wine had switched into using surface_cpu_blt() function instead which hadn't been able to do blits with defined destination rect in software for compressed surfaces at that moment of time. Instead, surface_cpu_blt() had been trying to do the usual width/height rect math and apply it to compressed texture which had been causing access violations try to access memory regions outside the locked surface part with the subsequent game crash as the consequence.
After commit 156d24c6 surface_cpu_blt() had been extended to support non-transforming block-based surfaces blits for compressed textures, and this support seems to look very similar to one that had been working correctly inside surface_cpu_bltfast(). Unfortunately something differs between those two implementations and I can't tell what is the difference exactly ATM, but I'm pretty sure that this difference is a bug trigger here. Going to investigate a bit deeper and report back any findings I would come with.
http://bugs.winehq.org/show_bug.cgi?id=30115
--- Comment #2 from Henri Verbeet hverbeet@gmail.com 2012-03-09 09:48:41 CST --- (In reply to comment #1)
After commit 156d24c6 surface_cpu_blt() had been extended to support non-transforming block-based surfaces blits for compressed textures, and this support seems to look very similar to one that had been working correctly inside surface_cpu_bltfast(). Unfortunately something differs between those two implementations and I can't tell what is the difference exactly ATM, but I'm pretty sure that this difference is a bug trigger here. Going to investigate a bit deeper and report back any findings I would come with.
I'm not sure if this is the problem, but I do see a potential issue with the code in surface_cpu_blt(). For the destination surface, it locks the destination rectangle if there is one, but for the source surface it always maps the entire surface and adjusts the pointer later. However, that's after the call to surface_cpu_blt_compressed(). (And wrong for compressed surfaces anyway.)
http://bugs.winehq.org/show_bug.cgi?id=30115
--- Comment #3 from Alexey Loukianov mooroon2@mail.ru 2012-03-09 09:58:38 CST --- (In reply to comment #2)
However, that's after the call to surface_cpu_blt_compressed(). (And wrong for compressed surfaces anyway.)
Actually I'm trying to hunt down the problem near it's roots, thus I'm working with commit 156d24c6 codebase. At that moment there hadn't been such thing as surface_cpu_blt_compressed() - the contents of this function had been essentially inlined into the surface_cpu_blt(). As the visual representation of the bug looks the same with wine-1.4 vs. wine-1.3.26-39-g156d24c - I hope that at the moment I would find a bug cause for wine-1.3.26-39-g156d24c - it would at the same time provide an insight on how to fix this bug with the current HEAD.
http://bugs.winehq.org/show_bug.cgi?id=30115
--- Comment #4 from Henri Verbeet hverbeet@gmail.com 2012-03-09 10:13:55 CST --- (In reply to comment #3)
Actually I'm trying to hunt down the problem near it's roots, thus I'm working with commit 156d24c6 codebase. At that moment there hadn't been such thing as surface_cpu_blt_compressed() - the contents of this function had been essentially inlined into the surface_cpu_blt().
Ok, but the flaw I described is essentially the same in that version. dbuf is a pointer to contents of the blitted rectangle in the destination surface, while sbuf is a pointer to the contents of the entire source surface.
http://bugs.winehq.org/show_bug.cgi?id=30115
--- Comment #5 from Alexey Loukianov mooroon2@mail.ru 2012-03-09 12:34:54 CST --- Created attachment 39274 --> http://bugs.winehq.org/attachment.cgi?id=39274 Patch that fixes software partial plain blits for compressed surfaces.
Yeah, thanks for pointing on this, it is actually the bug cause here. Here is patch that fixes the issue written against wine-1.4. It's pretty straightforward and I hadn't checked it for possible arithmetic overflows, but at least it works and fixes rendering issues in PW game client.
http://bugs.winehq.org/show_bug.cgi?id=30115
--- Comment #6 from Henri Verbeet hverbeet@gmail.com 2012-03-09 13:51:05 CST --- Created attachment 39276 --> http://bugs.winehq.org/attachment.cgi?id=39276 patch
Does this patch also work? There's a similar issue when the source and destination surfaces are the same.
http://bugs.winehq.org/show_bug.cgi?id=30115
--- Comment #7 from Alexey Loukianov mooroon2@mail.ru 2012-03-09 16:41:45 CST --- Hadn't tried compiling wine with your patch yet, but from what I see it should essentially do the same thing my patch does, but only if the "src_format->block_height == 1 && src_format->block_width == 1" condition is warranted to be TRUE for non-compressed formats.
A case when destination surface and source surface are the same one isn't handled for compressed formats in any case, there's FIXME("Only plain blits supported on compressed surfaces.\n") there catching such scenario. Sure, CPU-based blits for compressed surfaces should be extended to handle src == dest case correctly, but IMO it's a separate task that isn't closely related to the task of fixing the regression this bug report is about :-).
http://bugs.winehq.org/show_bug.cgi?id=30115
Henri Verbeet hverbeet@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Target Milestone|--- |1.4.x
--- Comment #8 from Henri Verbeet hverbeet@gmail.com 2012-03-12 13:28:12 CDT --- Should be fixed by 29468906bba02d97a7353729fd64ce2fd68e4c6d.
http://bugs.winehq.org/show_bug.cgi?id=30115
Vitaliy Margolen vitaliy-bugzilla@kievinfo.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|UNCONFIRMED |RESOLVED Resolution| |FIXED Target Milestone|1.4.x |---
--- Comment #9 from Vitaliy Margolen vitaliy-bugzilla@kievinfo.com 2012-03-12 21:38:44 CDT --- Marking fixed
http://bugs.winehq.org/show_bug.cgi?id=30115
Vitaliy Margolen vitaliy-bugzilla@kievinfo.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Fixed by SHA1| |29468906bba02d97a7353729fd6 | |4ce2fd68e4c6d
http://bugs.winehq.org/show_bug.cgi?id=30115
Henri Verbeet hverbeet@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Target Milestone|--- |1.4.x
http://bugs.winehq.org/show_bug.cgi?id=30115
Alexey Loukianov mooroon2@mail.ru changed:
What |Removed |Added ---------------------------------------------------------------------------- Fixed by SHA1|29468906bba02d97a7353729fd6 | |4ce2fd68e4c6d | Status|RESOLVED |UNCONFIRMED Resolution|FIXED |
--- Comment #10 from Alexey Loukianov mooroon2@mail.ru 2012-03-13 12:03:42 CDT --- Confirming fixed. Thanks for spending your time on this.
http://bugs.winehq.org/show_bug.cgi?id=30115
Alexey Loukianov mooroon2@mail.ru changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|UNCONFIRMED |RESOLVED Resolution| |FIXED
--- Comment #11 from Alexey Loukianov mooroon2@mail.ru 2012-03-13 12:04:10 CDT --- Fixed by 29468906bba02d97a7353729fd64ce2fd68e4c6d.
http://bugs.winehq.org/show_bug.cgi?id=30115
Henri Verbeet hverbeet@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Fixed by SHA1| |29468906bba02d97a7353729fd6 | |4ce2fd68e4c6d
http://bugs.winehq.org/show_bug.cgi?id=30115
--- Comment #12 from Alexandre Julliard julliard@winehq.org 2012-03-16 14:09:27 CDT --- Closing bugs fixed in 1.5.0.
http://bugs.winehq.org/show_bug.cgi?id=30115
Alexandre Julliard julliard@winehq.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|RESOLVED |CLOSED
--- Comment #13 from Alexandre Julliard julliard@winehq.org 2012-03-16 17:11:45 CDT --- Really closing bugs fixed in 1.5.0.
http://bugs.winehq.org/show_bug.cgi?id=30115
Alexandre Julliard julliard@winehq.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Target Milestone|1.4.x |---
http://bugs.winehq.org/show_bug.cgi?id=30115
Bart Bartolome linuxbastard@paglalakbay.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |linuxbastard@paglalakbay.co | |m
--- Comment #14 from Bart Bartolome linuxbastard@paglalakbay.com --- Good day!
The issue seems to be back in 1.6.2 and 1.7.16.
Icons in game are represented as "?" icons instead of the usual icons seen in windows installs.
This is a fresh install of Wine in Ubuntu 13.10 x64. PWI was installed using Arc install and the game launches OK and works except for the skills and items icons.
I thought about filing a new bug report but wanted to know if this is another regression.
Thanks