1

I've my R, G and B arrays which are uint8_t

uint8_t * R, * G, * B, * A;
int stride_r, stride_g, stride_b, stride_a;
getRGB(img, &R, &G, &B, &A, &stride_r, &stride_g, &stride_b, &stride_a);
// All the strides are equal and the img is img_width and img_height dimension
int RBG[stride_r * img_height];

SO my question is will the following work:

for(int i=0; i<stride_r * img_height; ++i)
{
    RGB[i] = R[i] << 24 + G[i] << 16 + B[i] << 8 + A[i];
}
pr0py_s
  • 171
  • 9
  • 1
    addition has more priority than shift. So this code won't work as expected. You need to put parentheses around shift operations. Also, to make sure, it is better to use uint32_t instead of int as int has platform dependent size. – rhaport Sep 01 '20 at 08:36
  • I was wondering if I should and it with ff – pr0py_s Sep 01 '20 at 08:37
  • RGB needs to be an `unsigned int` array instead of `int` for those shift operations to be correct. And preferably `uint32_t` to be most portable, else you are technically in undefined behavior (probably will work anyway). – selbie Sep 01 '20 at 08:37
  • 1
    Also, don't be surprised if it renders with inverted colors. Sometimes the encoding in memory is really ABGR or BGRA. You experiment with the byte ordering until it looks right. – selbie Sep 01 '20 at 08:40

2 Answers2

1

You are on the right track. However, rather than adding the four component bytes into the four-byte integer, you need to combine them using bitwise or operations:

for(int i = 0; i < stride_r * img_height; ++i)
{
    RGB[i] = (R[i] << 24) | (G[i] << 16) | (B[i] << 8) | A[i];
}

Also, bear in mind that, on many/most platforms, the components are actually stored in 'reverse' order (in hexadecimal format, 0xaaBBGGRR); so, if that is the case on your system, then you would need, instead:

for(int i = 0; i < stride_r * img_height; ++i)
{
    RGB[i] = (A[i] << 24) | (B[i] << 16) | (G[i] << 8) | R[i];
}

Also, as mentioned in the comments, you should really make the RGB an array of unsigned integers, preferably of fixed 32-bit size, using the uint32_t type.

Adrian Mole
  • 49,934
  • 160
  • 51
  • 83
1

Here are a couple of things to take into consideration:

  • are we really sure that the stride of all arrays is the same?
  • we should be using a type that really does have 32 bits, int can be 16 bits
  • we should be using size_t whenever dealing with array indices
  • it's important that we are ordering our array as RGBA, at least our variable name should reflect that
  • + has higher precedence than <<, we need to use parantheses to fix the code
  • we don't need to use +, we can use | as the shifted numbers have no bits in common
  • dynamically sized arrays like int RGB[non-constexpr size] are not part of the C++ standard, they are GCC/clang language extensions

Here is the code that would address all these problems:

assert(stride_r == stride_g && stride_b == stride_a && stride_r == stride_b);

size_t limit = stride_r * img_height;
std::unique_ptr<uint32_t[]> RGBA = std::make_unique<uint32_t[]>(limit);

for (size_t i = 0; i < limit; ++i) {
    RGBA[i] = (R[i] << 24) | (G[i] << 16) | (B[i] << 8) | A[i];
}
Jan Schultke
  • 17,446
  • 6
  • 47
  • 96