http://bugs.winehq.org/show_bug.cgi?id=9775
--- Comment #24 from Tobias Jakobi liquid.acid@gmx.net 2008-07-01 10:54:59 --- (In reply to comment #22)
Updating summary to include some other affected games.
Tobias, looks like this patch wasn't accepted yet?
No, and I dunno why. Henri Verbeet had some provisos about it and I explained it to him, but he didn't reply after that. I'm going to paste my message here:
H. Verbeet wrote: I don't feel strongly about it, it's just that the comment talks a lot about what MSDN says or doesn't say, without actually describing what D3DPOOL_SCRATCH is supposed to mean, while ultimately that's all we care about.
I replied: That wasn't my primary intention. I didn't know why exactly the comment in CreateOffscreenPlainSurface was there, maybe it's just because this information was also given in the MSDN DirectX9 docs (see docs to IDirect3DDevice9::CreateOffscreenPlainSurface, there it tells you the same thing).
I just wanted to make clear that MSDN is wrong there. I don't know very much about how D3D is implemented in wine, but I think most of the time it's just looking into the docs and implementing the behaviour that's described there. Now someone searching for a bug or doing a rewrite of the D3D (or something else that involves big changes to the code) could be confused to see that behaviour described by MSDN and implemented behaviour differ. That's the only reason for the large amount of comments. And I thought it would be good to also add a comment in CreateOffscreenPlainSurface, so the whole thing is consistent (it's bad if one comment tells you one thing and then the other the opposite). I would rather not rip the whole comment out, because now we've got a better documentation of the DX behaviour than the MSDN itself :-)
Concerning the description of the pool types. I'm not exactly sure what you would want in such a comment. The current comment just talks about what happens if you plug D3DPOOL_SYSTEMMEM (previously D3DPOOL_SCRATCH, but we now know this is wrong) into CreateOffscreenPlainSurface. According to the docs that results in a surface object with same properties like the old DX8 method CreateImageSurface. That's all what the comment states. It's not about how the different pool types work internally and I doubt CreateOffscreenPlainSurface would be the right method where to place such a general comment. I don't know where pool types are actually "implemented" in wine, but description of the types should belong there.
Keep in mind that I'm absolutely newbie on this matter (apart from some programming with OpenGL), so I could be talking a lot of nonsense here :-D
Greets, Tobias
(quote END)
If the length of comments and msdn links are controversial, I suggest trimming them for brevity or maybe avoiding altogether.
There are no links to the MSDN in the patch. In my opinion the comments are necesarry, see my message to H. Verbeet.
msdn links are mostly avoided afaik because they change all the time, besides, it's best to shortly describe what happens in your own words and/or write a test, since msdn is misleading quite often.
Again, I didn't included any links. There are some keywords mentioned in the patch, which you can search in the MSDN to find what I'm basing my statements on. But that's all. I'm well aware that MSDN is always in flux, so the critical (mis)information is included in the patch.
Since there is a test, there shouldn't be much questions, it may be ok even without comments at all or maybe with "... creates surfaces in D3DPOOL_SYSTEM; games ... rely on it" or similar one liner. And wrong comment in d3d9 can simply be removed, maybe in a separate patch.
Again see my message to H. Verbeet.
Wine's implementation of D3D is based on what you can read in the MSDN. If the MSDN is wrong then we should comment that in wine's code.
I have an idea Alexander, why don't you hand in the (my) patch? I guess it's going to be applied in no time ;-)
Greets, Tobias