tags:

views:

459

answers:

3

Hello there,

I have to convert a DWORD (unsigned long) RGBA to four int vars (R, G, B, and A) So far, I have this function to convert the 4 ints to a DWORD:

unsigned long RGBA2DWORD(int iR, int iG, int iB, int iA)
{
 return ((iA << 24) | (iR << 16) | (iG << 8) | iB);
}

How can I convert it back?

Something like

struct RGBA
{
 int R, G, B, A;
};

RGBA DWORD2RGBA(unsigned long dwColor)
{
 static RGBA tmp;
 //.......conversion process
 return tmp;
}

Any kind of help would be appreciated! :)

Thanks

+2  A: 

You can go the other way around like:

iA = rgb >> 24; iR = (0x00FF0000 & rgb) >> 16; iG = (0x0000FF00 & rgb) >> 8; iB = (0x000000FF & rgb);

Alex
Note that RGBA2DWORD is wrong - it's using logical rather than bitwise OR operators. See my answer below for the correct implementation.
Jason Williams
Oooops, yeah, just copy pasted from the original post.
Alex
@Alex if you find your answer to be wrong, please fix it. Otherwise i will have to downvote it.
Johannes Schaub - litb
Jesus, that was just a quote :D. Litb for homeland security
Alex
@Alex: The question isn't how it got there, but whether others, stumbling into this via google, will find an accepted answer that's correct or incorrect.
sbi
I, already, edited. Don't, you, see.
Alex
It's a tough crowd tonight, Alex :-)
Jason Williams
+2  A: 
Jason Williams
P.S. My union/struct example is a bit mixed-up. The order of the RGBA values will depend on whether you are running on a big-endian or little-endian CPU, so you may need RGBA or ABGR order in your struct to place the values in the correct locations. A little bit of experimentation (encode a 255,0,0,0 value and see what you get) will tell you which way around you need to do it on your target platform.
Jason Williams
Your union example is mixed-up in more than one way. The popular "union hack" is explicitly illegal in C. With a few exceptions, writing one member of an union and then reading another leads to undefined behavior. It might not work on an aggressively optimizing compiler. GCC, with its aggressive "type-punning" optimizations, spared the "union hack" for now (i.e. it will work with GCC), but I can't vouch for all compilers out there.
AndreyT
The question says "C++", and makes no mention of compiler or platform, so I gave a generalised answer. The purpose of unions is to allow you to interpret a single memory location as different data types. As you say, it'll be hard to find a compiler that doesn't suport this.
Jason Williams
The purpose of unions is to use one memory location to store different values *at different times*. However, the values are not allowed to "know" about each other. C and C++ are very explicit about one thing: it is illegal to write on member of the union and then read another. Once you write a member, it becomes "active" and you are allowed to read this member only, until you make another member "active" by writing it. This requirement has a number of exceptions, but what you do above is not one of them.
AndreyT
I do understand, that unions are often used for memory re-initerpretation in practice, but from the formal point of view that's an illegal hack, a "union hack".
AndreyT
I'm not arguing with that. Hack or not, it's a common practice.
Jason Williams
Well, to be fair, I do agree with Jason about "union hack" having a considerable practical value. For example, GCC supports "union hack" as a way to perform type-punning without running into optimizer problems. And, yes, we have to use it from time to time in real life. With my comment I was just trying to make a "useful sidenote", not to claim that Jason's solution is bad beyond repair. If you remember, "struct hack" was also illegal in C89/90, yet the technique was so useful that a feature to support it was added in C99.
AndreyT
While union hack is supported by at least GCC and i think it may be a valid solution if it only needs to work on a known set of compilers, i don't think GCC guarantees that all those chars in the struct are allocated without padding in between. You may need a `__attribute__((packed))` or something there.
Johannes Schaub - litb
Whenever you use a specific compiler on a specific platform to work with binary structures, you need to understand what packing will be used. And whenever you write any code, you need to test it to be sure that it behaves as you expect, so packing weirdness is unlikely to cause any great problem in practise. I'm not arguing the legality of this approach, I was simply pointing out a commonly used method. Hopefully my edit makes it slightly more palatable :-)
Jason Williams
+2  A: 

If I were you, I'd stick with multiplicative-additive operations in the packing/unpacking functions. Something like this

unsigned long RGBA2DWORD(int iR, int iG, int iB, int iA)
{        
  return ((iA * 256 + iR) * 256 + iG) * 256 + iB;
}

with a symmetrical unpacking function

RGBA DWORD2RGBA(unsigned long dwColor)
{        
  RGBA tmp; /* why did you declare it static??? */

  tmp.B = dwColor % 256; dwColor /= 256;
  tmp.G = dwColor % 256; dwColor /= 256;
  tmp.R = dwColor % 256; dwColor /= 256;
  tmp.A = dwColor % 256; /* dwColor /= 256; */

  return tmp;
}

Note that there's only one "magic constant" in the whole code.

Of course, if you have an external specification that is written in terms of bit patterns in the packed data, a version based on bit and shift opertions might be preferrable. Still

unsigned long RGBA2DWORD(int iR, int iG, int iB, int iA)
{        
  return (((((iA << 8) + iR) << 8) + iG) << 8) + iB;
}

RGBA DWORD2RGBA(unsigned long dwColor)
{        
  RGBA tmp; /* why did you declare it static??? */

  tmp.B = dwColor & 0xFF; dwColor >>= 8;
  tmp.G = dwColor & 0xFF; dwColor >>= 8;
  tmp.R = dwColor & 0xFF; dwColor >>= 8;
  tmp.A = dwColor & 0xFF; /* dwColor >>= 8; */

  return tmp;
}

has much less "magic constants".

Now you can wrap the repetivie actions/subexpressions in macros or, better, inline functions and arrive at very compact and readable packer/unpacker.

AndreyT
@Alex: Huh? No. There is absolutely *no* difference from the performance point of view betweein this and any other arithmetic solution offered so far. Where did you find the performance problems, care to elaborate?
AndreyT
Quite the opposite, this is arguably the most efficient solution so far, since the compiler will be able to store the only one (or two) "magic constant" in the register. The solutions with multiple "magic constants" will most likely force the compiler to generate machine instructions with embedded operands, which is notably worse performance-wise.
AndreyT
I mean this "(((iA * 256) + iR) * 256 + iG) * 256 + iB" compared to some simple "shift"s. Find that very confusing. However, I would like to take that comment back. Quite possible that I was talking out of my arse ;-)
Alex
@Alex: I'd understand if you didn't like the possile intermediate memory accesses in `dwColor /= 256`, but this??? This expression employs 3 multiplications (by 256) and 3 additions, while the original version used 3 shifts and 3 logical-ORs. I thought that it should be immediately obvious that operation-wise they have absolutely identical performance. And I maintain that smaller variety of immediate constants might lead to a better performing code.
AndreyT
Hmmm, delicious this is funny :-)))) If you guys still write good old C stuff (havn't done so for 10 years). Why don't you just access those bytes through a pointer? Wouldn't that be super straight forward?
Alex
Andrey, yes, yes, jo. I *was* talking out of my arse.
Alex
@Alex: Accesing these bytes through a pointer won't work in general because the result will depend of the endianness of the platform (little-endian vs. big-endian). Also, in C language "byte" is "char". And there's no guarantee in general that 'char' is exactly 8 bits. And finally, again - in general, such things might produce completely incorrect result because of compiler optimizations (read up on "type-punning" in GCC) (although, it should work fine with "bytes", i.e. chars).
AndreyT
Multiplication is not going to be as efficient as a shift, especially with a dependency chain like this. The bitwise solution is OK (but why not use `|`?) and it's probably not better than `(a<<24)|(r<<16)|(g<<8)|b` as there's little scope for hiding any latency.The magic constants theory is sound, but constant shift sizes and small (<=8 bits) mask constants are usually always part of the opcode, and so free. There is no real advantage to minimizing the number of such values.As always, the CPU datasheet and compiler output should be consulted before any microoptimizations.
brone
Firstly, a constant that is a part of opcode is not free. Performance-wise a constant that is a part of opcode is normally more expensive than a value in a register. But again, we are splitting hairs here about something that is actually hardware-dependent.
AndreyT
Secondly, and more importantly, "mutiplication" and "shift" in this code exist only at the level of C source file. Multiplication by 256 in this context is the same as shift left by 8, which means that with any self-respecting compiler in the generated machine code both versions will produce exactly the same code, most efficient from the compiler's point of view (quite possibly this won't be neither machine multiplication nor machine shift). This is what I mean by "absolutely identical performance".
AndreyT
The constant usually replaces one of the bitfields that would otherwise specify, e.g., a register number. (e.g., PPC/MIPS `andi`, ARM `and` with immediate Operand2 -- and likewise for shifts.) So it's as free as it gets, pretty much, and no need to wait for the construction of the constant to complete.You are right about the *256 thing. I am not sure why I assumed the compiler wouldn't spot it. And while checking it on my PC I noticed I had conveniently forgotten that x86 has variable length opcodes, and the shift constant takes another byte. So for x86 you are right about that too. Oops.
brone