Controlling frame rate

Question

I'm creating a bunch of threads that need to do work in a frame cycle. I would like to control how many frames are done in a second. I simplified the code I have into this so I can show you what I have wrote

// setup the frame timer
std::chrono::time_point<std::chrono::system_clock> start = std::chrono::system_clock::now();
std::chrono::time_point<std::chrono::system_clock> end = std::chrono::system_clock::now();

while(running == true)
{
    // update timer
    start = std::chrono::system_clock::now();
    std::chrono::duration<double> elapsed_seconds = start - end;
    double frameTime = elapsed_seconds.count();

    this->Work(frameTime);

    // update timer
    std::chrono::time_point<std::chrono::system_clock> afterWork = std::chrono::system_clock::now();
    std::chrono::duration<double> elapsedWorkTime = afterWork - end ;

    const double minWorkTime = 1000 / this->timer.NumberOfFramePerSeconds;
    if(elapsedWorkTime.count() < minWorkTime)
    {
        double timeToSleep = minWorkTime - elapsedWorkTime.count();
        std::this_thread::sleep_for(std::chrono::milliseconds((int)timeToSleep));
    }

    // update fps
    end = start;
    timer.FrameCount += 1;
}

Not all threads have an equal amount of work, some have more than others and so without the sleep, I would get results that are around this Thread 1 : 150fps, Thread 2: 5000fps, Thread 3: 5000fps, Thread 4: 5000fps

What I want is to be able to set the frames per second to 60, and so using the code above I would set the this->timer.NumberOfFramePerSeconds to 60. My problem is that when I do that I end up with a result like this Thread 1 : 30fps, Thread 2: 60fps, Thread 3: 60fps, Thread 4: 60fps

This suggests that there is something wrong with my code as thread 1 will happily do over 150 frames when I comment out the sleep, but when the sleep is there it will work below the 60 I'm trying to achieve.

Is my code/algorithm correct?

Answer 1

It looks to me that you likely have a units conversion error. This is typically caused by handling units of time outside of the chrono type system, and introducing explicit conversion factors. For example:

double frameTime = elapsed_seconds.count();
this->Work(frameTime);

Can Work be modified to take duration<double> instead of double? This will help ensure that double is not reinterpreted as something other than seconds inside of Work. If for some reason it can not, you can at least reduce your exposure to error with:

this->Work(elapsed_seconds.count());

The following looks very suspicious:

std::chrono::duration<double> elapsedWorkTime = afterWork - end ;

const double minWorkTime = 1000 / this->timer.NumberOfFramePerSeconds;
if(elapsedWorkTime.count() < minWorkTime)

elapsedWorkTime clearly has units of seconds. minWorkTime appears to have units of milliseconds. The if throws away all units information. This should look like:

std::chrono::duration<double> minWorkTime(1./this->timer.NumberOfFramePerSeconds);
if(elapsedWorkTime < minWorkTime)

Or if you really want minWorkTime to represent milliseconds (which is not necessary):

std::chrono::duration<double, std::milli> minWorkTime(1000./this->timer.NumberOfFramePerSeconds);
if(elapsedWorkTime < minWorkTime)

I recommend the former because the introduction of 1000 is just another chance for an error to creep in.

    double timeToSleep = minWorkTime - elapsedWorkTime.count();
    std::this_thread::sleep_for(std::chrono::milliseconds((int)timeToSleep));

Here again you are unnecessarily leaving the safety net of chrono and doing things manually. This should instead simply be:

    std::this_thread::sleep_for(minWorkTime - elapsedWorkTime);

Or if you want to be more verbose:

    auto timeToSleep = minWorkTime - elapsedWorkTime;
    std::this_thread::sleep_for(timeToSleep);

The use of .count() should be rare. It is currently necessary to use it when printing out durations. It can also be necessary to deal with legacy code which can not be made to work with chrono. If you find yourself using .count() more than that, step back and try to remove it. chrono is there to help prevent unit conversion errors, and when you exit the system via .count(), you subvert chrono's reason for existing.

Update

VS2013 has a bug such that it won't sleep on double-based durations. So as a workaround...

std::this_thread::sleep_for(std::chrono::duration_cast
                <std::chrono::milliseconds>(minWorkTime - elapsedWorkTime));

This is ugly, but still leaves all the conversion factors to chrono.

Thanks to Caesar for testing this workaround out for me.