liballeg / allegro5

The official Allegro 5 git repository. Pull requests welcome!
https://liballeg.org
Other
1.84k stars 281 forks source link

Why doesn't `ALLEGRO_PRIM_COLOR_ATTR` respect the `storage` declaration provided in `ALLEGRO_VERTEX_ELEMENT`? #1574

Open ghassanpl opened 1 month ago

ghassanpl commented 1 month ago

As far as I understand, there should be no reason for it not to. Non-float4 color data is supported by most modern cards, and it would be easy to error-out in case it's not supported for some reason.

If I could build allegro from source

I'd make a change at: https://github.com/liballeg/allegro5/blob/master/addons/primitives/prim_opengl.c#L158

ALLEGRO_PRIM_POSITION, ALLEGRO_PRIM_TEX_COORD and user attributes use convert_storage to get the parameters for glVertexAttribPointer to setup the vertex declaration, but the call for ALLEGRO_PRIM_COLOR_ATTR at the link above does not.

Analogous for DX.

There might be a reason this is not done, so feel free to correct me :)

SiegeLord commented 1 month ago

ALLEGRO_PRIM_COLOR_ATTR is meant to be used with arrays of ALLEGRO_COLOR, which has a specific layout. For other color formats you'd use one of the custom attributes.

ghassanpl commented 1 month ago

ALLEGRO_PRIM_COLOR_ATTR is meant to be used with arrays of ALLEGRO_COLOR, which has a specific layout. For other color formats you'd use one of the custom attributes.

But then I wouldn't be able to use the default primitive shader, right? Changing the storage of the other default attributes isn't an issue, why is color different? I.e. is there a reason Allegro needs me to use ALLEGRO_COLOR in my own vertex formats?

SiegeLord commented 1 month ago

That's fair. This can't be fixed for this existing constant for backwards compatibility reasons, but we could add a ALLEGRO_PRIM_COLOR_FLEX_ATTR or something to accomplish what you want.