[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]

Re: [pygame] [BUG] SRCALPHA - unexpected behaviour.



Hi,

I went through the surface.c and found a bunch of problems with it's
error checking.

It turns out you need to raise exceptions from init methods like this:
        RAISE (PyExc_SDLError, SDL_GetError ());
        return -1;

I only checked surface.c for this bug... so a TODO is to check all the
pygame source for this bug, as well as add tests for all the error
inputs.

Another TODO is to check for endian compliance... I'm not sure if the
masks are set correctly in there for big endian.  There's no checks in
the source for endian anyway.

Pretty much none of the pygame functions use keyword arguments... but
that would be nice if it changed.  We probably should add keyword
arguments, just to make it sane.

However I haven't added your test for keyword arguments yet.  I'm not
sure it's a good idea or not to add keyword argument support to all
the functions this late in the release process.  We'd need a bunch
more tests, and need to measure any performance difference adding
keyword arguments would make.  If keywords don't add any performance
change for the case when they are not used, then they should
definitely go in.  If it makes games drop from say 30fps to 20fps...
then they  should definitely not go in.  We need to test it.


On 8/20/07, Brian Fisher <brian@xxxxxxxxxxxxxxxxxxx> wrote:
> I disagree that an error is bad in that case.
>
> It's exactly because the video subsystem may make choices for you that
> I think raising an error is the right thing to do. Raising an error
> will make it so that when your art doesn't draw right, the program
> will know. Also, raising an error doesn't prevent you from handling
> that case as a programmer either - you can always catch the exception
> and try again with no alpha.
> Raising an error is the pythonic thing to do - it exposes problems
> instead of hiding them. Finding hidden problems is slow and not fun.
>
> Moreover, with respect to the "flags are just hints" point, as a user
> of pygame, I can confidently say that when I say I want alpha
> blending, it is much more than a "hint", I am demanding alpha
> blending. All the rest of the flags though, are still like "hints",
> and the patch doesn't change that (it doesn't confirm HWSURFACE, for
> instance)
>
> Finally, 16-bit surfaces support alpha, no problem. Running the
> attached script to see for yourself.
>
>
> On 8/19/07, René Dudfield <renesd@xxxxxxxxx> wrote:
> > Thanks Brian.
> >
> > I think there is one issue with raising an error though.  The flags
> > are just 'hints' and not all installations support each flag.
> > SRCALPHA is only supported on 32 bit surfaces (I think).  So if the
> > underlying video subsystem decides that it should return 16 bit
> > surfaces (for example), then the SRCALPHA flag should correctly have
> > no effect.
> >
> >
> > On 8/20/07, Brian Fisher <brian@xxxxxxxxxxxxxxxxxxx> wrote:
> > > Being able to have this succeed without error:
> > >    surf = pygame.Surface((100, 100), pygame.SRCALPHA)
> > > and get a surface that doesn't have SRCALPHA set seems like a clear
> > > and obvious bug to me. I'd say the best fix would be to always use
> > > SRCALPHA if requested, and raise an error if the request couldn't be
> > > satisfied. attached is a patch to svn pyame that seems to do that (it
> > > passes the test in svn for this, anyways)
> > >
> > > ------
> > >
> > > ... However I'm also seeing that this:
> > >    surf = pygame.Surface((100, 100), flags=pygame.SRCALPHA, depth=32)
> > > is returning a surface without alpha set. So it seems like the Surface
> > > constructor doesn't support keyword arguments at all.
> > >
> > > I've tested python 2.3 on windows, pygame 1.7 and svn pygame. I've
> > > also tried python 2.5 on macOSX, pygame 1.7
> > >
> > > I have no idea how to fix this, but I've attached a patch to the surface test
> > >
> > >
> >
>
>