[Scummvm-devel] colorToRGB() implementation

yotam barnoy yotambarnoy at gmail.com
Thu Mar 18 09:10:13 CET 2010


OK thanks for clearing it up Max. The way I came across it is that in
implementing 16-bit support on the PSP, I used a colormasking feature of the
hardware, which takes a 32 bit ARGB value as its parameter. Converting the
way that we do doesn't give the proper ARGB value causing the PSP not to
mask it, so I had to look into what the problem was.

I guess since ScummVm works with at most 16 bits, this isn't really a big
issue in general, since any conversion of a 16-bit format to RGB and back to
another 16bit format should result in minimal loss. As such, I don't see a
reason to rewrite the pixelFormat code unless someone else can think of
one.

Yotam


On Thu, Mar 18, 2010 at 3:36 AM, Max Horn <max at quendi.de> wrote:

> Hi Yotam,
>
> Our implementation of pixelFormat.colorToRGB is not incorrect. It converts
> a given color to an RGB triple (r,g,b) with the property that converting
> that RGB triple back to a color gives the same color as we started with. As
> such, it conforms to its specs to the letter. And it does computations very,
> very fast.
>
> However, you may have extra requirements on the RGB triple which make this
> particular conversion undesirable; in the end, it's a trade-off between
> speed and accuracy that has been made here. Whether this trade off is a good
> one depends on context, and of course we can re-evaluate it. This is why the
> ColorMask documentation says this:
>
> "
>  The k*Bits and k*Shift values can be used to extract R,G,B. I.e. to get
>  the red color component of a pixel, as a 8-bit value, you would write
>
>  R = ((color & kRedMask) >> kRedShift) << (8-kRedBits)
>
>  Actually, instead of the simple left shift, one might want to use somewhat
>  more sophisticated code (which fills up the least significant bits with
>  appropriate data).
> "
>
> This comment refers precisely to the "trick" Marcus already explained.
>
> So, it might be "nicer" if a fully saturated color in 565 format is
> converted to the RGB triple (255,255,255) instead of (248,252,248). But then
> again, what "nice" means really depends on the context; and colorToRGB resp.
> the ColorMask class never claimed that fully saturated colors are mapped to
> (255,255,255). Maybe it is desirable to do so, in that case, we can document
> this as a mandatory requirement (in the "spec", i.e. source comments for the
> class/functions) and modify the code accordingly.
>
> Note that so far it seems that the difference between those two values does
> not really matter :-). Now, everything having been said, I don't mind if you
> want to rewrite the code to use a different scheme, e.g. based on what
> Marcus wrote.
>
>
> Cheers,
> Max
>
> ------------------------------------------------------------------------------
> Download Intel® Parallel Studio Eval
> Try the new software tools for yourself. Speed compiling, find bugs
> proactively, and fine-tune applications for parallel performance.
> See why Intel Parallel Studio got high marks during beta.
> http://p.sf.net/sfu/intel-sw-dev
> _______________________________________________
> Scummvm-devel mailing list
> Scummvm-devel at lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/scummvm-devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.scummvm.org/pipermail/scummvm-devel/attachments/20100318/506ea36c/attachment.html>


More information about the Scummvm-devel mailing list