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,
> 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
>>
>>Attachment:
UintFromObj.patch
Description: Binary data
Attachment:
surf_get_masks.patch
Description: Binary data
Attachment:
intify.patch
Description: Binary data