RGB color converting into 5:6:5 format

Question

This code is supposed to convert a RGB color to an hex in the 5:6:5 format. 5 bits for red, 6 bits for green, 5 bits for blue. I have no idea why this is not picturing the correct color.

Does anyone knows why?

int rgb(unsigned char r, unsigned char g, unsigned char b) {
    if (r < 0 || 255 < r || g < 0 || 255 < g || b < 0 || b > 255)
        return -1;

    int result;

    int red = r * 31 / 255;
    int green = g * 63/ 255;
    int blue = b * 31 / 255;


    //int result = (red << 11) | (green << 5) | blue;

    green = green << 5;
    red = red << 11;

    result = red | green | blue;


//tests
    printf("\nred: %x", red);
    printf("\ngreen: %x", green);
    printf("blue: %x\n", blue);
    printf("result: %x\n", result);

    return result;
}

Answer 1

After another look at your question I don't really know what you're asking about. Anyway, I'm leaving my answer in case you find it useful.

---

Your rgb(...) function takes three byte arguments - they have 8 bits each.

Let's take "red" component into account first. If you pass XXXX XXXX (8 bits) and want to convert them into a 5-bit equivalent representation, it's enough to shift the value right by 3 bits, so:

int red = r >> 3;

The value XXXXXXXX will be truncated in the place of the pipeline character:

XXXXX|xxx

so that only the bits marked with large Xes will be saved to the red variable.

The same goes for blue, and for the green component, you have to shift it right by two (8 - 6 = 2).

---

You probably want your function to work like this:

int rgb(unsigned char r, unsigned char g, unsigned char b) {
    if (r < 0 || 255 < r || g < 0 || 255 < g || b < 0 || b > 255)
        return -1;

    unsigned char red = r >> 3;
    unsigned char green = g >> 2;
    unsigned char blue = b >> 3;

    int result = (red << (5 + 6)) | (green << 5) | blue;

//tests
    printf("red: %x\n", red);
    printf("green: %x\n", green);
    printf("blue: %x\n", blue);
    printf("result: %x\n", result);

    return result;
}

Answer 2

Assuming 8-bit char, your unsigned char arguments must already be in the 0-255 range, so you don't need to check that. And the multiplication you're trying to use to scale the color components is probably not a good approach.

A better approach would be to AND each component with a mask to get the upper 5 bits (6 for green), shift them to the proper positions, and OR them together. When shifting, remember to account for the fact that you're using the upper bits... and for the last component, you won't need to AND with a mask because the unneeded bits are shifted out anyway. So this gets you something like this (as the only line in your function):

return ((r & 0xf8) << 8) | ((g & 0xfc) << 3) | (b >> 3);

(r & 0xf8) gets the upper 5 bits of r. These are then left shifted by 8 bits, so they move from positions 3..7 into 11..15.

(g & 0xfc) gets the upper 6 bits of g. Those are then left shifted by 3 bits, from 2..7 into 5..10.

b doesn't need to be masked... it's just shifted right 3 bits. Its upper 5 bits are then moved from 3..7 into 0..4, and its lower 3 bits are discarded when they're shifted out.

All those values are then ORed together to get your RGB 5:6:5 value, and returned.

Alternatively, if you prefer shifts over AND, you can use:

return ((r >> 3) << 11) | ((g >> 2) << 5) | (b >> 3);

You might also consider changing the return type to an unsigned 16-bit type and not worry about returning an error value (there isn't really any kind of error condition to check for here).

Answer 3

You need a function that shows you the binary contents, so that you can "count" the bits and better find errors. My approach added a rounding routine:

#include <stdio.h>
#include <math.h>

char* sprint_bin (unsigned a, unsigned count, char* bin)
{
    char* p = bin;
    unsigned i;
    unsigned mask = pow(2,count-1);
    unsigned b;
    for (i = 0; i<count; ++i)
    {
        b = (a & mask) ? '1' : '0';
        p += sprintf (p, "%c ",b);
        mask >>= 1;
    }
    return bin;
}


unsigned rgb(unsigned char r, unsigned char g, unsigned char b) {

    char bin[64];
    int result;

    printf("r:      %s\n", sprint_bin(r,8,bin));
    printf("g:      %s\n", sprint_bin(g,8,bin));
    printf("b:      %s\n", sprint_bin(b,8,bin));

    // masks
    unsigned red = (unsigned)(r & 0xF8) << 8;
    unsigned green = (unsigned)(g & 0xFC) << 3;
    unsigned blue = (unsigned)(b >> 3);

    // rounding
    if ((r & 4) && (r<0xF8)) red += 0x0800;
    if ((g & 2) && (g<0xFC)) green += 0x20;
    if ((b & 4) && (b<0xF8)) blue++;

    // 5:6:5
    result = red | green | blue;

    // test
    printf("red:    %s\n", sprint_bin(red,16,bin));
    printf("green:  %s\n", sprint_bin(green,16,bin));
    printf("blue:   %s\n", sprint_bin(blue,16,bin));
    printf("result: %s\n", sprint_bin(result,32,bin));

    return result;
}

int main ()
{
    rgb (0x81, 0x87, 0x9F);

    return 0;
}