Update: Patches work fine on my 64-bit machine. don@xxxxxxxxxxxxxxxxx wrote:
Hm... the last attachment didn't make it in. here it is. On Tue, 7 Jul 2009 13:28:18 +0200, <don@xxxxxxxxxxxxxxxxx> 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 <don@xxxxxxxxxxxxxxxxx> 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.yoshi@xxxxxxxxx <mailto:hugo.yoshi@xxxxxxxxx>> wrote: On Thu, Jul 2, 2009 at 2:55 PM, <don@xxxxxxxxxxxxxxxxx <mailto:don@xxxxxxxxxxxxxxxxx>> wrote: > > Hi, > > On Thu, 2 Jul 2009 09:48:48 +1000, René Dudfield <renesd@xxxxxxxxx <mailto:renesd@xxxxxxxxx>> wrote: >> Cool, thanks for the testing. >> >> >> On Thu, Jul 2, 2009 at 9:19 AM, Lorenz Quack<don@xxxxxxxxxxxxxxxxx <mailto:don@xxxxxxxxxxxxxxxxx>> 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,alongwith 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>