Hm... the last attachment didn't make it in. here it is.
On Tue, 7 Jul 2009 13:28:18 +0200, <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. > > 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). > > 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). > > 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. > > > 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 > > > > On Tue, 07 Jul 2009 00:40:39 +0200, Lorenz Quack <d...@amberfisharts.com> > wrote: >> Hi, >> >> yup. the color test now pass. >> the failing midi is not really a bug as I don't have portmidi. >> I haven't looked into the failing surface test, yet. >> >> >> >> René Dudfield wrote: >>> Hi, >>> >>> the Color 64bit errors are fixed. A combination of testing against >>> LONG_MAX, and using Uint32. >>> >>> Committed revision 2472. >>> >>> cheers, >>> >>> >>> On Thu, Jul 2, 2009 at 11:52 PM, Hugo Arts <hugo.yo...@gmail.com >>> <mailto:hugo.yo...@gmail.com>> wrote: >>> >>> On Thu, Jul 2, 2009 at 2:55 PM, <d...@amberfisharts.com >>> <mailto:d...@amberfisharts.com>> wrote: >>> > >>> > Hi, >>> > >>> > On Thu, 2 Jul 2009 09:48:48 +1000, René Dudfield >>> <ren...@gmail.com <mailto:ren...@gmail.com>> wrote: >>> >> Cool, thanks for the testing. >>> >> >>> >> >>> >> On Thu, Jul 2, 2009 at 9:19 AM, Lorenz >>> Quack<d...@amberfisharts.com <mailto:d...@amberfisharts.com>> wrote: >>> >>> Ok, so here goes my analysis of the failures in ColorTypeTest: >>> >>> In src/color.c line 1435 (and some following lines) we bit >>> shift each >>> >> color >>> >>> component and stuff them >>> >>> in an unsigned long. >>> >> >>> >> Maybe this should be into a uint32 instead? >>> >> >>> > >>> > Yes, that would correspond to my solution 1) only better because >>> it ensures that it will have exactly 32 bit. >>> > BTW, are these types uint32, UInt8, ... in the C-standard? >>> > >>> > yours >>> > //Lorenz >>> > >>> >>> No. ANSI C only defines the char, int, float, and double types, >> along >>> with the short/long and signed/unsigned modifiers. >>> The sizes of these types are not defined in the standard. ANSI only >>> gives some minimum sizes, and mandates that >>> >>> short int <= int <= long int >>> float <= double <= long double >>> >>> So, in theory, the size of your int type is completely platform >>> dependent. In practice nearly every modern compiler uses the same >>> sizes, >>> char = 8, int = 32, float = 32, double = 64, but there are some >>> differences between 32bit and 64bit architectures (e.g. long int is >>> usually the same size as int on 32bit). Hence the often defined >>> uint32, uint8 etc. types. >>> >>> See also http://en.wikipedia.org/wiki/C_data_types >>> >>> Hugo >>> >>>
test_flags.patch
Description: Binary data