NextFrame optimization

While I was browsing my previous posts I noticed I had post the code for my “nextFrame” function (which is based on Arduboy’s NextFrame)

unsigned long lastFrameStart;
unsigned long nextFrameStart;
bool justRendered;
uint8_t lastFrameDurationMs;
uint16_t frameCount;
uint16_t frameDiff;

bool nextFrame
(void)
{
  unsigned long now = millis();

  if (justRendered) {
    lastFrameDurationMs = now - lastFrameStart;
    justRendered = false;
    return false;
  }
  else if (now < nextFrameStart) {
    if ((uint8_t)(nextFrameStart - now) > 1)
      delay(1);
    return false;
  }

  // pre-render
  justRendered = true;
  lastFrameStart = now;
  nextFrameStart = now + frameDiff;
  frameCount++;
  //if (frameCount > 20000)
    //frameCount = frameCount - 20000;
  return true;
}

some time later another person noted that it is basically a souped-up version of the “blink” sketch. because you can certainly do other things while a frame is not being drawn.
I thought "yeah the code should be pretty simple. But in addition to sleep (which is turned to delay because arduino Due do not have a sleep function yet) we also do some other things …
so I propped it open and noticed this chunk:

if (justRendered) {
    lastFrameDurationMs = now - lastFrameStart;
    justRendered = false;
    return false;
  }

which is basically saying that if we just draw a frame, we invalidate the flag and return.
this is assuming the fact that the last frame is not taking longer than it should (and it might not), and the … well, it only save one loop (from doing the comparison between now and nextFrameStart, doesn’t it?
Now of course, if most code is frame-linked then there is little code outside of the loop so the next loop will quickly return, doing the time comparison.
What I suggest is that we eliminate this altogether.

unsigned long nextFrameStart;
uint16_t frameCount;
uint16_t frameDiff;


bool nextFrame
(void)
{
  unsigned long now = millis();
  if (now < nextFrameStart) {
    if ((uint8_t)(nextFrameStart - now) > 1)
      delay(1);
    return false;
  }
  // pre-render
  nextFrameStart = now + frameDiff;
  frameCount++;
  //if (frameCount > 20000)
    //frameCount = frameCount - 20000;
  return true;
}

However a undesirable side effect is that we removed the frametime calculator (which is somewhat meaningful as a mean of calculating elapsed time) but it is indeed useless if this frametime is not used anywhere else.
the other way would be to do

bool nextFrame
(void)
{
  unsigned long now = millis();
  if (justRendered) {
    lastFrameDurationMs = now - lastFrameStart;
    justRendered = false;
  }

  if (now < nextFrameStart) {
    if ((uint8_t)(nextFrameStart - now) > 1)
      delay(1);
    return false;
  }
...
...
}

so that we don’t have to return false and wait to loop another time around.

Firstly, your code doesn’t quite represent what’s actually implemented in Arduboy2:

lastFrameDurationMs can’t be completely eliminated because nextFrameDEV depends on it:

Though it ought be possible to eliminate it from nextFrame by moving most of the logic into nextFrameDEV.

The proper equivalent would be:

bool Arduboy2Base::nextFrame()
{
	uint8_t now = (uint8_t) millis();
	uint8_t frameDurationMs = now - thisFrameStart;

	if (justRendered)
	{
		lastFrameDurationMs = frameDurationMs;
		justRendered = false;
	}
	
	if (frameDurationMs < eachFrameMillis)
	{
		++frameDurationMs;

		// Only idle if at least a full millisecond remains, since idle() may
		// sleep the processor until the next millisecond timer interrupt.
		if (frameDurationMs < eachFrameMillis)
			idle();

		return false;
	}

	// pre-render
	justRendered = true;
	thisFrameStart = now;
	frameCount++;

	return true;
}

It would seem at first glance that there’s no downside to this, but I may be missing something.


A version in which nextFrame and nextFrameDEV had entirely separate implementations would look like this:

bool Arduboy2Base::nextFrame()
{
	uint8_t now = (uint8_t) millis();
	uint8_t frameDurationMs = now - thisFrameStart;
	
	if (frameDurationMs < eachFrameMillis)
	{
		++frameDurationMs;

		// Only idle if at least a full millisecond remains, since idle() may
		// sleep the processor until the next millisecond timer interrupt.
		if (frameDurationMs < eachFrameMillis)
			idle();

		return false;
	}

	// pre-render
	thisFrameStart = now;
	frameCount++;

	return true;
}

bool Arduboy2Base::nextFrameDEV()
{
	uint8_t now = (uint8_t) millis();
	uint8_t frameDurationMs = now - thisFrameStart;

	if (justRendered)
	{
		lastFrameDurationMs = frameDurationMs;
		justRendered = false;
		return false;
	}
	
	if (frameDurationMs < eachFrameMillis)
	{
		++frameDurationMs;

		// Only idle if at least a full millisecond remains, since idle() may
		// sleep the processor until the next millisecond timer interrupt.
		if (frameDurationMs < eachFrameMillis)
			idle();

		return false;
	}

	// pre-render
	justRendered = true;
	thisFrameStart = now;
	frameCount++;
	
	if (lastFrameDurationMs > eachFrameMillis)
		TXLED1;
	else
		TXLED0;

	return true;
}
1 Like