2013/4/22 Kris De Gussem <kris.degussem@...400...>:
I had a look on the set property of the SPIPaint structure, and I noticed it is only used as a bool, while this property was defined as an unsigned integer. This is not a good programming practice imho. This results in comparing this integer with an a boolean in amongst others desktop-style.cpp line 505. Is this desirable?
The relevant change is:
=== modified file 'src/style.h' --- src/style.h 2013-04-08 12:16:02 +0000 +++ src/style.h 2013-04-22 12:19:57 +0000 @@ -136,11 +136,11 @@
/// Paint type internal to SPStyle. struct SPIPaint { - unsigned set : 1; - unsigned inherit : 1; - unsigned currentcolor : 1; - unsigned int colorSet : 1; - unsigned int noneSet : 1; + bool set; //todo check if other properties are also booleans + unsigned inherit; + unsigned currentcolor; + unsigned int colorSet; + unsigned int noneSet;
All these properties were declared as unsigned, one-bit bitfields (that's what ": 1" after the name meant), not as full unsigned integers.
See here for details: http://www.tutorialspoint.com/cprogramming/c_bit_fields.htm
Bitfields are slightly slower than unsigned ints, but on modern processors which internally support only aligned adressing modes and emulate non-aligned accesses in microcode, bitfields should have performance equivalent to bools. If there are more than 4 boolean values, as in the case of SPStyle, one-bit bitfields will save space compared to bools.
Comparing one-bit bitfields to booleans is entirely acceptable, since the former can only have the values 0 or 1.
Regards, Krzysztof