Using named colors

[Relic]

Hi guys! This is my 1st post here. I'm working on my game engine that uses sfml. And I want to share portion of my (and betajaen's) code. If it is helpfull may be it is worth to be wikied.  
I think that pre-defined named colors are much more convenient than their digital equivalents. In my engine's graphics utility class I'm using the following enum that was borrowed from OGRE's addon Gorilla created by betajaen (license included). Unlike sfml's approach where pre-defined basic colors (Red, Blue etc.) are static objects, I use a convertion function that returns sf::Color. May be this code will be useful for somebody.

 

Code: [Select]

/*
    Gorilla
    -------
   
    Copyright (c) 2010 Robin Southern
                                                                                 
    Permission is hereby granted, free of charge, to any person obtaining a copy
    of this software and associated documentation files (the "Software"), to deal
    in the Software without restriction, including without limitation the rights
    to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
    copies of the Software, and to permit persons to whom the Software is
    furnished to do so, subject to the following conditions:
                                                                                 
    The above copyright notice and this permission notice shall be included in
    all copies or substantial portions of the Software.
                                                                                 
    THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
    IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
    FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
    AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
    LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
    OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
    THE SOFTWARE.
   
*/
namespace NamedColor
{
enum NamedColor
{
  None = 0, // No Color.                                                        
  AliceBlue=0xf0f8ff,      Gainsboro=0xdcdcdc,            MistyRose=0xffe4e1,    
  AntiqueWhite=0xfaebd7,   GhostWhite=0xf8f8ff,           Moccasin=0xffe4b5,      
  Aqua=0x00ffff,           Gold=0xffd700,                 NavajoWhite=0xffdead,  
  Aquamarine=0x7fffd4,     Goldenrod=0xdaa520,            Navy=0x000080,          
  Azure=0xf0ffff,          Gray=0x808080,                 OldLace=0xfdf5e6,      
  Beige=0xf5f5dc,          Green=0x008000,                Olive=0x808000,        
  Bisque=0xffe4c4,         GreenYellow=0xadff2f,          OliveDrab=0x6b8e23,    
  Black=0x000000,          Grey=0x808080,                 Orange=0xffa500,        
  BlanchedAlmond=0xffebcd, Honeydew=0xf0fff0,             OrangeRed=0xff4500,    
  Blue=0x0000ff,           HotPink=0xff69b4,              Orchid=0xda70d6,        
  BlueViolet=0x8a2be2,     IndianRed=0xcd5c5c,            PaleGoldenrod=0xeee8aa,
  Brown=0xa52a2a,          Indigo=0x4b0082,               PaleGreen=0x98fb98,    
  Burlywood=0xdeb887,      Ivory=0xfffff0,                PaleTurquoise=0xafeeee,
  CadetBlue=0x5f9ea0,      Khaki=0xf0e68c,                PaleVioletRed=0xdb7093,
  Chartreuse=0x7fff00,     Lavender=0xe6e6fa,             PapayaWhip=0xffefd5,    
  Chocolate=0xd2691e,      LavenderBlush=0xfff0f5,        PeachPuff=0xffdab9,    
  Coral=0xff7f50,          LawnGreen=0x7cfc00,            Peru=0xcd853f,          
  CornflowerBlue=0x6495ed, LemonChiffon=0xfffacd,         Pink=0xffc0cb,          
  Cornsilk=0xfff8dc,       LightBlue=0xadd8e6,            Plum=0xdda0dd,          
  Crimson=0xdc143c,        LightCoral=0xf08080,           PowderBlue=0xb0e0e6,    
  Cyan=0x00ffff,           LightCyan=0xe0ffff,            Purple=0x800080,        
  DarkBlue=0x00008b,       LightGoldenrodyellow=0xfafad2, Red=0xff0000,          
  DarkCyan=0x008b8b,       LightGray=0xd3d3d3,            RosyBrown=0xbc8f8f,    
  DarkGoldenrod=0xb8860b,  LightGreen=0x90ee90,           RoyalBlue=0x4169e1,    
  DarkGray=0xa9a9a9,       LightGrey=0xd3d3d3,            SaddleBrown=0x8b4513,  
  DarkGreen=0x006400,      LightPink=0xffb6c1,            Salmon=0xfa8072,        
  DarkGrey=0xa9a9a9,       LightSalmon=0xffa07a,          SandyBrown=0xf4a460,    
  DarkKhaki=0xbdb76b,      LightSeagreen=0x20b2aa,        SeaGreen=0x2e8b57,      
  DarkMagenta=0x8b008b,    LightSkyblue=0x87cefa,         SeaShell=0xfff5ee,      
  DarkOlivegreen=0x556b2f, LightSlategray=0x778899,       Sienna=0xa0522d,        
  DarkOrange=0xff8c00,     LightSlategrey=0x778899,       Silver=0xc0c0c0,        
  DarkOrchid=0x9932cc,     LightSteelblue=0xb0c4de,       SkyBlue=0x87ceeb,      
  DarkRed=0x8b0000,        LightYellow=0xffffe0,          SlateBlue=0x6a5acd,    
  DarkSalmon=0xe9967a,     Lime=0x00ff00,                 SlateGray=0x708090,    
  DarkSeagreen=0x8fbc8f,   LimeGreen=0x32cd32,            SlateGrey=0x708090,    
  DarkSlateblue=0x483d8b,  Linen=0xfaf0e6,                Snow=0xfffafa,          
  DarkSlategray=0x2f4f4f,  Magenta=0xff00ff,              SpringGreen=0x00ff7f,  
  DarkSlategrey=0x2f4f4f,  Maroon=0x800000,               SteelBlue=0x4682b4,    
  DarkTurquoise=0x00ced1,  MediumAquamarine=0x66cdaa,     Tan=0xd2b48c,          
  DarkViolet=0x9400d3,     MediumBlue=0x0000cd,           Teal=0x008080,          
  DeepPink=0xff1493,       MediumOrchid=0xba55d3,         Thistle=0xd8bfd8,      
  DeepSkyblue=0x00bfff,    MediumPurple=0x9370db,         Tomato=0xff6347,        
  DimGray=0x696969,        MediumSeaGreen=0x3cb371,       Turquoise=0x40e0d0,    
  DimGrey=0x696969,        MediumSlateBlue=0x7b68ee,      Violet=0xee82ee,        
  DodgerBlue=0x1e90ff,     MediumSpringGreen=0x00fa9a,    Wheat=0xf5deb3,        
  FireBrick=0xb22222,      MediumTurquoise=0x48d1cc,      White=0xffffff,        
  FloralWhite=0xfffaf0,    MediumBioletRed=0xc71585,      WhiteSmoke=0xf5f5f5,    
  ForestGreen=0x228b22,    MidnightBlue=0x191970,         Yellow=0xffff00,        
  Fuchsia=0xff00ff,        MintCream=0xf5fffa,            YellowGreen=0x9acd32    
};
} // namespace

//-----------------------------------------------------------------------------
class GraphUtils
{

public:

  static sf::Color ToSfColor(unsigned int namedColor, unsigned char alpha = 255);
};

Code: [Select]

//-----------------------------------------------------------------------------
sf::Color GraphUtils::ToSfColor(unsigned int namedColor, unsigned char alpha)
{
  union
  {
    unsigned int whole;
    unsigned char byte[4];
  };
 
  whole = namedColor;
 
  return sf::Color(byte[2], byte[1], byte[0], alpha);
};

Using named colors...

Code: [Select]


...
label.SetFrameColor(GraphUtils::ToSfColor(NamedColor::BlueViolet));
...

[Laurent]

Hi

I have two questions:

1. What's the benefit of having constants + a conversions function, instead of the colors directly as sf::Color?

2. Your conversion trick involving a union is not portable (it's broken when endianness is reversed), and it's not even supported by the standard as a conversion method (even if in practice it always works), technically it is an undefined behaviour.

[Silvah]

1. What's the benefit of having constants + a conversions function, instead of the colors directly as sf::Color?

According to the standard, constants of non-POD types cannot be stored in constant data section, they have to be put into modifiable data section and initialized before main() instead. On the other hand, enumeration values are in the same class as numeric literals, they aren't stored anywhere in the binary at all (that is, they're almost always inlined into instruction stream).

[Laurent]

According to the standard, constants of non-POD types cannot be stored in constant data section, they have to be put into modifiable data section and initialized before main() instead. On the other hand, enumeration values are in the same class as numeric literals, they aren't stored anywhere in the binary at all (that is, they're almost always inlined into instruction stream).

Ok you're right, but what's the point? Saving memory? I prefer using slightly more memory (and honestly, how cares about these few bytes?) and a little less CPU at runtime -- and as a side effect, a less verbose syntax for using these colors.

[Relic]

The reason is simple. I did not want to manually split betajaen's RGB constants in parts and construct sf::Color for each named color, or write a few lines of python code that would do it for me. I'm lazy. And sf::Color has no suitable constructor that receives RGB as solid integer. As for performance, yes, the convertion is not too swift. But i don't think it matters, as assigning colors mostly occurs somewhere in initialization code that is executed once.

[Laurent]

The reason is simple. I did not want to manually split betajaen's RGB constants in parts and construct sf::Color for each named color, or write a fiew lines of python code that would do it for me. I'm lazy.

Maybe it's worth doing the effort, and providing users with a more efficient/clean/simple code

As for performance, yes, the convertion is not too swift. But i don't think it matters, as assigning colors mostly occurs somewhere in initialization code that is executed once.

Yep, performance doesn't really matter. What's the most important here is that your code doesn't work on big-endian processors (like PPC).

You should use this code instead:

Code: [Select]

sf::Color GraphUtils::ToSfColor(unsigned int namedColor, unsigned char alpha)
{
    sf::Uint8 r = static_cast<sf::Uint8>((namedColor & 0xFF0000) >> 16);
    sf::Uint8 g = static_cast<sf::Uint8>((namedColor & 0x00FF00) >> 8);
    sf::Uint8 b = static_cast<sf::Uint8>(namedColor & 0x0000FF);

    return sf::Color(r, g, b, alpha);
};

[Relic]

your code doesn't work on big-endian processors (like PPC)

Agreed... I'm just sitting on PC all the time.

[Silvah]

Ok you're right, but what's the point? Saving memory? I prefer using slightly more memory (and honestly, how cares about these few bytes?) and a little less CPU at runtime -- and as a side effect, a less verbose syntax for using these colors.

Unless your compiler is extremely bad at inlining (or is simply unable to inline) and constant propagation, you're saving both memory and CPU (program will start slightly faster, because it won't have to initialize all these "constants" at startup).

[Laurent]

We shouldn't focus on such unnoticeable things... The only important thing here is how this code is going to be used by people. And as a user, I would prefer using direct colors than constants + a conversion function every time I need them. There's no need for the constants alone, one always need to use them together with the conversion function. So why not providing the result directly?

[Relic]

Ok, I'll make an include file with the code similar to following

Code: [Select]

#include <SFML/Color.hpp>

namespace NamedColor
{
static const sf::Color AliceBlue(0xF0, 0xF8, 0xFF);
static const sf::Color AntiqueWhite(0xFA, 0xEB, 0xD7);
// more constants
...
} // namespace

The constants still sit in the namespace NamedColor. It can be changed. As sfml is opensource everyone can even move them inside class sf::Color and extend the existing list of pre-defined color constants.

[Nexus]

I have to agree with Laurent: To call a conversion function every time you need a sf::Color is an unnecessary burden resulting from questionable considerations.

The performance "arguments" brought here are typical for premature optimization. Load a single sf::Image, and the runtime and memory usage outweigh by huge factors. If we talked about time-critical code at the inside of an often-called loop and the big optimization opportunities have already been exploited, okay. But saving a few bytes and a few cycles once at initialization (which is hardly noticeable) at the cost of making code user-unfriendly is generally a bad idea.

About the laziness to convert every literal: It's no big deal to write a program that replaces AliceBlue=0xf0f8ff, with sf::Color AliceBlue(0xf0, 0xf8, 0xff);. Automated replacement is far less error-prone than manual one.

And just that you're aware, Relic: By using the static storage class (or no explicit keyword at const variables), the memory gets duplicated for each translation unit. The storage class extern avoids that, but then the definitions have to be moved to an implementation file.

[Silvah]

We shouldn't focus on such unnoticeable things... The only important thing here is how this code is going to be used by people.

Ah, it's no wonder that I have to wait thirty seconds before my internet browser starts.

The performance "arguments" brought here are typical for premature optimization.

Yeah, premature optimization is by all means bad. What about premature pessimization? Yes,
I know, PCs have several gigs of RAM and CPUs that can do zillions
of operations per second today. Maybe I was programming for machines with severly limited resources too long?

I've posted here a solution which didn't require explicit conversion and global constructors, nor it required more memory than sf::Color constants, and was also pretty efficient at runtime, at the cost of being somewhat hackish, but I deleted it, heh...

[Nexus]

Yeah, premature optimization is by all means bad. What about premature pessimization?

It's as bad. But I want to explain you why we're not talking about it in this topic.

Premature pessimization means not to think about efficiency where it is appropriate. It's like using the wrong container or algorithm, creating unnecessary copies, allocating too much memory, etc. Given the right knowledge, such mistakes can be easily avoided, while the alternative implies a similar implementation effort.

In contrast, premature optimization means to waste time at optimizing program parts that are not of time-critical relevance. Often, it goes hand in hand with the avoidance of profilers and measurement, the reliance on pure assumptions (which are not rarely wrong), and a neglect of the real bottlenecks. Premature optimization is especially evil as it often includes complex, hardly readable and non-portable code (in short: dirty hacks) and as it approves user-unfriendliness and error-proneness. The price to pay is far too high, being mindful of the fact that the effective performance gain is neither proven nor relevant. As you see, the thread and your alternative suggestion fit many of the just mentioned criterions.

[Silvah]

Well, the things you're talking about are pretty obvious. I do know what premature optimization and pessimization are.

It's as bad. But I want to explain you why we're not talking about it in this topic.

We do. I really don't want the browser to take thirty second to load (I'll use that analogy, until I'll find a better one) just because its developers love non-trivial global objects.

Honestly, I don't see any reason to argue about that code. It's unlikely that it will be incorporated into SFML, after all. It's unlikely that you or I will meet it anywhere in production. It's unlikely that... you got the idea.

By the way - I was once told to avoid global constructors altogether by all possible means, maybe I still subconsciously try to adhere to this rule, hehe

[Nexus]

We do. I really don't want the browser to take thirty second to load (I'll use that analogy, until I'll find a better one) just because its developers love non-trivial global objects.

What kind of argument is this?
"My browser starts slowly" => "The reason must be the construction of global objects" => "So I don't construct constants and use bad code instead"

Sorry, but you can't draw that conclusion. You don't know why the browser starts slowly. You don't know either how long the construction takes and how relevant it really is, because you have never measured it. You are just guessing.

Honestly, I don't see any reason to argue about that code.

It's just bad style to evoke undefined behaviour (union-cast) because of a non-existing problem.

By the way - I was once told to avoid global constructors altogether by all possible means, maybe I still subconsciously try to adhere to this rule, hehe

The one who told you that probably meant constructions of depending modules shouldn't happen at global level since the initialization order is undefined.