Hello.

I am trying to "git format-patch" for allegro5 and i have encounter a small
problem with trailing white space. My editor is set to remove the trailing
white space and i was so sure( Oh how wrong i was ! ) that every other editor
in the world does it. Well who the hell needs a bunch of white space characters
on an empty line.
And now when i formated the patch i noticed that the files i was working with
had this white space trailing on almost every empty line. The patch is now plagued
with lines like this:

Not only is the patch unnecessary large but it is also harder to look trough
and see what was really done.

Does allegro have any kind of guidelines regarding white space trailing in the code.
Should it be removed? Or should it be kept as the original author wrote it?

I am asking because it will be a real pain to return the white space in the files
to original state. My GIT-KUNG-FU is not that good to remove this kind of problem( if it is possible).
And my patch looks ugly.

We prefer the whitespace changes to be their own commits (although we rarely do them unless the whitespace is really bad). It's not particularly hard to split up a commit though:

First, undo the commit (this'll keep the changes):

git reset HEAD~1

Then use the interactive add mode.

git add -p <optionally put the files you meant to change here>

That'll put you in a mode where you'll go through every diff hunk. Press y if you want to stage it (i.e. the changes you really meant to do), or n if not. After that's done you can do:

git commit -m "Your commit message"

After that's done you'll still have your whitespace changes... I'd probably avoid committing them, but you can if you want.

If that gives you too much trouble, then don't worry too much we can do the split for you as well.

@bamccaig

I do use git, but I haven't really had the need for its more advanced features. I rarely need any more than git branch and git merge. If git can somehow magically remove whitespace changes from a commit that's cool and its news to me.

You may be able to hand edit the patch and literally remove whole swaths of ws changes.

Woah man, are you from the future?

{"name":"611906","src":"\/\/djungxnpq2nug.cloudfront.net\/image\/cache\/7\/2\/72e00ab6eb7bf3b17b698d64d215eedd.png","w":1920,"h":1080,"tn":"\/\/djungxnpq2nug.cloudfront.net\/image\/cache\/7\/2\/72e00ab6eb7bf3b17b698d64d215eedd"}

I'm looking at your patch now...

Seems there are options for dealing with white space changes ;

Yes, they are. Unfortunately.

But then i could just name the new functions something like:

al_blit_bitmap
al_blit_tinted_bitmap
al_blit_stretched_rotated_bitmap_region

and so on to keep the current al_draw_bitmap routines untouched.

Thank you Edgar for your suggestion and wording my intention with this patch correctly.
As my english is self-taught, sometimes its hard to write down exactly what i want.

Yes. The whole point of this patch is to make the drawing functions naming and parameter list consistent.
But my propposed changes are actually breaking the compatibility that is why i like your naming of the functions better.
I am willing to completely rewrite it if we can come up with a reasonable naming and parameter order.

I am proposing to use your naming witch does not interfere with the current drawing functions:
al_draw_bitmap_all-sane-variations and al_draw_bitmap_region_all-sane-variations

And parameter order list:
bitmap, tint, sx, sy, sw, sh, cx, cy, dx, dy, dw or scalex, dh or scaley, angle, flags

Witch would make the longest function name:

al_draw_bitmap_region_tinted_stretched_rotated( bmp, tint, sx, sy, sw, sh, cx, cy, dx, dy, dw, dh, angle, flags );

But i am realy fine with anything that is consistent.

Current naming and parameter order list in my patch ( breaks compatibility ):

#include <std_disclaimer.h>

Side question: Do... you actually need GIF support?

I mean, yeah yeah, lots of people on forums say that and they're often wrong and need to just STFU and answer the question...

But,

High-level question: do you really need GIF support? Are you using it for animations? Could you implement animations a different way? Because the only real use-case I can think of for GIF in >2010 is for an image editor or conversion tool and the thing is... Allegro is a bad candidate for that because Allegro is designed for games--not tools. It fails on all kinds of edge cases of images that if you were simply making a game, you'd already have complete control of file formats and just use a different file format, or the same format, but with a different internal header that works. That is, is Allegro fails on some variation of BMP/GIF/PNG, you simply convert all your files to be a workable format.

PNG is basically the only file format anyone uses anymore. Even BMP's are fat, and, may even be slower than PNG because compression increases the speed you can suck them from the hard drive while modern CPUs can easily run decompression. (That's the entire premise for NTFS compression, btw. And we've been using it since... the 2000's. Hard drives are slower than CPUs, so make the files smaller and decompress them once they hit the CPU.)

So I'm not sure if I'm illustrating my point effectively but if this is a game, you should be able to control your file formats and GIF doesn't add any features that other, better, formats don't already have. Except animation... for which I really wish APNG (animated PNG) would become a wildly accepted format. But for a game, there's almost zero situations (that I can think of) where you'd want to intentionally use the GIF format.

But if it's a tool (I'm speaking from experience here!) Allegro is going to bite you in the butt in strange places. Like loading BMP's which can have tons of different header styles and Allegro fails on some of them--even ones that GIMP outputs. Even if it's just one type, end-users aren't going to appreciate "It's not in my control, it's Allegro's fault." when your tool can't load every image on their computer. They just need it to work. And in that case, you should use a dedicated image loading library. Because Allegro is focused on making games, not making every edge-case image format work--unlike an image library.

Ok. Here it is.

This is how the function names and parameter list looks like:

8 years too late but never a day too soon to fix old mistakes.

Izual,

That's closer to what I had in mind, but there's an important problem. cx and cy should not be the second parameter in draw bitmap or the fifth in draw bitmap region. It should come after dw,dh.

So that you always have

al_draw_bitmap_X(bmp , dx , dy , ...)
al_draw_bitmap_scaled_X(bmp , dx , dy , dw , dh , ...)

and

al_draw_bitmap_region_X(bmp , sx , sy , sw , sh , dx , dy , ...)
al_draw_bitmap_region_scaled_X(bmp , sx , sy , sw , sh , dx , dy , dw , dh , ...)

Also, I have a question, are cx and cy pivot points in the source bitmap?

SiegeLord - the destination width and height would be assumed to be pre-rotation.

stretched vs scaled is obvious in their naming. Scaling uses factors and stretching involves dimensions.

Consistency is key. And with autocomplete these days you can type al_draw and then select which one you want and it will give you the parameters.

Anyway, this could all be hidden under ALLEGRO_UNSTABLE. And we could easily deprecate the old style drawing functions and have them forward themselves to the new ones. It could all happen rather painlessly. The only thing that would change is that you would get deprecation warnings when defining ALLEGRO_UNSTABLE.

My 2¢

@Edgar
Yes. cx,cy is a point on the source bitmap. This point is drawn at the dx,dy position and the bitmap is rotated around this point.
That is why i placed it together with the parameters that deal with the source bitmap. It seemed logical to me.

@SiegeLord
Thank you for your answear and stating the reasons. Just wanted to add a little bit to allready a great library.