wolfpld / etcpak

The fastest ETC compressor on the planet
Other
229 stars 41 forks source link

Misleading naming conventions regarding channel ordering #34

Open stohrendorf opened 1 year ago

stohrendorf commented 1 year ago

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.

wolfpld commented 1 year ago

This is true, but what is the issue here?

stohrendorf commented 1 year ago

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.