<div dir="ltr">If it's efficiency we're after, then the multiplication is a small part of the cost - division is much more expensive (about 4 times more on MIPS).<div><br></div><div>The problem is that we need to multiply by more correct ratios than our shifting simplifications allow. For example to convert a 5 bit channel to 8 bits, we need to multiply by 255/31 = 8.225 instead of 8, which is what we currently do.</div>
<div><br></div><div>Since the number of cases we're using is limited (reduced a,r,g,b range from 1 bit to 7 bits, with some cases being trivial) , we could provide specialized code (perhaps in templates?) to do the conversion. 8.23 means roughly an extra unit every 5 jumps, so code like</div>
<div> if (inputbits > 5) outputbits++;</div><div> if (inputbits > 10) outputbits++;</div><div> if (inputbits > 15) outputbits++;</div><div> ...</div><div>
outputbits += inputbits<<3;</div><div><br></div><div>should do the trick (obviously paying attention to the exact point where addition of a unit is needed). This should also be better than a lookup table.</div>
<div><br></div><div>Yotam<br><br></div><div><br></div><div><br><div class="gmail_quote">On Wed, Mar 17, 2010 at 9:35 AM, Vladimir Menshakov <span dir="ltr"><<a href="mailto:whoozle@yandex.ru">whoozle@yandex.ru</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">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" <<a href="mailto:yotambarnoy@gmail.com" target="_blank">yotambarnoy@gmail.com</a>>:<div><div></div><div class="h5"><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 style="white-space:pre"> </span>inline void colorToRGB(uint32 color, uint8 &r, uint8 &g, uint8 &b) const {</div>
<div><span style="white-space:pre"> </span>r = ((color >> rShift) << rLoss) & 0xFF;</div><div><span style="white-space:pre"> </span>g = ((color >> gShift) << gLoss) & 0xFF;</div><div><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></div></div></blockquote></div><br></div></div>