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!

Reply via email to