|
PSA/RFC: Changing projection transform to be bitmap local state |
SiegeLord
Member #7,827
October 2006
|
tldr: There's a question at the end of this wall of text for people using the projection transform API in 5.1. If you don't use it, it only affects you if you're programming on Android/iOS, which will send a few more DISPLAY_RESIZE events. For people who tried to use Allegro 5.1's projection API, you may have noticed how al_get/set_projection_transform act a little differently from al_use/get_current_transform in that it's display local. It's a bit more than that, since to be at all useful it gets adjusted every time you call al_set_target_bitmap (and in a dozen other cases) to be orthographic (i.e. the default projection where x/y coordinates directly correspond to pixel coordinates of the bitmap). This meant that the only practical way of using the projection transform API was to reset it to something you wanted every time you draw something. There is in fact a bug about this http://sourceforge.net/p/alleg/bugs/349/ submitted by myself 3 years ago which involved the font addon not properly restoring it after it was clobbered by an intra-addon al_set_target_bitmap call. After a year of on and off work (it wasn't a priority), I managed to change it to be a bitmap local state. Now, it is acts essentially the same as the regular bitmap transform, and it preserves its set value at in all situations except two:
I think it works pretty good, actually and resulted in some simplification in the internals' code. Some other benefits of this change include these projection transforms actually working properly for sub-bitmaps (they did not work reliably before this change). This mostly affects people who use projection transforms (naturally), but it also affects people who don't in the following way:
Now here's a question: the projection API is 5.1 only, so in principle we do reserve to change it at any time, but I am wondering how big of a deal it is (as it has been in 5.1 basically since the beginning) to people who actually use it. Here's the migration strategy if the current API is removed: al_get_projection_transform(display) changes to al_get_current_projection_transform(). This is approximate, as you might need to select the bitmap you want to grab the projection transform from using a al_set_target_bitmap call. al_set_projection_transform(display, &trans) changes to al_use_projection_transform(&trans) (same caveat as above). In fact, that caveat is pretty strong as you really need to examine what semantics you were going for when you were using the current functions. In the common case of drawing to the backbuffer, however, the changes above are correct. That said, in principle you should be able to remove many al_set_projection_transform calls altogether if all you were doing is resetting them every frame. Now you can just set it and forget it (except for the two cases outlined above). So would you rather we kept the old API for compatibility (with the full understanding that it might silently break your current code) or remove it, and have the compiler guide you where you need to re-examine what you're doing? If you want to glance at the implementation, check out: https://github.com/liballeg/allegro5/pull/13 "For in much wisdom is much grief: and he that increases knowledge increases sorrow."-Ecclesiastes 1:18 |
Thomas Fjellstrom
Member #476
June 2000
|
You know what would be really nice? `al_get_projection_transform(ALLEGRO_BITMAP *)` Having to actually set the current target first is a bit round about. Though I'm not actually sure thats someone people will actually use a lot (will people often want to get the transform when it isn't being drawn to??) Given its a 5.1 thing, I don't think we need to care too much about providing compatibility code for it. I say that as someone who actually uses this so it'll be a little annoying to "fix" but it may make things more sane later. -- |
SiegeLord
Member #7,827
October 2006
|
That's a good point. Obviously I'm mimicking the al_use/get_current_transform, but that is that way for historical reasons (the normal transform was also display local when it was introduced). I think we're pretty happy about the normal transform being bitmap local, so maybe this isn't as important anymore... Perhaps I could also add a al_get_bitmap_transform and al_get_bitmap_projection_transform? "For in much wisdom is much grief: and he that increases knowledge increases sorrow."-Ecclesiastes 1:18 |
Polybios
Member #12,293
October 2010
|
I haven't used the projection transforms, but it all sounds very reasonable to me. Besides, straightening out the API before a 5.2 release is the right thing to do, IMHO. There would be no need for all the "unstable" warnings if the advantage of having those was not ...err... taken advantage of, i.e. being able to break compatibility in the 5.1 branch. |
Elias
Member #358
May 2000
|
Polybios said: Besides, straightening out the API before a 5.2 release is the right thing to do, IMHO. There would be no need for all the "unstable" warnings if the advantage of having those was not ...err... taken advantage of, i.e. being able to break compatibility in the 5.1 branch. I agree. So my opinion, just go ahead and change it, that's what the unstable branch is about One thing I'm wondering - what is the relation of al_use_transform and al_set_projection_transform? I assume we don't really need al_set_projection_transform anymore at all and could just require composing any transformation with a projection before passing to al_use_transform. Which of course would be a huge breakage so we should not do it. But we should make it clear in the documentation that they are essentially the same and you can use just one and not the other if you like (and Allegro will do an internal al_compose_transform for you somewhere). Well, if I'm right about this, I didn't actually check the code. -- |
Thomas Fjellstrom
Member #476
June 2000
|
Well now that they are both bitmap local, it doesn't make a whole lot of sense to have both. -- |
SiegeLord
Member #7,827
October 2006
|
As implemented, the differences are as follows:
Aside from those two differences, I believe you can keep the regular transform at identity, and do all transformations through the projection transform (EDIT: Doing the opposite is less reliable, as the projection transform does revert back occasionally and the aforementioned bitmap hold issue applies). That said, it's worthwhile to keep them both, as it's a pain to deal with the projection transform all the time if you're not really using this feature. "For in much wisdom is much grief: and he that increases knowledge increases sorrow."-Ecclesiastes 1:18 |
Thomas Fjellstrom
Member #476
June 2000
|
Hmm, yeah. I was mainly thinking in terms of the programmable pipeline. There isn't really any matrix state. You pass in whatever matrix you need and apply it in the shaders. -- |
SiegeLord
Member #7,827
October 2006
|
Allegro passes in the combined matrix, so from the shader's POV, it indeed doesn't matter what you use. "For in much wisdom is much grief: and he that increases knowledge increases sorrow."-Ecclesiastes 1:18 |
|