patterncModerate
Mandelbrot image generator
Viewed 0 times
imagemandelbrotgenerator
Problem
I have written a C program to generate a PPM (Portable Pixmap) image of the Mandelbrot set. The program implements many things in C I am fairly unfamiliar with (Structures, error handling, use of new libraries (complex.h), and files (I'm completely unfamiliar with image files)) and the purpose of writing this program is primarily to test my knowledge.
If possible, could someone tell me things I've done well here, and things I've done badly, as well as a brief summary of how to do things better in the future? I'd also quite like to know any ways I could reduce execution time. All feedback is greatly appreciated.
I've added comments to give a brief summary of each function and their purposes, as well as make some possibly unclear areas clearer.
```
//mandelbrot.c - generates a .PPM (Portable Pixmap format) file of the Mandelbrot set with shading
//Still to add: Implement a better file format, optimise to reduce time,
#include "stdio.h"
#include "complex.h"
#include "math.h"
#define MAX_TESTS 650
int error(const char *message);
struct colour mandelbrot_test(double complex c);
struct colour rgb_gen(int iterations, double complex num);
struct dimensions dim_gen(int height);
struct dimensions
{
double hinc;
double winc;
unsigned int height;
unsigned int width;
};
struct colour
{
unsigned int red;
unsigned int green;
unsigned int blue;
};
int main(int argc, char *argv[])
{
if (argc = 4)
{
return rgb_gen(i, x);
}
}
return rgb_gen(MAX_TESTS, x);
}
struct colour rgb_gen(int iterations, double complex num) //Function to generate the RGB values for a given complex number
{
struct colour rgb;
if (iterations == MAX_TESTS)
{
rgb.red = 0;
rgb.green = 0;
rgb.blue = 0;
}
else
{
double z = sqrt(creal(num) creal(num) + cimag(num) cimag(num));
int brightness = 256.0 * log2(2.75 + iterations - log2(log2(z))) / log2((double)MAX_TESTS); //this
If possible, could someone tell me things I've done well here, and things I've done badly, as well as a brief summary of how to do things better in the future? I'd also quite like to know any ways I could reduce execution time. All feedback is greatly appreciated.
I've added comments to give a brief summary of each function and their purposes, as well as make some possibly unclear areas clearer.
```
//mandelbrot.c - generates a .PPM (Portable Pixmap format) file of the Mandelbrot set with shading
//Still to add: Implement a better file format, optimise to reduce time,
#include "stdio.h"
#include "complex.h"
#include "math.h"
#define MAX_TESTS 650
int error(const char *message);
struct colour mandelbrot_test(double complex c);
struct colour rgb_gen(int iterations, double complex num);
struct dimensions dim_gen(int height);
struct dimensions
{
double hinc;
double winc;
unsigned int height;
unsigned int width;
};
struct colour
{
unsigned int red;
unsigned int green;
unsigned int blue;
};
int main(int argc, char *argv[])
{
if (argc = 4)
{
return rgb_gen(i, x);
}
}
return rgb_gen(MAX_TESTS, x);
}
struct colour rgb_gen(int iterations, double complex num) //Function to generate the RGB values for a given complex number
{
struct colour rgb;
if (iterations == MAX_TESTS)
{
rgb.red = 0;
rgb.green = 0;
rgb.blue = 0;
}
else
{
double z = sqrt(creal(num) creal(num) + cimag(num) cimag(num));
int brightness = 256.0 * log2(2.75 + iterations - log2(log2(z))) / log2((double)MAX_TESTS); //this
Solution
Several things:
-
I compiled your code on my Linux host (Centos 7) using gcc version 4.8.3
via the command line:
You need to add
-
System include files should have angle brackets instead of quotes, as others have pointed out.
-
My personal taste is to initialize all variable prior to use, this I would write
This limits the scope of your index variables to just the loops.
-
I compiled your code on my Linux host (Centos 7) using gcc version 4.8.3
via the command line:
gcc -std=c99 -pedantic -Wall mandlebrot.c -o mandlebrot -lm and I got the following warning:gcc -std=c99 -pedantic -Wall mandlebrot.c -o mandlebrot -lm
mandlebrot.c: In function ‘main’:
mandlebrot.c:42:5: warning: implicit declaration of function ‘atoi’ [-Wimplicit-function-declaration]
dim.height = atoi(argv[2]);You need to add
#include to your program. -
System include files should have angle brackets instead of quotes, as others have pointed out.
-
My personal taste is to initialize all variable prior to use, this I would write
FILE* file=NULL;
-
I dislike positional command line arguments (i.e. file name first and then height); you might want to look at the getopt library if you are using Linux\Unix (even if you are using Windows it is still worth a look).
-
You can "leak resources", you never close the file you open in 43 if the height is less than 1000. Probably not a big issue in this code sample, but you should get in the habit of releasing any resources that you acquire. For this program you can do it by rearranging your tests:
if(dim.height < 1000)
{
return error("image cannot be less than 1000px in height");
}
if(NULL != (file = fopen(file_name, "w")))
{
// ... generate mandlebrote image here....
fclose(file);
}
else
{
// ... handle failure to open file
}
-
As others have mentioned, typedef'ing your structures can make your code more readable, and save you on typing.
-
Assuming you are using a c99 compliant compiler (and seeing as how you are using the //` single line comment it appears you are), you can write your for-loops like:for (double j = -1.0; j <= 1.0; j += dim.hinc)
{
for (double i = -2.0; i <= 0.5; i += dim.winc)
{
num = i + j * I;
rgb = mandelbrot_test(num);
fprintf(file, "%d %d %d ", rgb.red, rgb.green, rgb.blue);
}
fprintf(file, "\n");
}This limits the scope of your index variables to just the loops.
Code Snippets
gcc -std=c99 -pedantic -Wall mandlebrot.c -o mandlebrot -lm
mandlebrot.c: In function ‘main’:
mandlebrot.c:42:5: warning: implicit declaration of function ‘atoi’ [-Wimplicit-function-declaration]
dim.height = atoi(argv[2]);if(dim.height < 1000)
{
return error("image cannot be less than 1000px in height");
}
if(NULL != (file = fopen(file_name, "w")))
{
// ... generate mandlebrote image here....
fclose(file);
}
else
{
// ... handle failure to open file
}for (double j = -1.0; j <= 1.0; j += dim.hinc)
{
for (double i = -2.0; i <= 0.5; i += dim.winc)
{
num = i + j * I;
rgb = mandelbrot_test(num);
fprintf(file, "%d %d %d ", rgb.red, rgb.green, rgb.blue);
}
fprintf(file, "\n");
}Context
StackExchange Code Review Q#91502, answer score: 12
Revisions (0)
No revisions yet.