LeColor - representation and performance impact
Opened this issue · 3 comments
Reviewing the code while updating the Wavefront Object file parser ... my brain says some things are wrong with the implementation of the LeColor class.
- Representation
Now the colors are stored by individual uint8_t components and no longer by uint32_t words.
The basic operation the renderer is to do color blits, which are copying a uint32_t from one location to another. In order to perform that basic task, now the execution go through two C++ '=' operators which deal with individual components, potentially decomposing and recomposing the color at each transfer.
I don't know if one can assume the compiler to be 'smart' as there are casts (forcing to uint8_t) and no guaranty of data alignment in the class.
For these reasons I would prefer the data stored in a uint32_t and r, g, b, a are "accessors" or "fields" of an union to components.
- Format
Current format is:
uint8_t b;
uint8_t g;
uint8_t r;
uint8_t a;
Which is super weird to me.
Windows bitmaps and native contexts, store their colors in:
uint8_t r;
uint8_t g;
uint8_t b;
uint8_t a;
If you think "big endian":
uint8_t a;
uint8_t b;
uint8_t g;
uint8_t r;
So I think it works right now because the mapping match (with = operators) but it is constantly entangling / detangling the colors and can make the non asm implementation way slower than it should be. I did not reprofiled the execution speed since this change.
Can I have your thoughts on that @m0ppers ?
I was just as surprised as you are but it is actually the correct format for windows. there will not be any conversion:
https://msdn.microsoft.com/en-us/library/windows/desktop/dd162938(v=vs.85).aspx
But I think your argument is super valid. My idea was: "Ok the critical path is implemented in asm anyway so it is not a big problem". But of course the non asm version is now slower than it should be (not sure if this is a problem right now - is anything targeting non SSE windows/linux right now?).
I don't really have a solution right now to be honest. On the other hand it is probably good to have an idea if and how big the performance overhead is before digging into it.
also if you think this is a big problem right now it is probably worthwhile to look how others solved this. If you think that this is a problem to be solved right now because it bugs you I would look into that as I created the mess ;)
@m0ppers
The windows RGBQUAD structure is used to represent colors ... for a palette!
I'll make some checks with a hex viewer but I have still a problem to believe that.
It seems you are totally right. Weird stuff though :-)