Thread: part 1 of 2 questions .. should I use malloc here? 2 arrays of structs?

I have just finished writing my new mandelbrot set program and it compiles and runs .. sometimes though it segment faults when I try and increase the iteration count maximum and the escape value. I did change the way I color the set but as long as I dont press any keys and keep zooming the program works fine.

the only changes I have made from my older program which runs with no problems is that I have made more use of structures and arrays .. this leads me to wonder whether I should be allocating memory with malloc ( no experience there ).

the other question I have is should I be using a struct containing an array or use a struct then make an array of structures? at the moment I am using arrays contained in a struct but Im not sure this is the way to go and regards q1 should I be using allocated memory.

anyway heres the code anyone with any comments very welcome thanks in advance al.

Code:

#include<stdio.h>
#include<stdlib.h>
#include<math.h>
#include<SDL/SDL.h>
 
#define smooth log( 0.5 * log( window->ESCAPE ))

SDL_Surface *screen;
SDL_Event event;

//prototyping the structures .. still debating merits of one v many structs at this point in time.
struct window{
  int xscreen,yscreen,Nxscreen,Nyscreen;
  int a,b,c,d;
  unsigned long long int MAX,ESCAPE;
  long double ximage,yimage,xoff,yoff;
};

struct data{
  int Nxscreen[50],Nyscreen[50];
  long double ximage[50],yimage[50],xoff[50],yoff[50];
};

struct pxcolor{
  int r[512];
  int g[512];
  int b[512];
}rgb,*prgb;

//prototyping all the functions.

void write_data(struct data* data);
struct window *image_size( struct window* window );
struct window *window_size( struct window* window );
struct window *mouse( int z, struct window* window );
void putpixel( int x, int y, Uint32 colors );
Uint32 getcolor( int count,struct window* window, struct pxcolor* prgb );
int multiply(long double real, long double imaginary,struct window* window );
void numbers( struct window* window, struct pxcolor* prgb );
void repeat( struct window* window, struct pxcolor* prgb );
void control( struct window* window, struct data* data, struct pxcolor* prgb );
void start( );
void numbers( struct window* window, struct pxcolor* prgb );
void unlock();
void lock();
void sdl( struct window* window );

//fopen write data file; use later to draw big images.
void write_data(struct data* data){
  FILE *fp;
  int c;
  
  fp = fopen("image_info","w");
  for(c = 0;c < 50; c++){
    fprintf(fp,"c = %i\n",c);
    fprintf(fp,"Nxscreen = %i\n",data->Nxscreen[c]);
    fprintf(fp,"Nyscreen = %i\n",data->Nyscreen[c]);
    
    fprintf(fp,"ximage = %.40Lf\n",data->ximage[c]);
    fprintf(fp,"yimage = %.40Lf\n",data->yimage[c]);
    
    fprintf(fp,"xoff = %.40Lf\n",data->xoff[c]);
    fprintf(fp,"yoff = %.40Lf\n",data->yoff[c]);
  }
  fclose(fp);
}
// image size used to find new images size so it fits new zoom window

struct window *image_size( struct window* window ){
  long double s,t,u,v,z;

  z = window->ximage / window->Nxscreen;
  s = z * window->c - window->xoff;
  u = z * window->a - window->xoff;
  
  z = window->yimage / window->Nyscreen;  
  t = window->yoff - ( z * window->d );
  v = window->yoff - ( z * window->b );
  
  window->ximage = u - s;
  window->yimage = t - v;
  window->xoff = -s;
  window->yoff = t;
  return window;
}
// window size used to find new dimensions of window when zooming in

struct window *window_size( struct window* window ){

  int t;
   
  window->Nxscreen = window->a - window->c;
  window->Nyscreen = window->b - window->d;
  
  if( window->Nxscreen > window->Nyscreen ){
    t = window->xscreen / window->Nxscreen;
  }
  else{
    t = window->yscreen / window->Nyscreen;
  }

  window->Nxscreen = window->Nxscreen * t;
  window->Nyscreen = window->Nyscreen * t;
  return window;
}
// mouse uses sdl to return mousewindow events. I am using two events high left 
// to low right so I can zoom in.

struct window *mouse( int z, struct window* window ){
 
  if( z == 1 ){
    window->c = event.button.x;
    window->d = event.button.y;
  }
  if( z != 1 ){
    window->a = event.button.x;
    window->b = event.button.y;
  }
  return window;  
}
// draw pixel by pixel ..
void putpixel( int x, int y, Uint32 colors ){

  Uint32 *bufp;
  bufp = ( Uint32 *)screen->pixels + y*screen->pitch / 4 + x;
  *bufp = colors;
}
// one of many coloring schemes, currently using it due to being able to use
// bigger numbers of iterations which will come in handy later.
Uint32 getcolor( int count, struct window* window, struct pxcolor* prgb ){
  int  red = prgb->r[( count % 512 )];
  int  green = prgb->g[( count % 512 )];
  int blue = prgb->b[( count % 512 )];
  Uint32 colors;
   if(count == window->MAX){
     return( colors = SDL_MapRGB(screen->format,0,0,0));
   }
   else{
     return colors = SDL_MapRGB(screen->format,
				( blue ),( green ),( red ));
   }
}
// multiply is an iterative process which returns a number .. count which is 
// converted into a color. 
int multiply(long double real, long double imaginary, struct window* window ){

  int count = 1;
  long double x,y,z = 0,xx,yy,tempr = real,tempi = imaginary;
 
  while( count < window->MAX &&
	  abs( z ) < window->ESCAPE ){
    x = tempr * tempr;
    y = tempi * tempi;
    z = tempr * tempi;

    xx = x - y;
    yy = 2 * z;
    z = x + y;
  
    tempr = real + xx;
    tempi = imaginary + yy;
    count++;
  }
  if(count >= window->MAX ){
    return count;
  }
  else{
    return ( count - ( log( 0.5 * log( z )) - smooth ) / log( 2 ));
  }
}
// numbers takes the window and gives an imaginary number for each pixel. 
void numbers( struct window* window, struct pxcolor* prgb ){
  
  int x,y,count;
  Uint32 colors;
  long double real,imaginary, convx,convy;
 
  //do it once do it right save lots of cpu cycles for teh hard work .. sort
  //out conversion factor once rinse repeat ..

  convx =  window->ximage / window->Nxscreen;
  convy =  window->yimage / window->Nyscreen;

  for( x = 1; x < window->Nxscreen; x++ ){
    for( y = 1; y < window->Nyscreen; y++ ){
      real = convx * x - window->xoff;
      imaginary = window->yoff - ( convy * y );
      count = multiply( real, imaginary, window );
      colors = getcolor( count, window, prgb);
      putpixel( x, y, colors );
    }//redraw screen once instead of line by line ..
  }SDL_UpdateRect( screen,0,0,window->Nxscreen,window->Nyscreen );
  printf("draw finish\n");
}
//all the functions to redraw are here ..
void repeat( struct window* window, struct pxcolor* prgb){

  sdl( window );
  lock();
  numbers( window,prgb );
  unlock();
}
//control function giving some options .. increase size of screen .. iteration count and save image
//press ESC to quit.
void control(struct window* window, struct data* data, struct pxcolor* prgb ){

  int c = 1;
  int z = 1;

  while( SDL_WaitEvent( &event )){
    switch( event.type ){
    case SDL_MOUSEBUTTONDOWN:{
      if (event.button.button == SDL_BUTTON_LEFT){
      mouse( z, window );
      if( z != 1 ){

	image_size( window );
	window_size( window );

	data->Nxscreen[c] = window->Nxscreen;
	data->Nyscreen[c] = window->Nyscreen;
	data->ximage[c] = window->ximage;
	data->yimage[c] = window->yimage;
	data->xoff[c] = window->xoff;
	data->yoff[c] = window->yoff;

	c++;
	repeat( window, prgb  );
      }
         z = -z;
      }

	 break;
      }
    case SDL_KEYDOWN:{

      /***DOUBLE WINDOW SIZE***/
      if( event.key.keysym.sym == SDLK_2 ){
	window->Nxscreen = window->Nxscreen * 2;
	window->Nyscreen = window->Nyscreen * 2;
	repeat( window, prgb  );
      }

      if( event.key.keysym.sym == SDLK_3 ){
	window->MAX = 4096;
	window->ESCAPE = 100;	
	repeat( window, prgb  );

      }
      /***GO DEEPER INCREASE ITERATION COUNT***/
      if( event.key.keysym.sym == SDLK_4 ){
	window->MAX = window->MAX * 2;
	window->ESCAPE = window->ESCAPE * 2;	
	repeat( window, prgb  );
      }
      /***GO BACK HERE***/
      if( event.key.keysym.sym == SDLK_b ){
	c = c - 1;
	window->Nxscreen = data->Nxscreen[c];
	window->Nyscreen = data->Nyscreen[c];

	window->ximage = data->ximage[c];
	window->yimage = data->yimage[c];

	window->xoff = data->xoff[c];
	window->yoff = data->yoff[c];
	repeat( window, prgb  );
      }
      //press s for save as shot.bmp ..
      /***SAVE HERE***/

      if( event.key.keysym.sym == SDLK_s ){
	SDL_SaveBMP( screen, "shot.bmp" );
      } 
      /***QUIT***/     
      if( event.key.keysym.sym == SDLK_ESCAPE ){
	write_data(data);
	return( SDL_Quit());
      }
      break;
    }
    }}

}
// all the sdl processes here.
void unlock(){

   if( SDL_MUSTLOCK( screen )){   
    SDL_UnlockSurface( screen );
  }
}
void lock(){
           
  if( SDL_MUSTLOCK( screen )){
    if( SDL_LockSurface( screen ) < 0 ){
      return;
    }
  }
}
void sdl( struct window* window ){

  SDL_Init( SDL_INIT_VIDEO | SDL_INIT_NOPARACHUTE );
  SDL_WM_SetCaption("smooth.c",NULL);
 
              screen = SDL_SetVideoMode
		( window->Nxscreen,window->Nyscreen,
		  32,SDL_SWSURFACE );  
}
//setting some values for the original picture of the mandelbrot set ..
void start( ){
 
  int c;
  // creating structures and giving terms for the first window .. image and color.
  struct window wdw;
  struct window *window;
  window = &wdw;

  struct data dta;
  struct data *data;
  data = &dta;

  struct pxcolor arraycolor;
  prgb = &arraycolor;

  window->MAX = 128;
  window->ESCAPE = 4;
  window->xscreen = 809;
  window->yscreen = 500;
  window->Nxscreen = window->xscreen;
  window->Nyscreen = window->yscreen;
  window->ximage = 3.2;
  window->yimage = 2.0;
  window->xoff = 2.2;
  window->yoff = 1.0;

  //initialise all data struct elements to zero
  for(c = 0; c < 50; c++){
    data->Nxscreen[c] = 0;
    data->Nyscreen[c] = 0;
    data->ximage[c] = 0;
    data->yimage[c] = 0;
    data->xoff[c] = 0;
    data->yoff[c] = 0;
  }
  //make sure initial image is avaliable so no further attempt to go back is made.
  data->Nxscreen[0] = 809;
  data->Nyscreen[0] = 500;
  data->ximage[0] = 3.2;
  data->yimage[0] = 2.0;
  data->xoff[0] = 2.2;
  data->yoff[0] = 1.0;
  //initialise all color elements to zero 
  for( c = 0; c < 512; c++ ){
    prgb->r[c] = 0;
    prgb->g[c] = 0;
    prgb->b[c] = 0;
  }
  //provide colors
  for( c = 0; c < 512; c++ ){
    prgb->r[c] = c % 256;
    prgb->g[c] = ( c + 32 ) % 256;
    prgb->b[c] = ( c + 64 ) % 256;
  }
  repeat( window, prgb );
  control( window, data, prgb );  
}
int main( int argc, char * argv[] ){
  
  start();
  return(0);
}

As for your second question: if you have one thing, that needs to have 850 of the same thing inside it, then that's an array inside a struct. If you have 850 of the same things, and each of those things has one of these, and one of that, etc., then you have an array of structs. I have no idea what the things inside some of your structs are supposed to be (a, b, c, and d?) so who knows if they make sense. (Although it is pretty clear that struct pxcolor does NOT make sense.)

You've got enough hard-coded numbers that who knows what they all mean either, but one of them is probably unhappy when you increase your MAX or whatever.

The first makes more sense now, since a pixel contains one each of r, g, b and you have a bunch of pixels. I have no idea what this "data" thing represents to you, so I have no idea whether it makes sense to contain arrays or for you to have an array of them. But if each data thing is logically one unit, and then you need many of them, then yes you should make it into an array of structs.

You have many hard-coded values in your program: 809, 500, 512, 50, 32, 64, 256, 100, 3.2, 2.2, 4, 128. These numbers are written directly in your program, so it's impossible to say what they mean, and also they will never change when your variables change.

ah ok yes some of them are persistant .. 809 and 500 are the reference width and height for resizing the screen.
512 is the number of colors in the array made up of red green and blue which ,
50 is a number for the data struct to hold info wtih each zoom this in excess of what I think you could use zooming into the set without using bignums .. I write this info out at the end of the program so that if I see something and I want a real big pic I can use my other program that does not use sdl and runs in the background
the 32 is part of the sdl code .. it relates to the number of colors I can use ..
the 128 and 4 are the initial conditons for iteration loop maximum and bailout value .. later when zooming in the detail is not good so I have provided a method to increase both .. pressing 3 goes directly to 4096 and 100 and pressing 4 doubles each of these values ..
3.2, 2.2 are the starting corners and size of the image in the imaginary plane .. which is not the the same as the screen size .. when you zoom in these values change ..

I thought that using structs was a good way to keep a lot of similar information together and have a pointer to it in more than one function ..

appreciate any adice thanks al.