[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]
Re: [pygame] BUG: pygame.Color bugs - 64bit. Re: beginning GSOC preparations: was Re: [pygame] Thank You
Cool, thanks.
I think we can look applying this patch after Lenard has merged his
py3k stuff back in... so we don't get any many conflicts. As I
noticed he's already touched the color.c.
We shouldn't ever be returning a long I don't think. So perhaps we
can do a conversion from python long to python int at the end of the
functions.
For endian checking, you can use this define...
#if SDL_BYTEORDER == SDL_LIL_ENDIAN
// lil endian code here...
#else
// big endian code here...
#endif
However, I'm not positive the color code *should* be endian safe...
without using the map/unmap methods on Surface for example. However I
think maybe it should be. We will have to study how it is being used
at the moment... and how the old behaviour worked(as some code might
rely on it being endian unsafe).
ps. with patches it's good to check them to make sure they're only
for the change you made... not for other issues at the same time. eg,
there were lots of windows end of line characters in the color test
before, and your patch removes them.
pps. just a note, that a request has been put in to the lovely server
administrators at seul.org for adding svn accounts for the all the
GSOC peoples. So you should get your svn account within a week or so.
cu,
On Sat, May 2, 2009 at 11:42 AM, Tyler Laing <trinioler@xxxxxxxxx> wrote:
> Hmm, just noticed another issue. I had to make the change from INT_MAX to
> LONG_MAX because the outputs for the specific number had an 'L' on the end
> of them, where the number exceeds INT_MAX, and so a 'L' is appended to the
> end of the outputs from oct and hex, which causes a lot of fails, as when
> Python converts the numbers to hex/int, there is no 'L' on the end of that
> value.
>
> Basically this line:
>
> self.assertEquals (hex (c), str(hex (0xCC00CC11))
>
> fails with this message:
>
> ======================================================================
> FAIL: ColorTypeTest.test_webstyle
> ----------------------------------------------------------------------
> Traceback (most recent call last):
> File "/usr/lib/python2.5/site-packages/pygame/tests/color_test.py", line
> 465, in test_webstyle
> self.assertEquals (hex (c), hex (0xCC00CC11))
> AssertionError: '0xcc00cc11L' != '0xcc00cc11'
>
> My fix, which I do not like, at all, is:
>
> self.assertEquals (str(hex (c))[:-1], str(hex (0xCC00CCFF)))
>
> I don't know if this would behave differently on a 32 bit platform.
>
> Here is the patch.diff.
>
> -Tyler
>
> On Fri, May 1, 2009 at 6:23 PM, René Dudfield <renesd@xxxxxxxxx> wrote:
>>
>> Hi,
>>
>> cool, nice one.
>>
>> make your change to the file/files, make sure it compiles and passes
>> the tests...
>>
>> Then run: svn diff > patch.diff
>>
>> Then send the patch to the mailing list... or upload it somewhere, and
>> send a link to it if it's really big... or just paste it into the
>> email if it's tiny.
>>
>> cu,
>>
>>
>>
>>
>> On Sat, May 2, 2009 at 11:21 AM, Tyler Laing <trinioler@xxxxxxxxx> wrote:
>> > Rene,
>> >
>> > Okay, so I've got a fix. You have to prefix color->r with (long), and
>> > then
>> > for hex and oct functions, you need to change INT_MAX to LONG_MAX for a
>> > 64
>> > bit platform.
>> >
>> > How do I make a patch for submission?
>> >
>> > -Tyler
>> >
>> > On Fri, May 1, 2009 at 5:33 PM, Tyler Laing <trinioler@xxxxxxxxx> wrote:
>> >>
>> >> Rene,
>> >>
>> >> You are right. I isolated the specific issue, and here's a sample .c
>> >> file
>> >> that shows the error on the 64 bit platform. When I get something that
>> >> works
>> >> on the the test file, I'll try it on the actual pygame code and see how
>> >> the
>> >> test performs.
>> >>
>> >> -Tyler
>> >>
>> >> On Fri, May 1, 2009 at 4:30 PM, Tyler Laing <trinioler@xxxxxxxxx>
>> >> wrote:
>> >>>
>> >>> I'll try that then. For reference, I am using an AMD Athlon 64 X2
>> >>> 5200+
>> >>> processor. What would be the proper way to make it endian safe?
>> >>>
>> >>> Change the unsigned long tmp to unsigned int tmp?
>> >>>
>> >>> -Tyler
>> >>>
>> >>> On Fri, May 1, 2009 at 4:24 PM, René Dudfield <renesd@xxxxxxxxx>
>> >>> wrote:
>> >>>>
>> >>>> hi,
>> >>>>
>> >>>> there's some parts like this...
>> >>>>
>> >>>> static PyObject*
>> >>>> _color_float (PyColor *color)
>> >>>> {
>> >>>> unsigned long tmp = (color->r << 24) + (color->g << 16) +
>> >>>> (color->b
>> >>>> << 8) +
>> >>>> color->a;
>> >>>> return PyFloat_FromDouble ((double) tmp);
>> >>>> }
>> >>>>
>> >>>> this code isn't endian or 64bit safe... since it is using bit
>> >>>> shifting
>> >>>> for packing. On different platforms, this produces different
>> >>>> outputs.
>> >>>>
>> >>>> I think it has to convert into the same 32bit unsigned int, and then
>> >>>> return that.
>> >>>>
>> >>>>
>> >>>>
>> >>>>
>> >>>> On Sat, May 2, 2009 at 8:54 AM, Tyler Laing <trinioler@xxxxxxxxx>
>> >>>> wrote:
>> >>>> > Taking a look at color.c, I believe the bug may actually rest in
>> >>>> > the
>> >>>> > Py<type>_FromUnsignedLong/Py<type>_FromDouble/Py<type>_FromString
>> >>>> > functions
>> >>>> > provided by the Python libs. There is no logical or numerical
>> >>>> > reason
>> >>>> > why,
>> >>>> > from the numbers we have, we would get those values with those
>> >>>> > operations.
>> >>>> > The tests beforehand affirm that the r, g, b, and a variables all
>> >>>> > the
>> >>>> > proper
>> >>>> > values, it just happens to be the one step in the code. I'll
>> >>>> > examine
>> >>>> > further.
>> >>>> >
>> >>>> > -Tyler
>> >>>> >
>> >>>> > On Fri, May 1, 2009 at 3:28 PM, René Dudfield <renesd@xxxxxxxxx>
>> >>>> > wrote:
>> >>>> >>
>> >>>> >> hi,
>> >>>> >>
>> >>>> >> Below are the failing unittests for Color on 64bit ubuntu.
>> >>>> >>
>> >>>> >>
>> >>>> >>
>> >>>> >>
>> >>>> >>
>> >>>> >>
>> >>>> >> ======================================================================
>> >>>> >> > FAIL: ColorTypeTest.test_float
>> >>>> >> >
>> >>>> >> >
>> >>>> >> > ----------------------------------------------------------------------
>> >>>> >> > Traceback (most recent call last):
>> >>>> >> > File
>> >>>> >> > "/usr/lib/python2.5/site-packages/pygame/tests/color_test.py",
>> >>>> >> > line
>> >>>> >> > 412, in test_float
>> >>>> >> > self.assertEquals (float (c), float (0xCC00CC00))
>> >>>> >> > AssertionError: 1.844674407283719e+19 != 3422604288.0
>> >>>> >> >
>> >>>> >> >
>> >>>> >> >
>> >>>> >> > ======================================================================
>> >>>> >> > FAIL: ColorTypeTest.test_hex
>> >>>> >> >
>> >>>> >> >
>> >>>> >> > ----------------------------------------------------------------------
>> >>>> >> > Traceback (most recent call last):
>> >>>> >> > File
>> >>>> >> > "/usr/lib/python2.5/site-packages/pygame/tests/color_test.py",
>> >>>> >> > line
>> >>>> >> > 442, in test_hex
>> >>>> >> > self.assertEquals (hex (c), hex (0xCC00CC00))
>> >>>> >> > AssertionError: '0xffffffffcc00cc00L' != '0xcc00cc00'
>> >>>> >> >
>> >>>> >> >
>> >>>> >> >
>> >>>> >> > ======================================================================
>> >>>> >> > FAIL: ColorTypeTest.test_int
>> >>>> >> >
>> >>>> >> >
>> >>>> >> > ----------------------------------------------------------------------
>> >>>> >> > Traceback (most recent call last):
>> >>>> >> > File
>> >>>> >> > "/usr/lib/python2.5/site-packages/pygame/tests/color_test.py",
>> >>>> >> > line
>> >>>> >> > 494, in test_int
>> >>>> >> > self.assertEquals (int (c), int (0xCC00CC00))
>> >>>> >> > AssertionError: 18446744072837188608L != 3422604288
>> >>>> >> >
>> >>>> >> >
>> >>>> >> >
>> >>>> >> > ======================================================================
>> >>>> >> > FAIL: ColorTypeTest.test_long
>> >>>> >> >
>> >>>> >> >
>> >>>> >> > ----------------------------------------------------------------------
>> >>>> >> > Traceback (most recent call last):
>> >>>> >> > File
>> >>>> >> > "/usr/lib/python2.5/site-packages/pygame/tests/color_test.py",
>> >>>> >> > line
>> >>>> >> > 511, in test_long
>> >>>> >> > self.assertEquals (long (c), long (0xCC00CC00))
>> >>>> >> > AssertionError: 18446744072837188608L != 3422604288L
>> >>>> >> >
>> >>>> >> >
>> >>>> >> >
>> >>>> >> > ======================================================================
>> >>>> >> > FAIL: ColorTypeTest.test_oct
>> >>>> >> >
>> >>>> >> >
>> >>>> >> > ----------------------------------------------------------------------
>> >>>> >> > Traceback (most recent call last):
>> >>>> >> > File
>> >>>> >> > "/usr/lib/python2.5/site-packages/pygame/tests/color_test.py",
>> >>>> >> > line
>> >>>> >> > 427, in test_oct
>> >>>> >> > self.assertEquals (oct (c), oct (0xCC00CC00))
>> >>>> >> > AssertionError: '01777777777771400146000L' != '031400146000'
>> >>>> >> >
>> >>>> >> >
>> >>>> >> >
>> >>>> >> > ======================================================================
>> >>>> >> > FAIL: ColorTypeTest.test_webstyle
>> >>>> >> >
>> >>>> >> >
>> >>>> >> > ----------------------------------------------------------------------
>> >>>> >> > Traceback (most recent call last):
>> >>>> >> > File
>> >>>> >> > "/usr/lib/python2.5/site-packages/pygame/tests/color_test.py",
>> >>>> >> > line
>> >>>> >> > 458, in test_webstyle
>> >>>> >> > self.assertEquals (hex (c), hex (0xCC00CC11))
>> >>>> >> > AssertionError: '0xffffffffcc00cc11L' != '0xcc00cc11'
>> >>>> >> >
>> >>>> >> >
>> >>>> >
>> >>>> >
>> >>>> >
>> >>>> > --
>> >>>> > Visit my blog at http://oddco.ca/zeroth/zblog
>> >>>> >
>> >>>
>> >>>
>> >>>
>> >>> --
>> >>> Visit my blog at http://oddco.ca/zeroth/zblog
>> >>
>> >>
>> >>
>> >> --
>> >> Visit my blog at http://oddco.ca/zeroth/zblog
>> >
>> >
>> >
>> > --
>> > Visit my blog at http://oddco.ca/zeroth/zblog
>> >
>
>
>
> --
> Visit my blog at http://oddco.ca/zeroth/zblog
>