[Bug 9775] Syberia 2, Max Payne 1 & 2 - Screenshots and thumbnails broken

[wine-bugs at winehq.org]

http://bugs.winehq.org/show_bug.cgi?id=9775





--- Comment #24 from Tobias Jakobi <liquid.acid at 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


-- 
Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email
Do not reply to this email, post in Bugzilla using the
above URL to reply.
------- You are receiving this mail because: -------
You are watching all bug changes.