1

I'm currently attempting to create a color gradient class for my Mandelbrot Set explorer.

It reads the color constraints (RGBA8888 color and position between 0 and 1) from a text file and adds them to a vector, which is lateron used to determine colors at a certain position.

To compute a color, the algorithm searches the next constraint to either side from the given position, splits the color into the four single channels, and then, for each one, searches the lower of both and adds a portion of the difference equal to the ratio (x-lpos)/(upos-lpos) to the lower color. Afterwards, the channels are shifted and ORed together, and then returned as RGBA8888 unsigned integer. (See the code below.)

EDIT: I completely rewrote the gradient class, fixing some issues and making it more readable for the sake of debugging (It gets slow as hell, though, but -Os more or less takes care of that). However, It's still not as it's supposed to be.

class Gradient { //remade, Some irrelevant methods and de-/constructors removed
private:
    map<double, unsigned int> constraints;
public:
    unsigned int operator[](double value) {
        //Forbid out-of-range values, return black
        if (value < 0 || value > 1+1E-10) return 0xff;
        //Find upper and lower constraint
        auto upperC = constraints.lower_bound(value);
        if (upperC == constraints.end()) upperC = constraints.begin();
        auto lowerC = upperC == constraints.begin() ? prev(constraints.end(), 1) : prev(upperC, 1);
        if (value == lowerC->first) return lowerC->second;
        double lpos = lowerC->first;
        double upos = upperC->first;
        if (upos < lpos) upos += 1;
        //lower color channels
        unsigned char lred = (lowerC->second >> 24) & 0xff;
        unsigned char lgreen = (lowerC->second >> 16) & 0xff;
        unsigned char lblue = (lowerC->second >> 8) & 0xff;
        unsigned char lalpha = lowerC->second & 0xff;
        //upper color channels
        unsigned char ured = (upperC->second >> 24) & 0xff;
        unsigned char ugreen = (upperC->second >> 16) & 0xff;
        unsigned char ublue = (upperC->second >> 8) & 0xff;
        unsigned char ualpha = upperC->second & 0xff;
        unsigned char red = 0, green = 0, blue = 0, alpha = 0xff;
        //Compute each channel using
        //  lower color + dist(lower, x)/dist(lower, upper) * diff(lower color, upper color)
        if (lred < ured)
            red = lred + (value - lpos)/(upos - lpos) * (ured - lred);
        else red = ured + (upos - value)/(upos - lpos) * (ured - lred);
        if (lgreen < ugreen)
            green = lgreen + (value - lpos)/(upos - lpos) * (ugreen - green);
        else green = ugreen + (upos - value)/(upos - lpos) * (ugreen - lgreen);
        if (lblue < ublue)
            blue = lblue + (value - lpos)/(upos - lpos) * (ublue - lblue);
        else blue = ublue + (upos - value)/(upos - lpos) * (ublue - lblue);
        if (lalpha < ualpha)
            alpha = lalpha + (value - lpos)/(upos - lpos) * (ualpha - lalpha);
        else alpha = ualpha + (upos - value)/(upos - lpos) * (ualpha - lalpha);
        //Merge channels together and return
        return (red << 24) | (green << 16) | (blue << 8 ) | alpha;
    }
    void addConstraint(unsigned int color, double position) {
        constraints[position] = color;
    }
};

Usage in the update method:

image[r + rres*i] = grd[ratio];
//With image being a vector<unsigned int>, which is then used as data source for a `SDL_Texture` using `SDL_UpdateTexture`

It only works partially, though. When I only use a black/white gradient, the resulting image is as intended:

black-white works flawlessly

Gradient file:

2
0 000000ff
1 ffffffff

However, when I use a more colorful gradient (a linear version of the Ultra Fractal gradient, input file below), the image is far from the intended result the image still doesn't show the desired coloring:

enter image description here

Gradient file:

5
0       000764ff
.16     206bcbff
.42     edffffff
.6425   ffaa00ff
0.8575  000200ff

What am I doing wrong? I've rewritten the operator[] method multiple times, without anything changing.

Questions for clarification or general remarks on my code are welcome.

Community
  • 1
  • 1
s3lph
  • 4,575
  • 4
  • 21
  • 38
  • You really ought to move a lot of that code out of `operator[]` and into the method that reads the constraints. Use a sorted `vector` instead of a `map` to hold the constraints, and parse those into separate RGBA components as early as possible. – Alnitak Apr 01 '15 at 16:16
  • oh, and if you store your RGBA components in a class of its own, you should probably put the method that does the basic math on those colours inside that class. – Alnitak Apr 01 '15 at 16:19
  • Why should the map be a problem? I actually used a vector before, but then switched to a map, because the values in a map **are sorted** by key ascending, which gives logarithmic access times when initializing. – s3lph Apr 01 '15 at 16:19
  • The RGBA is just an unsigned int and the `operator[]` actually IS the method that reads the constraints. – s3lph Apr 01 '15 at 16:20
  • Oops, you're right about the map being sorted. My point still stands about there being too much code in that function. – Alnitak Apr 01 '15 at 16:23
  • What do you exactly mean by "the method that reads the constraints" – s3lph Apr 01 '15 at 16:24
  • I mean the one that ought to be reading the gradient file and building the map. Your code will get a lot more comprehensible (and likely shorter) if it reads into an object that stores colours as separate RGBA components instead of packed into a single `int`. – Alnitak Apr 01 '15 at 16:26
  • FWIW, I actually need a similar colour map function in my C++ ray tracer - I may try to write one tonight :) – Alnitak Apr 01 '15 at 16:26
  • Why should it do that? The colors need to be uints, or I can't pass them to SDL lateron. And besides, I only made this code so extensive so I can debug it more easily (I'll shorten some things later) – s3lph Apr 01 '15 at 16:29
  • I did as you said and created a RGBA class. The result is nicer now, but still not correct (I will update the question later) – s3lph Apr 01 '15 at 17:12
  • Glad you got it sorted out, and hopefully you can see that breaking the RGBA code out of the Gradient code made it simpler to determine where the fault was. As for my solution, well, that's just math ;-) – Alnitak Apr 02 '15 at 13:37

2 Answers2

2

Your problem is due to an over-complicated interpolation function.

When linearly interpolating in the range a .. b using another factor r (with range 0 .. 1) to indicate the position in that range it's completely unnecessary to determine whether a or b is greater. Either way around you can just use:

result = a + r * (b - a)

If r == 0 this is trivially shown to be a, and if r == 1 the a - a cancels out leaving just b. Similarly if r == 0.5 then the result is (a + b) / 2. It simply doesn't matter if a > b or vice-versa.

The preferred formulation in your case, since it avoids the b - a subtraction that possibly hits range clamping limits is:

result = (1 - r) * a + r * b;

which given appropriate * and + operators on your new RGBA class gives this trivial implementation of your mid function (with no need for per-component operations since they're handled in those operators):

static RGBA mid(const RGBA& a, const RGBA& b, double r) {
    return (1.0 - r)  * a + r * b;
}

See https://gist.github.com/raybellis/4f69345d8e0c4e83411b, where I've also refactored your RGBA class to put the clamping operations in the constructor rather than within the individual operators.

Alnitak
  • 334,560
  • 70
  • 407
  • 495
1

After some extensive trial-and-error, I finally managed to get it working. (at this point many thanks to @Alnitak, who suggested using a separate RGBA color class.)

The major problem was that, when a color value of the upper constraint was lower than the one of the lower one, I still multiplied with the ratio (x-l)/(u-l), when instead I should have used its pendant, 1 - (x-l)/(u-l), to refer to the color of the upper constraint as the basis for the new one.

Here follows the implementation of the RGBA class and the fixed gradient class:

class RGBA {
private:
    unsigned int red = 0, green = 0, blue = 0, alpha = 0;
public:
    static RGBA mid(RGBA a, RGBA b, double r) {
        RGBA color;
        if (a.red < b.red) color.red = a.red + (b.red - a.red) * r;
        else color.red = b.red + (a.red - b.red) * (1-r);
        if (a.green < b.green) color.green = a.green + (b.green - a.green) * r;
        else color.green = b.green + (a.green - b.green) * (1-r);
        if (a.blue < b.blue) color.blue = a.blue + (b.blue - a.blue) * r;
        else color.blue = b.blue + (a.blue - b.blue) * (1-r);
        if (a.alpha < b.alpha) color.alpha = a.alpha + (b.alpha - a.alpha) * r;
        else color.alpha = b.alpha + (a.alpha - b.alpha) * (1-r);
        return color;
    }
    RGBA() {};
    RGBA(unsigned char _red, unsigned char _green, unsigned char _blue, unsigned char _alpha) :
            red(_red), green(_green), blue(_blue), alpha(_alpha) {};
    RGBA(unsigned int _rgba) {
        red = (_rgba >> 24) & 0xff;
        green = (_rgba >> 16) & 0xff;
        blue = (_rgba >> 8) & 0xff;
        alpha = _rgba & 0xff;
    };
    operator unsigned int() {
        return (red << 24) | (green << 16) | (blue << 8 ) | alpha;
    }
    RGBA operator+(const RGBA& o) const {
        return RGBA((red + o.red) & 0xff, (green + o.green) & 0xff, (blue + o.blue) & 0xff, (alpha + o.alpha) & 0xff);
    }
    RGBA operator-(const RGBA& o) const {
        return RGBA(min(red - o.red, 0u), min(green - o.green, 0u), min(blue - o.blue, 0u), min(alpha - o.alpha, 0u));
    }
    RGBA operator~() {
        return RGBA(0xff - red, 0xff - green, 0xff - blue, 0xff - alpha);
    }
    RGBA operator*(double _f) {
        return RGBA((unsigned int) min(red * _f, 0.) & 0xff, (unsigned int) min(green * _f, 0.) & 0xff,
                    (unsigned int) min(blue * _f, 0.) & 0xff, (unsigned int) min(alpha * _f, 0.) & 0xff);
    }
};

class Gradient {
private:
    map<double, RGBA> constraints;
public:
    Gradient() {
        constraints[0] = RGBA(0x007700ff);
        constraints[1] = RGBA(0xffffffff);
    }
    ~Gradient() {}
    void addConstraint(RGBA color, double position) {
        constraints[position] = color;
    }
    void reset() {
        constraints.clear();
    }
    unsigned int operator[](double value) {
        if (value < 0 || value > 1+1E-10) return 0xff;
        auto upperC = constraints.lower_bound(value);
        if (upperC == constraints.end()) upperC = constraints.begin();
        auto lowerC = upperC == constraints.begin() ? prev(constraints.end(), 1) : prev(upperC, 1);
        if (value == lowerC->first) return lowerC->second;
        double lpos = lowerC->first;
        double upos = upperC->first;
        if (upos < lpos) upos += 1;
        RGBA lower = lowerC->second;
        RGBA upper = upperC->second;
        RGBA color = RGBA::mid(lower, upper, (value-lpos)/(upos-lpos));
        return color;
    }
    size_t size() {
        return constraints.size();
    }
};

This is the result:

elephant valley

s3lph
  • 4,575
  • 4
  • 21
  • 38
  • You actually don't need to compare the start and end components to interpolate them. If `r` is the degree of interpolation (in range 0 .. 1) then you just need `val = (1 - r) * start + r * end`. In fact your entire `mid(a, b, r)` function can become `return (1 - r) * a + r * b`. – Alnitak Apr 02 '15 at 07:29