7
\$\begingroup\$

With the SDL library, I'm creating an SDL_Surface* at w and h dimensions. SDL_Color is a struct with 4 unsigned char fields: a, r, g and b for alpha, red, green and blue. c1 is the origin color, c2 the destination color, and c is the color of the pixel at position (i,j).

That gradient works fine, but it is very basic and I'm just wondering if there is any way to optimize it, whether with a better SDL use or a faster algorithm.

SDL_Surface* DrawDescendingDiagonalGradient(SDL_Color& c1, SDL_Color& c2, int w, int h)    {
    SDL_Color c;
    SDL_Surface *s = SDL_CreateRGBSurface(0, w, h, 32, 0, 0, 0, 0);
    SDL_LockSurface(s);
    for (int j = 0; j < h; j++) {
        for (int i = 0; i < w; i++) {
            c.a = c1.a * (w + h - i - j) / (w + h) + c2.a * (i + j) / (w + h);
            c.r = c1.r * (w + h - i - j) / (w + h) + c2.r * (i + j) / (w + h);
            c.g = c1.g * (w + h - i - j) / (w + h) + c2.g * (i + j) / (w + h);
            c.b = c1.b * (w + h - i - j) / (w + h) + c2.b * (i + j) / (w + h);
            PutPixel32(s, i, j, SDL_MapRGBA(s->format, c.r, c.g, c.b, c.a));
        }
    }
    SDL_UnlockSurface(s);
    return s;
}
\$\endgroup\$
4
\$\begingroup\$

In addition to Janos's answer.

You're truncating the color value too early here:

c.a = c1.a * (w + h - i - j) / (w + h) + c2.a * (i + j) / (w + h);

This results in a rounding error on each channel. Also this forces the compiler to perform two divisions per channel, as it cannot combine the divisions due to truncation rules of integer types.

For example:

int a = 1;
int b = 2;
int c = 3;

int result = a / c + b / c;

The result must be calculated as (a/c) + (b/c)=0 if optimized to (a+b)/c=1 the result will differ. And in your case you actually want (a+b)/c as the correct answer.

In your case you should do:

SDL_Surface* DrawDescendingDiagonalGradient(SDL_Color& c1, SDL_Color& c2, int w, int h){
    SDL_Color c;
    SDL_Surface *s = SDL_CreateRGBSurface(0, w, h, 32, 0, 0, 0, 0);
    SDL_LockSurface(s);
    int wh = w + h;
    for (int j = 0; j < h; j++) {
        for (int i = 0; i < w; i++) {
            int ij = i + j;
            c.a = (c1.a * (wh - ij) + c2.a * ij) / wh;
            c.r = (c1.r * (wh - ij) + c2.r * ij) / wh;
            c.g = (c1.g * (wh - ij) + c2.g * ij) / wh;
            c.b = (c1.b * (wh - ij) + c2.b * ij) / wh;
            PutPixel32(s, i, j, SDL_MapRGBA(s->format, c.r, c.g, c.b, c.a));
        }
    }
    SDL_UnlockSurface(s);
    return s;
}

This will make sure you get the correct rounding and remove one division per channel. Also when running the code un-optimized (while debugging) this should perform slightly better due to elimination of some subexpressions.

\$\endgroup\$
2
\$\begingroup\$

I don't think there's a lot you can improve here. I would use temporary variables to reduce the repetition in i + j and w + h, and extract the common transformation operation to a function:

Uint8 transformColorField(Uint8 c1x, Uint8 c2x, int wh, int ij) {
    return c1x * (wh - ij) / wh + c2x * ij / wh;
}

SDL_Surface* DrawDescendingDiagonalGradient(SDL_Color& c1, SDL_Color& c2, int w, int h)    {
    SDL_Color c;
    SDL_Surface *s = SDL_CreateRGBSurface(0, w, h, 32, 0, 0, 0, 0);
    SDL_LockSurface(s);
    int wh = w + h;
    for (int j = 0; j < h; j++) {
        for (int i = 0; i < w; i++) {
            int ij = i + j;
            c.a = transformColorField(c1.a, c2.a, wh, ij);
            c.r = transformColorField(c1.r, c2.r, wh, ij);
            c.g = transformColorField(c1.g, c2.g, wh, ij);
            c.b = transformColorField(c1.b, c2.b, wh, ij);
            PutPixel32(s, i, j, SDL_MapRGBA(s->format, c.r, c.g, c.b, c.a));
        }
    }
    SDL_UnlockSurface(s);
    return s;
}

Of course, none of this will have any real effect on the speed. For that, you'd need a different algorithm. (And I don't know what, and how exactly.)

\$\endgroup\$
  • \$\begingroup\$ A simple function used by only one function, always given the same expression for some arguments: That calls for auto+lambda. \$\endgroup\$ – Deduplicator Sep 11 '18 at 20:33

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.