http://bugs.winehq.org/show_bug.cgi?id=29162
Bug #: 29162 Summary: Gens 11b rerecording: fails to initialize drawing surface Product: Wine Version: 1.3.33 Platform: x86-64 OS/Version: Linux Status: UNCONFIRMED Severity: normal Priority: P2 Component: -unknown AssignedTo: wine-bugs@winehq.org ReportedBy: marzojr@yahoo.com Classification: Unclassified
Created attachment 37617 --> http://bugs.winehq.org/attachment.cgi?id=37617 Log with WINEDEBUG=all+relay,+seh,+tid,+ddraw,+d3d_surface
Starting with Wine 1.3.33, when I start Gens-11b rerecording (available for free at http://code.google.com/p/gens-rerecording), I get the following error message:
Ooups... Error with lpDD_Back->SetSurfaceDesc !
The error message appears on a message box, twice on a row -- once trying to create a 2x surface, another time trying to fallback to 1x. Following that, the emulator window is transparent and won't draw anything; everything else seems to work fine. This is for the emulator in windowed mode, by the way.
If I try it full screen instead, the emulator fails to initialize fullscreen and tries to fallback to windowed mode, in which case the error chain proceeds as above.
The error happens whether directdraw drawing mode is GDI or OpenGL, and whether or not a virtual desktop is enabled in winecfg.
Up until the previous version of Wine, the emulator worked perfectly fine. I am working on Ubuntu Oneiric, and using the Wine version from the official Wine PPA. I have tried with a clean Wine directory, as well as one with the D3D libraries overridden using Winetricks. Other Windows games work normally.
I am unsure which component exactly is affecting this, so I am marking it as 'unknown'.
Attached is the backtrace with WINEDEBUG=all+relay,+seh,+tid,+ddraw,+d3d_surface
http://bugs.winehq.org/show_bug.cgi?id=29162
--- Comment #1 from marzojr@yahoo.com 2011-12-03 19:11:41 CST --- Still happens in 1.3.34.
http://bugs.winehq.org/show_bug.cgi?id=29162
GyB gyebro69@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Keywords| |download, regression URL| |http://code.google.com/p/ge | |ns-rerecording/downloads/de | |tail?name=Gens11b.zip CC| |gyebro69@gmail.com
--- Comment #2 from GyB gyebro69@gmail.com 2011-12-03 22:51:21 CST --- If the application worked with a previous Wine version, please perform a regression test: http://wiki.winehq.org/RegressionTesting
http://bugs.winehq.org/show_bug.cgi?id=29162
--- Comment #3 from marzojr@yahoo.com 2011-12-04 18:43:27 CST --- Created attachment 37800 --> http://bugs.winehq.org/attachment.cgi?id=37800 The end result of regression testing with git bisect.
http://bugs.winehq.org/show_bug.cgi?id=29162
marzojr@yahoo.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |stefan@codeweavers.com Component|-unknown |directx-d3d Regression SHA1| |c3d6061593601463ac96d632956 | |b645c4c6342c2
--- Comment #4 from marzojr@yahoo.com 2011-12-04 18:46:52 CST --- Thanks for the link; I have performed the regression testing and have attached the final git bisect output to the tracker.
http://bugs.winehq.org/show_bug.cgi?id=29162
--- Comment #5 from marzojr@yahoo.com 2011-12-04 19:16:28 CST --- Created attachment 37801 --> http://bugs.winehq.org/attachment.cgi?id=37801 Patch done with git diff
I have looked at the commitdiff and came up with a patch that fixes the problem; I have attached it to this post. It seems that rejecting out of hand all but the two bits listed in the comment gives incorrect behavior, and they should be ignored instead.
http://bugs.winehq.org/show_bug.cgi?id=29162
--- Comment #6 from Stefan Dösinger stefan@codeweavers.com 2011-12-05 12:33:48 CST --- Thanks for doing the regression test! It is plausible that IDirectDrawSurface3, IDirectDrawSurface4 and IDirectDrawSurface7 have different SetSurfaceDesc behavior. You can help by porting the set_surface_desc_test in dlls/ddraw/tests/dsurface.c to IDirectDrawSurface4 and IDirectDrawSurface7 and see if any of those functions behave differently.
http://bugs.winehq.org/show_bug.cgi?id=29162
marzojr@yahoo.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Attachment #37801|0 |1 is obsolete| |
--- Comment #7 from marzojr@yahoo.com 2011-12-05 16:21:31 CST --- Created attachment 37815 --> http://bugs.winehq.org/attachment.cgi?id=37815 Patch done with git diff
I did as you asked, then cross-compiled and tested on my XP VM; the full patch is attached. The tests for IDirectDrawSurface3, IDirectDrawSurface4 and IDirectDrawSurface7 all work on Windows; with my proposed patch above, they fail on Wine. There is clearly something strange going on in SetSurfaceDesc.
For what is worth, I also went through the Gens rerecording source (as it might yield some clues) and found the relevant code; here is a link to an extract of the relevant file, on pastebin: http://pastebin.com/enHt8JNp
As you can see, it sets up a call to SetSurfaceDesc from lines 257-297 that clearly uses more that DDSD_LPSURFACE and DDSD_PIXELFORMAT; however, it explicitly checks the return code for failure, which means that the call succeeds on Windows.
What *could* be happening is that SetSurfaceDesc fails is you *change* anything other than what DDSD_LPSURFACE and DDSD_PIXELFORMAT allow, but it works if they would result on no change. I will see if I can make a simpler test case this weekend to test this.
Which means that, for now, I would not recommend actually applying the patch(es) I attached to the Wine source, even though they fix the problem.
http://bugs.winehq.org/show_bug.cgi?id=29162
--- Comment #8 from Stefan Dösinger stefan@codeweavers.com 2011-12-06 04:08:46 CST --- (In reply to comment #7)
I did as you asked, then cross-compiled and tested on my XP VM; the full patch is attached. The tests for IDirectDrawSurface3, IDirectDrawSurface4 and IDirectDrawSurface7 all work on Windows; with my proposed patch above, they fail on Wine. There is clearly something strange going on in SetSurfaceDesc.
Great, thanks a lot! Yeah, it doesn't look like there is version specific behavior here.
Two suggestions: You QueryInterface IDirectDrawSurface4 from IDirectDrawSurface4 and IDirectDrawSurface7 from IDirectDrawSurface7. That's not necessary, you can just drop the QI calls(and the 2nd surface pointer). In the IDirectDrawSurface3 test this is necessary because IDirectDraw::CreateSurface and IDirectDraw2::CreateSurface return an IDirectDrawSurface, not an IDirectDrawSurface3. (And IDirectDraw3 does not exist in ddraw.dll. It exists on ly in ddrawex.dll, which is part of Internet Explorer and not used by other apps).
For IDirectDraw7, it is better to create it via DirectDrawCreateEx instead of QIing it from IDirectDraw. There's a difference between those creation methods, see the IDirect3D* QI tests in tests/refcount.c.
As you can see, it sets up a call to SetSurfaceDesc from lines 257-297 that clearly uses more that DDSD_LPSURFACE and DDSD_PIXELFORMAT; however, it explicitly checks the return code for failure, which means that the call succeeds on Windows.
The source code is pretty helpful. The Rend < 2 check looks suspicious. It looks that this is the switch that toggles between windowed and fullscreen mode. I'll boot my Windows box and see if the app works in Windowed mode there. It is possible that it never enters the SetSurfaceDesc codepath on Windows.
What *could* be happening is that SetSurfaceDesc fails is you *change* anything other than what DDSD_LPSURFACE and DDSD_PIXELFORMAT allow, but it works if they would result on no change. I will see if I can make a simpler test case this weekend to test this.
I tested what happens when you pass the original surface description to SetSurfaceDesc, and this fails. However, the change got lost when my harddrive died two weeks ago.
I can imagine that there are some flags in the original SetSurfaceDesc that fail even if the values are equal, but others like width, height and pitch are accepted.
Which means that, for now, I would not recommend actually applying the patch(es) I attached to the Wine source, even though they fix the problem.
Patches that break the tests aren't accepted anyway.
http://bugs.winehq.org/show_bug.cgi?id=29162
--- Comment #9 from Stefan Dösinger stefan@codeweavers.com 2011-12-06 05:14:01 CST --- I compiled Gens on Windows and SetSurfaceDesc is called and succeeds. I did some more testing:
*) DDSD_CAPS is the reason why the original description cannot be passed to SetSurfaceDesc *) DDSD_HEIGHT is changeable, but must not be set to 0 *) DDSD_WIDTH and DDSD_PITCH are changeable as well, but must not be 0 and must be changed together. The pitch can be bigger than the one ddraw would calculate.
What I forgot to test before I rebooted was if (width | pitch) or height need DDSD_LPSURFACE as well, just like the pixelformat or if it can be set alone.
http://bugs.winehq.org/show_bug.cgi?id=29162
--- Comment #10 from Stefan Dösinger stefan@codeweavers.com 2011-12-06 06:12:31 CST --- Created attachment 37820 --> http://bugs.winehq.org/attachment.cgi?id=37820 ddraw: SetSurfaceDesc can set width, height and pitch under certain conditions
I've extended the SetSurfaceDesc test and adjusted the implementation. Now Gens starts successfully again. A port to surface4 and surface7 is still missing though.
The app is very slow here on Wine. According to top the CPU(s) are mostly idle, and oprofile says most of the time is spent in read_hpet() in the kernel. As far as I know this this function is used for waiting, but I'm not certain about this.
http://bugs.winehq.org/show_bug.cgi?id=29162
--- Comment #11 from marzojr@yahoo.com 2011-12-06 06:32:49 CST --- (In reply to comment #10)
I've extended the SetSurfaceDesc test and adjusted the implementation. Now Gens starts successfully again. A port to surface4 and surface7 is still missing though.
Thanks; I will try the patch locally and write the tests as you directed; my DirectX has been a bit rusty, since I haven't used it in years.
The app is very slow here on Wine. According to top the CPU(s) are mostly idle, and oprofile says most of the time is spent in read_hpet() in the kernel. As far as I know this this function is used for waiting, but I'm not certain about this.
Yeah, I have been meaning to file a bug report on it; in Ubuntu, at least, I think it worked very well until about Maverick or maybe Lucid. I have to check the versions to see -- and confirm whether it really is a problem in Wine or if it is something else.
http://bugs.winehq.org/show_bug.cgi?id=29162
marzojr@yahoo.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Attachment #37815|0 |1 is obsolete| |
--- Comment #12 from marzojr@yahoo.com 2011-12-06 07:49:15 CST --- Created attachment 37822 --> http://bugs.winehq.org/attachment.cgi?id=37822 ddraw: SetSurfaceDesc can set width, height and pitch under certain conditions, plus set_surface_desc_test ports to IDirectDrawSurface4 and IDirectDrawSurface7
I made the ports again, using the new tests you added. I also modified the text for test failure to be more consistent and always include the interface in question; I combined your patch in mine due to this.
I looked at the docs, and it seems that using DirectDrawCreateEx would fail with DDERR_DIRECTDRAWALREADYCREATED in this case -- there already is a DirectDraw created. So I left it out.
http://bugs.winehq.org/show_bug.cgi?id=29162
--- Comment #13 from Stefan Dösinger stefan@codeweavers.com 2011-12-06 12:16:14 CST --- (In reply to comment #12)
I made the ports again, using the new tests you added. I also modified the text for test failure to be more consistent and always include the interface in question; I combined your patch in mine due to this.
My test is missing at least one thing: Test pitch != width * format_bpp, especially pitch < width * format_bpp. I'll adjust my test and send it to wine-patches. Once my test is applied you can send your patches. This is easier than merging the changes into one patch: It keeps the patches smaller and the authorship clear.
I looked at the docs, and it seems that using DirectDrawCreateEx would fail with DDERR_DIRECTDRAWALREADYCREATED in this case -- there already is a DirectDraw created. So I left it out.
Hmm, it worked for me for the QueryInterface tests I recently wrote(not yet sent in). I guess the docs are wrong here, or probably it only works that way when one of the instances is in exclusive mode.
http://bugs.winehq.org/show_bug.cgi?id=29162
--- Comment #14 from Stefan Dösinger stefan@codeweavers.com 2011-12-07 17:16:31 CST --- I sent my patch to wine-patches. I fixed the ok message inconsistencies myself, and I moved the unrelated SetSurfaceDesc(ddsd=NULL) test away. I recommend to wait with the dsurface4 and 7 ports until my patches are merged.
http://bugs.winehq.org/show_bug.cgi?id=29162
Stefan Dösinger stefan@codeweavers.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Fixed by SHA1| |481e2f336685cdbc6a14b0ebf92 | |74bb98cbe5cea Status|UNCONFIRMED |RESOLVED Resolution| |FIXED
--- Comment #15 from Stefan Dösinger stefan@codeweavers.com 2011-12-08 17:14:28 CST --- My patches made it in, so this bug should be fixed.
You can now go ahead and update or redo your ports of the test to surface4 and surface7. I've also taken care of the DirectDrawCreateEx function in dsurface.c(we load it at runtime because not all ddraw.dll version on windows have it).
I'm resolving this bug, for any further discussions of the tests we can use wine-devel.
http://bugs.winehq.org/show_bug.cgi?id=29162
Alexandre Julliard julliard@winehq.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|RESOLVED |CLOSED
--- Comment #16 from Alexandre Julliard julliard@winehq.org 2011-12-16 13:29:53 CST --- Closing bugs fixed in 1.3.35.
-
wine-bugs＠winehq.org