Multiplication could be harmful on portable platforms in a place which could be called for every pixel of the screen. Shifting is much cheaper. We need to check that gcc optimizes multiplication by 255 as '(m << 8) - m', or consider adding 7 there ;)<br />17.03.10, 08:35, "yotam barnoy" <yotambarnoy@gmail.com>:<blockquote style="border-left:1px solid #CCCCCC;margin:0pt 0pt 0pt 0.8ex;padding-left:1em;"><div dir="ltr">Hey guys<div><br /></div><div>I came across our implementation of pixelFormat.colorToRGB() and colorToRGBA() implementation and it seems incorrect to me. Currently it's</div><div><br /></div><div><div><span class="Apple-tab-span" style="white-space: pre; "> </span>inline void colorToRGB(uint32 color, uint8 &r, uint8 &g, uint8 &b) const {</div><div><span class="Apple-tab-span" style="white-space: pre; "> </span>r = ((color >> rShift) << rLoss) & 0xFF;</div><div><span class="Apple-tab-span" style="white-space: pre; "> </span>g = ((color >> gShift) << gLoss) & 0xFF;</div><div><span class="Apple-tab-span" style="white-space: pre; "> </span>b = ((color >> bShift) << bLoss) & 0xFF;</div><div><br /></div><div>The problem is, you can't ever restore the intended r,g,b values from the original 16-bit color. For example in R5G5B5  mode, 0x7FFF, which is supposed to be the brightest white, will only become 248,248,248 instead of 255,255,255. What we really need to do is </div><div><br /></div><div>r = (((color >> rShift) & rMask) * 255) / rMax();</div><div><br /></div><div>in order to translate the values correctly. rMax() should probably be saved in the struct for speed.</div><div><br /></div><div>The same criticism applies to the conversion in colormasks.h.</div><div><br /></div><div>Feedback?</div><div><br /></div><div>Yotam</div></div></div></blockquote>