manojkumarsure / quadra

Automatically exported from code.google.com/p/quadra
0 stars 0 forks source link

remove Bitmap class #74

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
There seems to be two uses for the Bitmap class. One of them is to store
images that are loaded, and SDL_Surface works just fine for that (I added a
Image::new_surface() method that create an SDL_Surface from the Image).

The other thing is to do source clipping on other Bitmap (I *think*, can't
be sure yet, since it actually takes a void pointer to raw pixel data).
This is rather iffy, because it makes it pretty hard to understand what is
actually happening, and how a Bitmap can suddenly be not good anymore (for
example, there's Bitmap::reload() that I wanted to remove, since I thought
it was simpler just to delete the old one and put the new one in place, but
that invalidated a whole bunch of sub-Bitmaps and made the game crash).

We should convert all of this to be just SDL_Surface, and use the source
rectangle of SDL_BlitSurface to do the work (made available by the new
version of Video_bitmap::put_surface I added recently). For blitting to a
Bitmap, we can use SDL_SetClipRect() to restrain the blit.

We should add a function to calculate the intersection of two SDL_Rect,
this would allow us to have an SDL_Rect where we had those nasty Bitmap,
and quickly calculate the SDL_Rect to use for Video_bitmap::put_surface.

Original issue reported on code.google.com by pphaneuf on 20 Jun 2008 at 5:47

GoogleCodeExporter commented 9 years ago
Stéphane expressed his disagreement with this in person, yesterday. I'm not 
100% sure
if I understand correctly, but his position seemed to be that it was good to 
have our
own abstractions.

I clarified my own feelings, which is that maybe it'd be okay to have a wrapper 
class
around SDL_Surface, yes (although I'm usually finding it okay to deal with 
directly,
maybe use a smart pointer to do SDL_FreeSurface automatically), but Bitmap is 
not a
wrapper, it's its own brand of evil, using way too much of the "create from 
existing
pixels", making the ownership and lifetime of the whole thing almost impossible 
to
understand.

Since it looks like most place that has a Bitmap to do this *also* has a 
pointer to
the thing that has the *real* bitmap, I think that if they had an SDL_Rect to 
pass to
Video_bitmap::put_surface would do just fine.

When I mentioned the refcounting that SDL_Surface could do, Rémi pointed out 
that
maybe we could use that, but I still think that's iffy. Shared ownership isn't 
really
what we want here (which is the #1 reason I didn't introduce shared_ptr, 
actually, I
think it's more dangerous than anything if you don't understand what you're 
doing,
and I sure don't!), when we get rid of the background image (which is the usual 
thing
we have in a Bitmap), what the others have is really a weak reference to that 
one, it
should go away now, and they should start doing whatever they were doing with 
the new
one (AKA, what they want is access to the *current* background surface, not a 
copy
of, or a reference to whatever happens to be the background surface when they 
were
created).

Original comment by pphaneuf on 23 Jun 2008 at 5:11

GoogleCodeExporter commented 9 years ago
Update on this: SDL2 works best if you can put your pixel data in "textures", 
that you pretty much can't really access directly (you can lock them, but if 
you do, that overwrites the entire texture, as it does not download the 
existing texture from video memory, and will do a full upload afterward).

So the whole business of having pointers inside video memory and such, that's 
just not really the best (whether we wrap SDL or not), and things like 
Video_bitmap and Bitmap are all about the pixel access...

At the moment, the SDL2 port (currently at 
https://github.com/pphaneuf/quadra/tree/sdl2 but watch out, this is unstable, I 
rebase it often!) uses a single texture for the whole screen, and does a full 
update on every frame (this doesn't horribly suck probably because 640x480 is 
nothing nowadays), but it'd be neat if we could pre-render all the tetrominos 
(in each orientation) in a texture, ahead of time, then use the fast texture 
blitting to put them on screen. Anyway... :-)

Original comment by pphaneuf on 11 Feb 2014 at 5:39

GoogleCodeExporter commented 9 years ago
Oh, there's a SDL_IntersectRect in SDL2 now, too, BTW.

Original comment by pphaneuf on 11 Feb 2014 at 5:41