PSA/RFC: Changing projection transform to be bitmap local state
SiegeLord

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:

  • The backbuffer bitmap's projection transform reverts back to orthographic when it is resized (simultaneously with the al_acknowledge_resize call).

  • A video bitmap's projection transform reverts back to orthographic when it is converted to a memory bitmap (those only support orthographic transforms).

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:

  • iOS now sends the resize events somewhat more often (this shouldn't be a big issue)

  • Android now sends a resize event after ALLEGRO_EVENT_DISPLAY_RESUME_DRAWING. This is to allow ALLEGRO_EVENT_DISPLAY_RESUME_DRAWING event to not change the backbuffer's transform (instead, the user simply needs to handle the resize event and everything will happen automagically).

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

Thomas Fjellstrom

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

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?

Polybios

I haven't used the projection transforms, but it all sounds very reasonable to me.
If there was a al_get_projection_transform(ALLEGRO_BITMAP *), there should probably also be a al_get_transform(ALLEGRO_BITMAP *).

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
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 :P

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

Well now that they are both bitmap local, it doesn't make a whole lot of sense to have both.

SiegeLord

As implemented, the differences are as follows:

  • The projection transform isn't pre-applied when bitmap drawing is held... it seems a bit of a pain to change as we'd have to keep around the w and z coordinates for each bitmap cache vertex... it likely will have some effect on the memory throughput of bitmap drawing.

  • For the fixed function pipeline, the projection transform is sent to the GL_PROJECTION, while normal projection is set to GL_MODELVIEW. There are subtle differences between those.

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.

Thomas Fjellstrom

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

Allegro passes in the combined matrix, so from the shader's POV, it indeed doesn't matter what you use.

Thread #615257. Printed from Allegro.cc