SDL_FillRect problem

I think there is something wrong but I can't figure out what.

I have an function that calls SDL_FillRect to fill the main screen with one colour but if I do that, the program just crashes and the log file is empty even though it was supposed to be written prior to the call!.

If I don't, the window is created normally and the log file is written.

Code:

---

int main(int argc, char* argv[]) {
  SDL_Surface* screen;    /* The screen that we draw on */
  SDL_Event event;        /* Events that we handle in our game loop */
  int done = 0;          /* The control for the game loop */
 
  logSetup();
  if(SDL_Init(SDL_INIT_EVERYTHING) == -1) {
    logWrite("Failed to initialise SDL: %s", SDL_GetError());
    SAFE_EXIT(-1);
  }
  initVideo(screen, 640, 480);
 
  SDL_WM_SetCaption(title, NULL);
  fillSurface(screen, FILL_WHITE); <- problem here
 
  while(!done) {
    while(SDL_PollEvent(&event)) {
      switch(event.type) {
        case SDL_QUIT: {
          logWrite("Application exit.");
          done = 1;
        }
      }
    }
  }
  SDL_Quit();
  logClose();
  return 0;
}

---

Code:

---

#ifndef SYSTEM_H
#define SYSTEM_H

...

/*
  This function sets the optimal video mode that the graphics adapter can
  handle. The parameter w and h indicates the video mode width and height
  that you want, if it isn't avaliable then the function would use what is
  avaliable. The function will automatically get the pixel format of the
  hardware.
*/
void initVideo(SDL_Surface* src, int w, int h);

...

/*
  These macros defines common RGB values to be passes to SDL_MapRGB.
*/
#define FILL_BLACK 0x00, 0x00, 0x00
#define FILL_WHITE 0xff, 0xff, 0xff
#define FILL_RED  0xff, 0x00, 0x00
#define FILL_GREEN 0x00, 0xff, 0x00
#define FILL_BLUE  0x00, 0x00, 0xff

void fillSurface(SDL_Surface* src, Uint8 R, Uint8 G, Uint8 B);

#endif

---

Code:

---

#include "system.h"

void initVideo(SDL_Surface* src, int w, int h) {
  const SDL_VideoInfo* info = SDL_GetVideoInfo();
  SDL_Rect** modes;
  Uint32 flags = 0, init = 0;
  Sint32 i;
  char driver[255];
  init = SDL_WasInit(SDL_INIT_VIDEO);
  if(init & SDL_INIT_VIDEO) {
    SDL_VideoDriverName(driver, 255);
    logWrite("Video initialised.");
    logWrite("Video driver name: %s", driver);
    if(info->hw_available) {  /* Check if video RAM is avaliable */
      flags |= SDL_HWSURFACE; /* If yes, use video RAM */
      flags |= SDL_DOUBLEBUF; /* Only hardware could use this */
      flags |= SDL_HWPALETTE; /* Exclusive palette access */
      logWrite("Hardware surface avaliable, using video memory.");
    } else {
      flags |= SDL_SWSURFACE; /* Use system RAM */
    }
    modes = SDL_ListModes(info->vfmt, flags); /* Get list of avaliable resolutions */
    if(modes == (SDL_Rect**)0) {              /* No resolutions avaliable */
      logWrite("No resolutions avaliable.");
      SAFE_EXIT(-1);
    } else if(modes == (SDL_Rect**)-1) {      /* All resolutions avaliable */
      logWrite("All resolution avaliable.");
      /* So we use the desired resolution */
      src = SDL_SetVideoMode(w, h, info->vfmt->BitsPerPixel, flags);
    } else {                      /* Only if we are limited to some resolutions */
      for(i = 0; modes[i]; ++i) { /* Find out how many avaliable resolutions */
      }
      i /= 2; /* Choose the optimal medium , not too large, not too small mode */
      src = SDL_SetVideoMode(modes[i]->w, modes[i]->h, info->vfmt->BitsPerPixel, flags);
    }
    if(src == NULL) {
      logWrite("Unable to set video mode: %s", SDL_GetError());
      SAFE_EXIT(-1);
    } else {
      logWrite("Video mode: %d x %d, %d bits per pixel", src->w, src->h, src->format->BitsPerPixel);
    }
    return;
  }
  SAFE_EXIT(-1);
}

...

void unlock(SDL_Surface* src) {
  if(SDL_MUSTLOCK(src)) {
    SDL_UnlockSurface(src);
  }
  return;
}

void fillSurface(SDL_Surface* src, Uint8 R, Uint8 G, Uint8 B) {
  Uint32 color = SDL_MapRGB(src->format, R, G, B);
  if(SDL_FillRect(src, NULL, color) == -1) {
    logWrite("Unable to fill surface: %s", SDL_GetError());
    SAFE_EXIT(-1);
  }
  SDL_UpdateRect(src, 0, 0, 0, 0);
  return;
}

---

I've cut the useless working bits out, I am concerned about the initialising function, because it does it's job when fillSurface() is not called but if it is, it doesn't seen to do anything because the log file isn't written! I'm sure there must be something wrong but I've already checked and tested so many times.

Please help! I can't continue writing the program if I can't sort this out.

Quote:

---

the program just crashes and the log file is empty even though it was supposed to be written prior to the call!

---

Flush the log file. Any data in the buffer will be written to the file. Use fflush() for this.

Code:

---

  initVideo(screen, 640, 480);
 
  SDL_WM_SetCaption(title, NULL);
  fillSurface(screen, FILL_WHITE); <- problem here

---

You can't expect screen to be assigned to the surface to write to; because of the way it's passed to initVideo, initVideo can't make it point to anything. You need to pass a pointer-to-a-pointer, or return the modified value. That is, you need either

Code:

---

void initVideo(SDL_Surface** src, int w, int h);

---

or

Code:

---

SDL_Surface *initVideo(int w, int h);

---

The second has simpler syntax, but a return value can be ignored.

Quote:

---

If I don't, the window is created normally and the log file is written.

---

This is because you never try to access screen; and since the program doesn't crash, the data in the buffer is written to the log file when you close it or when the program exits.

[edit] BTW, excellent post. I was able to figure out the problem with no extra information required. :) [/edit]

Ah, fixed, thanks.

BTW, why couldn't it make it point to something else? SDL_SetVideoMode returns a pointer, why can't I assign that to my pointer?

Because you don't pass the address of screen to initVideo. The function therefore has no way to change the value of screen. It is passed the value of screen, and can modify that in its own copy of the variable, but it can't change the original. Removing a level of indirection, the problem your code illustrates is like the following:

Code:

---

#include <stdio.h>

void increment(int x) {
    x ++;
}

int main(void) {
    int x = 1;

    increment(x);

    printf("%i\n", x);  /* prints 1 */

    return 0;
}

---

You'd need to add a level of indirection to the parameter to increment(), or return the modified value, to get the program to do what was intended.

Ok, thanks for the explaination. I've figured it out through the provided example. Pass argument by reference!

So if I had passed a SDL_Surface** src, in the code I should assign *src = SDL_SetVideoMode(...) to make them pointing at the same SDL_Surface.