etcpak icon indicating copy to clipboard operation
etcpak copied to clipboard

Misleading naming conventions regarding channel ordering

Open stohrendorf opened this issue 2 years ago • 2 comments

The encoding expects a channel ordering of BGRA, whereas every function argument or function name uses RGBA. Even more confusing, the decoding returns RGBA, and not BGRA, which means that the semantics of the data suddenly changes when doing a simple encode/decode roundtrip (given you could put in in-memory data as mentioned in #33), as the channel ordering gets changed.

stohrendorf avatar Jan 15 '23 01:01 stohrendorf

This is true, but what is the issue here?

wolfpld avatar Jan 15 '23 16:01 wolfpld

It makes it error-prone to write and understand code. For example,

    const uint8_t b = *data++;
    const uint8_t g = *data++;
    const uint8_t r = *data++;
    data++;

    const int dr = a[bid][0] - r;
    const int dg = a[bid][1] - g;
    const int db = a[bid][2] - b;

can easily be considered a bug when one assumes the data is in RGBA order, as every identifier suggests, or it can lead to wrong channel extraction when writing some custom code.

The issue here is that the variables do not contain what they say.

stohrendorf avatar Jan 15 '23 20:01 stohrendorf