0

Here is my code in c++. It's working perfectly fine but i'm curious what could i did wrong in terms of optimalization and code clarity. Also can you help my understand why " warning: comparing floating point with == or != is unsafe "?

Thank you for your time :)

Here is RGB to HSV:

struct hsv_struct {
    float h=0,s=0,v=0;
};
struct rgb_struct {
    float r=0,g=0,b=0;
};

hsv_struct rgb_to_hsv(hsv_struct hsv,rgb_struct rgb){
        rgb.r = rgb.r/255.0f;
        rgb.g = rgb.g/255.0f;
        rgb.b = rgb.b/255.0f;
        float max=0,min=0;
        if(rgb.r>rgb.g && rgb.r>rgb.b){ max=rgb.r;
           if(rgb.g<rgb.b) min=rgb.g;
           else min=rgb.b;
           }
        else if(rgb.g>rgb.b){ max=rgb.g;
           if(rgb.b<rgb.r) min=rgb.b;
           else min=rgb.r;
           }
        else{ max=rgb.b;
           if(rgb.r<rgb.g) min=rgb.r;
           else min=rgb.g;
           }
        if (rgb.r==max)     hsv.h=(rgb.g-rgb.b)/(max-min);
        else if(rgb.g==max) hsv.h=(2.0f+(rgb.b-rgb.r)/(max-min));
        else if(rgb.b==max) hsv.h=(4.0f+(rgb.r-rgb.g)/(max-min));

        hsv.h = hsv.h * 60;
        if(hsv.h<0) hsv.h = hsv.h+360;

        hsv.s = (max-min) / max;
        if (max==0) hsv.s=0;

        hsv.v=max;

        return hsv;
}

And here HSV to RGB:

rgb_struct hsv_to_rgb(hsv_struct hsv,rgb_struct rgb){
    float p, q, t, ff;
    long i;
    if(hsv.v==0.00000f) {
        rgb.r = 0;
        rgb.g = 0;
        rgb.b = 0;
    }
    else {
        if(hsv.h >= 360.0) hsv.h  = 0.0;
        hsv.h  /= 60.0;
        i = floor(hsv.h);
        ff = hsv.h - i;
        p = hsv.v * (1.0f - hsv.s);
        q = hsv.v * (1.0f - (hsv.s * ff));
        t = hsv.v * (1.0f - (hsv.s * (1.0f - ff)));

        switch(i){
        case 0:
            rgb.r = hsv.v;
            rgb.g = t;
            rgb.b = p;
            break;
        case 1:
            rgb.r = q;
            rgb.g = hsv.v;
            rgb.b = p;
            break;
        case 2:
            rgb.r = p;
            rgb.g = hsv.v;
            rgb.b = t;
            break;
        case 3:
            rgb.r = p;
            rgb.g = q;
            rgb.b = hsv.v;
            break;
        case 4:
            rgb.r = t;
            rgb.g = p;
            rgb.b = hsv.v;
            break;
        case 5:
        default:
            rgb.r = hsv.v;
            rgb.g = p;
            rgb.b = q;
            break;
        }
    }
rgb.r*=255;
rgb.g*=255;
rgb.b*=255;
return rgb;
}

Dont mind me i'm here to fool the "mostly code" error!.. Do i work?

  • 2
    If the code really is "working perfectly fine", and you just want a review of this working code (to improve it), then I suggest you post on [CodeReview](http://codereview.stackexchange.com/tour) instead. Don't forget to check [what you can ask about there](https://codereview.stackexchange.com/help/on-topic) first. – Some programmer dude Nov 27 '18 at 15:29
  • The compiler is not convinced that your test on 0.00000f is kosher. It is fishy, adding more zeros doesn't make it more accurate. if (hsv.v < MinValue) solves it, for a to-be-determined value of MinValue :) – Hans Passant Nov 27 '18 at 15:39
  • Thank you for the suggestion, I'm going to post my code there also. – Konrad Bernaszuk Nov 27 '18 at 15:42
  • Thanks Hans! Fixed and remembered :) – Konrad Bernaszuk Nov 27 '18 at 15:48

1 Answers1

0

On floating point arithmetic please read Is floating point math broken?

Also you can pass structures as const type_name& to functions for optimal performance

grapes
  • 8,185
  • 1
  • 19
  • 31