From 3ddc1a1722c9ef42d506eecbeb59929dce251018 Mon Sep 17 00:00:00 2001
From: Thomas Harte <thomas.harte@gmail.com>
Date: Thu, 25 Jun 2020 23:59:44 -0400
Subject: [PATCH] Eliminates hard-coded concept of timer jitter.

---
 ClockReceiver/VSyncPredictor.hpp   | 44 ++++++++++++++++++++++++++----
 OSBindings/Qt/scantargetwidget.cpp | 10 ++++++-
 OSBindings/Qt/scantargetwidget.h   |  2 ++
 3 files changed, 50 insertions(+), 6 deletions(-)

diff --git a/ClockReceiver/VSyncPredictor.hpp b/ClockReceiver/VSyncPredictor.hpp
index f133762c1..d30f837bc 100644
--- a/ClockReceiver/VSyncPredictor.hpp
+++ b/ClockReceiver/VSyncPredictor.hpp
@@ -11,24 +11,38 @@
 
 #include "TimeTypes.hpp"
 #include <cmath>
+#include <cstdio>
 
 namespace Time {
 
 /*!
 	For platforms that provide no avenue into vsync tracking other than block-until-sync,
-	this class tracks: (i) how long frame draw takes; and (ii) the apparent frame period; in order
-	to suggest when you should next start drawing.
+	this class tracks: (i) how long frame draw takes; (ii) the apparent frame period; and
+	(iii) optionally, timer jitter; in order to suggest when you should next start drawing.
 */
 class VSyncPredictor {
 	public:
+		/*!
+			Announces to the predictor that the work of producing an output frame has begun.
+		*/
 		void begin_redraw() {
 			redraw_begin_time_ = nanos_now();
 		}
 
+		/*!
+			Announces to the predictor that the work of producing an output frame has ended;
+			the predictor will use the amount of time between each begin/end pair to modify
+			its expectations as to how long it takes to draw a frame.
+		*/
 		void end_redraw() {
 			redraw_period_.post(nanos_now() - redraw_begin_time_);
 		}
 
+		/*!
+			Informs the predictor that a block-on-vsync has just ended, i.e. that the moment this
+			machine calls retrace is now. The predictor uses these notifications to estimate output
+			frame rate.
+		*/
 		void announce_vsync() {
 			const auto vsync_time = nanos_now();
 			if(last_vsync_) {
@@ -37,16 +51,35 @@ class VSyncPredictor {
 			last_vsync_ = vsync_time;
 		}
 
+		/*!
+			Adds a record of how much jitter was experienced in scheduling; these values will be
+			factored into the @c suggested_draw_time if supplied.
+
+			A positive number means the timer occurred late. A negative number means it occurred early.
+		*/
+		void add_timer_jitter(Time::Nanos jitter) {
+			timer_jitter_.post(jitter);
+		}
+
+		/*!
+			Announces to the vsync predictor that output is now paused. This ends frame period
+			calculations until the next announce_vsync() restarts frame-length counting.
+		*/
 		void pause() {
 			last_vsync_ = 0;
 		}
 
+		/*!
+			@return The time at which redrawing should begin, given the predicted frame period, how
+			long it appears to take to draw a frame and how much jitter there is in scheduling
+			(if those figures are being supplied).
+		*/
 		Nanos suggested_draw_time() {
-			const auto mean = (vsync_period_.mean() - redraw_period_.mean()) / 1;
-			const auto variance = (vsync_period_.variance() + redraw_period_.variance()) / 1;
+			const auto mean = vsync_period_.mean() - redraw_period_.mean() - timer_jitter_.mean();
+			const auto variance = vsync_period_.variance() + redraw_period_.variance() + timer_jitter_.variance();
 
 			// Permit three standard deviations from the mean, to cover 99.9% of cases.
-			const auto period = mean + Nanos(3.0f * sqrt(float(variance)));
+			const auto period = mean - Nanos(3.0f * sqrt(float(variance)));
 
 			return last_vsync_ + period;
 		}
@@ -94,6 +127,7 @@ class VSyncPredictor {
 
 		VarianceCollector vsync_period_{1'000'000'000 / 60};	// 60Hz: seems like a good first guess.
 		VarianceCollector redraw_period_{1'000'000'000 / 60};	// A less convincing first guess.
+		VarianceCollector timer_jitter_{0};						// Seed at 0 in case this feature isn't used by the owner.
 };
 
 }
diff --git a/OSBindings/Qt/scantargetwidget.cpp b/OSBindings/Qt/scantargetwidget.cpp
index 5d5ebc1b1..8fe604a8f 100644
--- a/OSBindings/Qt/scantargetwidget.cpp
+++ b/OSBindings/Qt/scantargetwidget.cpp
@@ -17,6 +17,11 @@ void ScanTargetWidget::initializeGL() {
 }
 
 void ScanTargetWidget::paintGL() {
+	if(requested_redraw_time_) {
+		vsyncPredictor.add_timer_jitter(Time::nanos_now() - requested_redraw_time_);
+		requested_redraw_time_ = 0;
+	}
+
 	glClear(GL_COLOR_BUFFER_BIT);
 
 	// Gmynastics ahoy: if a producer has been specified or previously connected then:
@@ -60,10 +65,12 @@ void ScanTargetWidget::vsync() {
 	vsyncPredictor.announce_vsync();
 
 	const auto time_now = Time::nanos_now();
-	const auto delay_time = ((vsyncPredictor.suggested_draw_time() - time_now) / 1'000'000) - 5;	// TODO: the extra 5 is a random guess.
+	requested_redraw_time_ = vsyncPredictor.suggested_draw_time();
+	const auto delay_time = (requested_redraw_time_ - time_now) / 1'000'000;
 	if(delay_time > 0) {
 		QTimer::singleShot(delay_time, this, SLOT(repaint()));
 	} else {
+		requested_redraw_time_ = 0;
 		repaint();
 	}
 }
@@ -83,6 +90,7 @@ void ScanTargetWidget::stop() {
 	isConnected = false;
 	setDefaultClearColour();
 	vsyncPredictor.pause();
+	requested_redraw_time_ = 0;
 }
 
 void ScanTargetWidget::setDefaultClearColour() {
diff --git a/OSBindings/Qt/scantargetwidget.h b/OSBindings/Qt/scantargetwidget.h
index cfc8e0449..846370a77 100644
--- a/OSBindings/Qt/scantargetwidget.h
+++ b/OSBindings/Qt/scantargetwidget.h
@@ -34,6 +34,8 @@ class ScanTargetWidget : public QOpenGLWidget
 		GLuint framebuffer = 0;
 		MachineTypes::ScanProducer *producer = nullptr;
 
+		Time::Nanos requested_redraw_time_ = 0;
+
 		void setDefaultClearColour();
 
 	private slots: