Re: wine3d3: Check the destination recangle when for FastBlt().

[Peter Dons Tychsen]

On Mon, 2007-07-30 at 22:18 +0200, Peter Dons Tychsen wrote:
> On Sun, 2007-07-29 at 23:08 +0200, Stefan Dösinger wrote:
> > Am Sonntag, 29. Juli 2007 21:28 schrieb Peter Dons Tychsen:
> > > Hello Wine!
> > >
> > > I have fixed a small bug in FastBlt(), which caused Wine to crash if the
> > > application tried to do FastBlt() to a surface using a bad destination
> > > setup. The problem is already fixed in normal Blt(), but was broken for
> > > FastBlt().
> > >
> > > This fixes a crash for the little game "Treasure Mole Winter Vacations":
> >
> > This check should be in ddraw, not wined3d. ddraw, d3d8 and d3d9 have 
> > different ideas about such checks, that's why the check was moved from 
> > WineD3D to the client libs. Originally it was in LockRect, and when it was 
> > moved the check was lost in Blt and BltFast. There should be a check in ddraw 
> > already, maybe it is not sufficient.
> >
> > Also please write a test case to verify that the fix is correct(look at 
> > dlls/ddraw/tests/dsurface.c). This will also prevent the problem from coming 
> > back. Treasure Mole broke after the ddraw rewrite because exactly this check 
> > was missing. I implemented it in LockRect, like in old ddraw, but I forgot to 
> > write a test for BltFast. Then a d3d9 game was broken because d3d9 LockRect 
> > behaved differently, a test was written for Lock(Rect) in ddraw, d3d8 and 
> > d3d9, and the range check moved to ddraw, d3d8 and d3d9 to be able to 
> > implement different checking in each version. Since I forgot to write a test 
> > when I originally fixed Treasure Mole the bug could creep back in again :-(

Thanks for your comments Stefan,

I have taken another look at it. I still think this specific problem
should be fixed in wined3d. The problem occurs because BltFast (ddraw,
d3d and d3d-gdi) all take two DWORDs as the offset write position, and
then later casts them into a RECT structure which has signed values.
This must for any caller be considered an error. No caller could get
anything useful out of this.

Code:

IWineGDISurfaceImpl_BltFast(IWineD3DSurface *iface,
                            DWORD dstx,
                            DWORD dsty,
                            IWineD3DSurface *Source,
                            RECT *rsrc,
                            DWORD trans)

...

lock_dst.left = dstx;  <--- bad cast!
lock_dst.top = dsty; <--- bad cast!
lock_dst.right = dstx + w; <--- bad cast!
lock_dst.bottom = dsty + h; <--- bad cast!

Why does this not trigger a warning? Not sure.

I can still move the fix to ddraw, but as said, i am not sure it's the
right move.

Please comment,

/pedro

Please note: this is a re-send. I think my mail-server trashed the first
one. Not sure what happened. Sorry if you get this more than once.