On Tue, Jul 7, 2009 at 9:28 PM, <d...@amberfisharts.com> wrote: > Hi, > > I'm getting the feeling that either there are quite some bugs in the > surface_test code or that I don't get what's going on. > > first I looked at the failing test I mentioned before (~line 104): > > def make_surf(bpp, flags, masks): > pygame.Surface((10, 10), flags, bpp, masks) > self.failUnlessRaises(ValueError, make_surf, 32, 0, > (0xFF000000, 0xFF0000, 0xFF00, 0)) > > and saw no good reason why the call to make_surf should raise an > ValueError. > Please tell me if I'm wrong here. >
I don't understand why either? > > So then I thought maybe it's not a bug with 64-bit machines but rather with > 32-bit ones and indeed I found a suspicious piece of code in base.c (line > 335): > > *val = (Uint32) PyInt_AsLong (intobj); > > if the intobj is negative (like 0xFF000000) the result won't be as desired. > I changed that function (UintFromObj) to use a PyLong instead of a PyInt > and then call PyLong_AsUnsignedLong (patch attached). > Yeah, that seems sensible. It should really be called Uint32FromObj. > > Again: If my assumption that the unit test should not raise that ValueError > is wrong this might be a moot point. > > with this patch applied the ValueError is not raised but then two other > test in surface_test.py fail. namely test_blit_blend_rgba and > test_blit_blend. > > I first looked into test_blit_blend_rgba and found the function > surf_get_masks to be questionable. the Py_BuildValue uses "i" as format > characters which represent ints using "I" for unsigned int makes the Error > go away (patch attached). > sounds good. > > last but not least test_blit_blend. My suspicion is that the intify helper > function screws things up. I also attached a patch with my idea of an intify > function. > > with all the patches applied all test pass except for test_flags (because > of the UintFromObj.patch) so I also attached a patch which fixes the test > (and renames it to test_masks because that is what is tested and not the > flags. > > again looks good. > > Note that I didn't test these patches on my 64-bit machine yet. > I'll let you know as soon as I've done so. > > Please let me know if I got something wrong. > > > yours > //Lorenz > > > Committed revision 2486. cheers!