5
\$\begingroup\$

I am a relatively novice programmer, and I am taking steps into graphics programming. I am writing a quick pong clone and want to cap the FPS to save system resources. This is how I have implemented this (I've omitted other code for brevity)

#include <stdio.h>
#include <stdbool.h>
#include "SDL.h"
#include "Windows.h"
#define WIN32_LEAN_AND_MEAN  
#define FPS 30 

Uint32 starttime, endtime, deltatime;

int main()
{
 //initialize SDL and do other stuff
 //start game loop
 timeBeginPeriod(1);
 while (appisrunning)
 {
 starttime = GetTickCount();
    SDL_PollEvent(&event);
    if (event.type == SDL_QUIT)
    {
        SDL_DestroyRenderer(renderer);
        SDL_DestroyWindow(window);
        SDL_Quit();
        appisrunning = false;
        break;
    }
 // render stuff here
 endtime = GetTickCount();
    if (endtime > starttime)
    {
        deltatime = endtime - starttime;
    }
    else // handles wraparound
    {
        deltatime = ((2 ^ 32) - starttime) + endtime;
    }
    if (deltatime > (1000 / FPS)) {} 
    else
    {
        Sleep((1000 / FPS) - deltatime);
    }
 }
 timeEndPeriod(1);
 return 0;
}

I use timeBeginPeriod(1) in an effort to give Sleep() a ~1ms resolution. According to the MSDN documentation, this function sets the timer resolution system wide and can result in higher CPU usage as the scheduler has to switch tasks more often (if I understand correctly).

Is this the right approach for this task? What feedback do you have for me?

\$\endgroup\$
  • 1
    \$\begingroup\$ Doesn't SDL have abstractions to keep your code portable (i.e. remove #include "Windows.h")? \$\endgroup\$ – Toby Speight Jan 17 at 8:52
  • \$\begingroup\$ GetTickCount() is not affected by timeBeginPeriod(). Moreover to count FPS (~33 ms) the default system timer resolution is more than adequate. \$\endgroup\$ – Adriano Repetti Jan 17 at 15:09
2
\$\begingroup\$

Code does not handle "wrap-around" correctly.

2 ^ 32 is the exclusive-or of 32 and 2. That has the value of 34. Instead, as unsigned math is defined to wrap-around, just subtract.

//if (endtime > starttime) {
//    deltatime = endtime - starttime;
//} else {
//    deltatime = ((2 ^ 32) - starttime) + endtime;
//}

deltatime = endtime - starttime;

Missing objects

event, appisrunning, renderer, window are not obviously declared before use in SDL_PollEvent(&event);. Perhaps an SDL thing?

Unnecessary use of global variables

Uint32 starttime, endtime, deltatime; belongs in main(). Or even better, within the loop.

Use an auto formatter

The below code hints at manual formatting. Use an auto formatter and save time.

 // render stuff here
 endtime = GetTickCount();
    if (endtime > starttime)

An auto formatter would have the below and constant indent amount per level (rather than 2 and sometimes 4 spaces).

Example re-format

#include <stdio.h>
#include <stdbool.h>
#include "SDL.h"
#include "Windows.h"

#define WIN32_LEAN_AND_MEAN
#define FPS 30

int main(void) {
  timeBeginPeriod(1);
  while (appisrunning) {
    Uint32 starttime = GetTickCount();
    SDL_PollEvent(&event);

    if (event.type == SDL_QUIT) {
      SDL_DestroyRenderer(renderer);
      SDL_DestroyWindow(window);
      SDL_Quit();
      appisrunning = false;
      break;
    }

    Uint32 endtime = GetTickCount();
    Uint32 deltatime = endtime - starttime;

    if (deltatime > (1000 / FPS)) {
    } else {
      Sleep((1000 / FPS) - deltatime);
    }
  }
  timeEndPeriod(1);
  return 0;
}
\$\endgroup\$
  • \$\begingroup\$ Thanks for pointing out the unsigned math. The "missing objects" were declared earlier in my code. I just left that out for brevity. I agree that the auto formatted code does look cleaner. I think part of the inconsistent indentation was introduced when I was transferring the code to this site. \$\endgroup\$ – Jason Mills Jan 18 at 3:22
  • \$\begingroup\$ @JasonMills "I just left that out for brevity." --> See Check that your code works. \$\endgroup\$ – chux Jan 18 at 4:01
  • \$\begingroup\$ I'll keep that in mind for the future. \$\endgroup\$ – Jason Mills Jan 18 at 4:59
2
\$\begingroup\$

As noted by Toby Speight in the comments, there is no need to use Windows.h. SDL provides SDL_GetTicks() for timing, and SDL_Delay() for waiting on the current thread.


The SDL cleanup code should be outside the main loop. There will probably be other reasons to exit (e.g. pressing the escape key or clicking on a quit button), and we don't want to duplicate that code.


Limiting the frame rate:

  • For limiting the rate of physics in a game, standard practice is to use an accumulator and a fixed update time-step, as in the penultimate code listing in this article.

  • For limiting the rate of rendering, we can make sure vsync is turned on.

  • For limiting the overall frame-rate to prevent excessive CPU consumption (and the rate at which we poll events) there is no clear consensus. For my own opinion, see below.

  • Note that timeBeginPeriod() has some serious disadvantages, as mentioned in the MSDN docs:

    Setting a higher resolution can improve the accuracy of time-out intervals in wait functions. However, it can also reduce overall system performance, because the thread scheduler switches tasks more often. High resolutions can also prevent the CPU power management system from entering power-saving modes.

    Creating issues like these for the entire system is arguably worse than consuming a little extra CPU.


opinion:

For a PC game, don't limit the frame rate while the user's attention is on the game. Generally we expect games to run smoothly and respond immediately to user input, and since they occupy the user's full attention, consumption of resources isn't an issue.

If the user minimizes or unfocuses the game window, they want to use resources for something else, so we should use Sleep or SDL_Delay to consume less CPU. It doesn't matter if we wake up slightly late and have a choppy frame rate, because the user isn't paying attention, so there's no reason to call timeBeginPeriod. (The physics will still behave properly if we've fixed the time step, and we may even have paused the game so there's not much to update anyway).

\$\endgroup\$

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.