2013-06-28 2 views
1

У меня есть класс цвета RGB с двумя перегруженными операторами сравнения (operator ==). Один для типа self и один для int (HEX).Ошибка перегруженного оператора сравнения [C++ 11]

// this one assigns the value correctly 
RGB  RGB::operator=(const int hex) 
{ 
    this->r = (hex>>16 & 0xFF)/255.0f; 
    this->g = (hex>>8 & 0xFF)/255.0f; 
    this->b = (hex  & 0xFF)/255.0f; 
    return *this; 
} 
//-------------------------------------------------------------------------------------- 
// also works 
bool RGB::operator==(const RGB &color) 
{ 
    return (r == color.r && g == color.g && b == color.b); 
} 
// this is evil 
bool RGB::operator==(const int hex) 
{ 
    float rr = (hex>>16 & 0xFF)/255.0f; 
    float gg = (hex>>8 & 0xFF)/255.0f; 
    float bb = (hex  & 0xFF)/255.0f; 

    // if i uncomment these lines then everything is fine 
    //std::cout<<r<<" "<<rr<<std::endl; 
    //std::cout<<g<<" "<<gg<<std::endl; 
    //std::cout<<b<<" "<<bb<<std::endl; 

    return (r == rr && 
      g == gg && 
      b == bb); 
} 

RGB::RGB(int hex) 
{ 
    setHex(hex); 
} 

inline void RGB::setHex(unsigned hex) 
{ 
    r = (float)(hex >> 16 & 0xFF)/255.0f; 
    g = (float)(hex >> 8 & 0xFF)/255.0f; 
    b = (float)(hex & 0xFF)/255.0f; 
} 

... тогда я сравниваю в main.cpp, как:

RGB a = 0x555555; 
bool equals = (a == 0x555555); // returns false 

Я не знаю, что происходит. Сравнение возвращает значение false, но если я раскомментирую строки std :: cout в определении, то функция работает как ожидалось и возвращает true.

Это также работает без проблем:

RGB a = 0x555555; 
RGB b = 0x555555; 
bool equals = (a == b); // returns true 

Любой имеет идею?

+0

Почему вы используете с плавающей точкой в ​​качестве базового типа для значений RGB? – aryjczyk

+0

Поскольку он упрощает вычисления с помощью цветов (смешивания, преобразования между цветовыми пространствами и т. Д.), А OpenGL принимает цветовые компоненты как плавающие. – plasmacel

+1

@ user2430597 Пожалуйста, разместите определение 'RGB :: RGB (int hex)'. Код [works] (http://coliru.stacked-crooked.com/view?id=d5578037553aa013b07b797457554d17-add99d1eb3698568526b0ba407c61bb0), если вы используете только операторов, которые вы опубликовали. – Praetorian

ответ

4

Вы не должны получать эффекты сравнения с плавающей запятой без оптимизации. Это связано с тем, что в обоих случаях у вас одинаковая функция.

Без оптимизации, верно следующее:

float func(float); 
float a = ...; 
func(a) == func(a); //< always true 

Это то, что у вас есть, где ваша функция сдвига и разделить на 255.

Однако - с оптимизацией, это другая история. GCC имеет оптимизацию (см., Например, -freciprocal-math, -fassociative-math), которая может изменить ваши выражения.

В вашем случае, у вас есть:

float rr = (X)/255.0f; 
... 
r == rr 

Например, он может сделать то, что сводится к тому, по оптимизации:

255.0f * r == (X) 

Теперь это делает страдает от плавающих эффектов сравнения точек , Однако, введя stdout в середине, вы вынуждаете его оценивать выражения ближе к тому, как они написаны, что снова возвращает вас к здравомыслию оценивать правду на одной и той же функции против себя.

Вы можете изменить определение класса для хранения значений в виде целых чисел и преобразовать их в float только тогда, когда вам нужны поплавки или хранить как шестнадцатеричное представление, так и поплавки и используя шестнадцатеричный код для сравнения. Или вы можете выполнить сравнение, проверяя, находятся ли два значения с плавающей запятой в пределах 1/255 друг от друга, используя значение больше/меньше, чем вместо двойного равенства. Например. что-то вроде этого:

return (abs(r - rr) < 1/255.0f && ...); 
2

Знаете ли вы, что ваш RGB::operator=() никогда не вызывается?

RGB a = 0x555555; 

вызывает конструктор RGB, который принимает int. Не имея такого конструктора, ваш код не будет компилироваться, и, таким образом, данный фрагмент неполный. Тем не менее,

RGB a; 
a = 0x555555; 

по умолчанию строит RGB экземпляр и вызывает ваш RGB::operator=(int). I tried your code with both, clang++ and g++ and the comparison always evaluates to true`.

Точка, что код ведет себя по-разному с линиями std::cout, прокомментированными в или вне довольно странно. Это может быть проблема оптимизации вместе с плавающей точкой, которая сравнивается с злом: google для «сравнения с плавающей точкой», чтобы понять, почему. Чтобы проверить это, я хотел бы приложить к нему отладчик и посмотреть фактические (шестнадцатеричные) значения r, rr, g, gg, b и bb.

Обратите внимание, что оператор присваивания должен возвращать ссылку на *this, а не копию.

0

Спасибо, ребята, проблема была в оптимизации, как заявила JoshG79. Чтобы устранить эту проблему, сначала я попытался сохранить вычисления в переменных переменных, которые могут помешать их оптимизации, но они делают некоторые другие вещи и вызывают накладные расходы. Поэтому я решил использовать GCC function attributes.

Так объявление функции в заголовке выглядит следующим образом:

bool operator==(const unsigned int hex) __attribute__((optimize("O0"))); 

Теперь все хорошо и работать как шарм.

+1

Более надежным и портативным решением может быть замена (r == rr) на (abs (r-rr) <1.0/512.0) или на все, что вы считаете нужным. Или преобразуйте значения float в целые числа и сравните целые числа, а не наоборот. –