2

I am having trouble determining which class structure to use for storing RGB/RGBA color information.

I am making a very simple 3D game engine for fun and to hone my OO-programming abilities.

I would like my engine to have support for both RGB and RGBA colors. On top of that, I would like to have it so RGB and RGBA values can be specified as either floats clamped to [0, 1] or unsigned chars. I figured that for storage purposes, it would be better to use unsigned chars as they use a fourth as much memory as floats.

So now, I have devised the following classes to use for this: Color, Color3, Color3f, Color3c, Color4, Color4f, and Color4c. Color is the superclass of Color3 and Color4, while Color3 is the superclass of Color3f and Color3c, as Color4 is with Color4f and Color4c.

This seems like a much more complicated approach than need be. I also thought for a while about using generics, so instead of Color3f and Color3c, one would use Color3<float> and Color3<unsigned char>. However, this approach didn't seem right either.

Ideally, I would like to be able to write code like:

Color3 red1(255, 0, 0);
Color3 red2(1.0f, 0.0f, 0.0f);
Color* someColor = new Color3(1.0f, 0.0f, 0.5f);
Color4 anotherColor = *someColor;
Color4 thisColor = anotherColor.toChar();
//thisColor.r = 255, thisColor.g = 0, thisColor.b = 127
thisColor = thisColor.toFloat();
//thisColor.r = 1.0f, thisColor.g = 0.0f, thisColor.b = 0.5f

Now this can't be implemented (at least to my knowledge) in C++, but how could I muster the same functionality without creating 7 separate classes? And without storing wasted information? For example, picture a 1024x1024 image in memory. That would be an array with over a million of these Colors, so how can I make a Color class hierarchy that is flexible and reusable? i.e., store RGB and RGBA values in different structures through unsigned chars, but provide functionality to retrieve float values?

Sorry if this isn't specific enough or what, this is my first question! Let me know what else would be helpful. I can post code for what I have tried so far if you guys would prefer, but hopefully you understand what I'm trying to accomplish.

Xeo
  • 129,499
  • 52
  • 291
  • 397
berserkguard
  • 335
  • 2
  • 9
  • 5
    OOver-engineering much, eh? :) Just for note, what you wrote as an example is completely possible in C++ to do, with a bit of pimpl idiom and interfaces. Not that it would be a good idea to do it like that... – Xeo Dec 31 '11 at 19:29

4 Answers4

3

You only need one class Color (or prefferably Colour)

Internally you can store the value however you like, a 32bit unsigned int RGBA (or BGRA) is common and then just use masks and bit shifts to extract each component.

You can have constructors that take Color(unsigned char red,unsigned char green,unsigned char blue, unsigned char alpha=0xff) or floats

And similarly getter/setters for each component in each form.

edit: but if you are planning any sort of high performance an image consisting of an array of color objects for each pixels probably isn't the way to go. You might want to look at a set of static functions to set/get each component and a constructor that takes a pixel in your image format's format (ie a BGRA or RGB unsigned int)

Martin Beckett
  • 94,801
  • 28
  • 188
  • 263
  • Bit packing is the way to go if you want space-efficency. – Xeo Dec 31 '11 at 19:35
  • 1
    Although a struct containing only 4 chars should pack to the same efficiency. – Martin Beckett Dec 31 '11 at 19:37
  • Err, right, nvm me. I had some weird thoughts about padding in my head. – Xeo Dec 31 '11 at 20:00
  • Interesting how you noted to use Colour as opposed to Color. When searching stackoverflow to see if a similar question had already been asked, I came across [this post](http://stackoverflow.com/questions/2444650/colour-instead-of-color) that proposed the exact opposite. Not in the scope of this question, but just wanted to note that. Also, I'm looking over everyone's comments. All very helpful; I didn't even think to use bitmasking. I'll respond later with progress - thanks again guys! – berserkguard Dec 31 '11 at 20:17
  • @berserkguard - colour is the British (and Canadian etc) spelling, color is American, so almost all software uses color. Much to the annoyance of Brits! – Martin Beckett Dec 31 '11 at 20:19
  • Accepted as answer - decided to go with using the unsigned int to store the RGBA information, with getters/setters for both floats and unsigned chars. Also changed my code to have a character array with the data for each texture, as opposed to vector of Colors (so the getPixelAt(int x, int y) method first finds the position in the character array, THEN returns a Color object representing that pixel). Don't know why I thought a list of objects would be better, considering how rarely getPixelAt() will be called. Thanks again! – berserkguard Jan 01 '12 at 15:08
2

You want to make a class for all the possibilities. This is just wrong. One single class can deal with all of them just fine.

First, your GPU will expect 4x [0, 1] floats for colour data, and two, floating [0, 1] is way more precise than [0, 255] RGB colour. That means that it's lossy to go from float 0,1 to char 0, 255. These two facts mean that you realistically have no choice but to use 4x [0, 1] as the implementation. Whether you offer accessors and constructors for [0, 255] in addition is up to you.

Secondly, you can trivially support RGB and RGBA in the same class by simply defaulting A to be 1- this way, anyone who doesn't want to deal with A won't have to.

Puppy
  • 144,682
  • 38
  • 256
  • 465
  • Except that using 4x[0,1] floats requires 4 times as much memory. This is unacceptable for some applications manipulating very large images, such as 2K video frames. – André Caron Jan 01 '12 at 05:07
  • My thoughts exactly André - while you make great points DeadMG, is it reasonable to use 16 bytes to represent RGB color for 1024x1024 textures? I Doubt I'll go much higher than that, though. Maybe 2048x2048? That would seem like a lot of wasted memory, when it could be represented reasonably well with 3 bytes... – berserkguard Jan 01 '12 at 05:34
  • @Andre: The GPU uses it's own compression algorithms, as far as I know, and you don't have to implement your own. And secondly, modern high-end GPUs come with a *lot* of VRAM. – Puppy Jan 01 '12 at 11:43
  • @berserkguard: It could be. But as soon as you move it to your GPU it has to go into 12 bytes *anyway*, so you're not saving yourself a single byte of room in reality. 2048x2048x12 is only 48MB anyway- a drop in the bucket for the 2 or 4 GB you can find on a graphics card now. – Puppy Jan 01 '12 at 11:44
  • Thank you for describing the problem in term of the GPU, as that definitely pertains to what I'm trying to accomplish on a deeper level. +1 – berserkguard Jan 01 '12 at 15:04
  • @DeadMG: that's why I said unnacceptable for **some** applications. I'll let OP judge whether or not this is acceptable for the task at hand. – André Caron Jan 01 '12 at 16:56
1

Xeo's point about over-engineering is a good one, and you might want to read my answer on avoiding over-engineering. But the whole point of OO is to conceal complexity, not encourage it.

Start with a Color class with some efficient internal representation, and overload ctors for the data types you want as arguments. until and unless you have some need to expose the implementation, don't bother with all the subclasses. Take the simplest approach that could possibly work.

Community
  • 1
  • 1
Charlie Martin
  • 110,348
  • 25
  • 193
  • 263
  • Thanks for that very-informative post. I've always had a tendency to overthink every detail, but I really like your approach of only doing what's absolutely needed and adding other features only where necessary. – berserkguard Jan 01 '12 at 15:01
1

This is just coding what the others said because it looks like fun. This is not an answer.

class color {
    unsigned char red_, green_, blue_, alpha_;
public:
    color() 
    :red_(0), green_(0), blue_(0), alpha_(0xFF) {}
    color(unsigned char red, unsigned char green, unsigned char blue, unsigned char alpha=0xFF)
    :red_(red), green_(green), blue_(blue), alpha_(alpha) {}
    color(float red, float green, float blue, float alpha=1.0)
    :red_(red*255.0+.5), green_(green*255.0+.5), blue_(blue*255.0+.5), alpha_(alpha*255.0+.5) {}
    color(const color& rhs)
    :red_(rhs.red_), green_(rhs.green_), blue_(rhs.blue_), alpha_(rhs.alpha_) {}
    //operator= and dtor automatically generated
    unsigned char& c_red() {return red_;}
    unsigned char& c_green() {return green_;}
    unsigned char& c_blue() {return blue_;}
    unsigned char& c_alpha() {return alpha_;}
    const unsigned char& c_red() const {return red_;}
    const unsigned char& c_green() const {return green_;}
    const unsigned char& c_blue() const {return blue_;}
    const unsigned char& c_alpha() const {return alpha_;}
    void set_f_red(float val) {red_=val*255.0+.5;}
    void set_f_green(float val) {green_=val*255.0+.5;}
    void set_f_blue(float val) {blue_=val*255.0+.5;}
    void set_f_alpha(float val) {alpha_=val*255.0+.5;}
    float get_f_red() const {return red_/255.0;}
    float get_f_green() const {return green_/255.0;}
    float get_f_blue() const {return blue_/255.0;}
    float get_f_alpha() const {return alpha_/255.0;}

    unsigned int rgba() {return (red_<<24)|(green_<<16)|(blue_<<8)|(alpha_<<0);}
};
Mooing Duck
  • 64,318
  • 19
  • 100
  • 158
  • interesting... I would have stored as an unsigned int, since I would expect the rgba method to be called a lot more compared to ctors and getters/setters. any reason for that preference? – João Portela Dec 31 '11 at 20:22
  • Basically, portablility. Some applications want argb, some rgba, some rbg... Also, no fear of endian issues... You're right that unsigned int rgba would be faster in most cases though. – Mooing Duck Dec 31 '11 at 23:45