From 7e39550fc078f07f12a40e136e67fd6e5517b1f2 Mon Sep 17 00:00:00 2001
From: Thomas Harte <thomas.harte@gmail.com>
Date: Sun, 15 Nov 2020 21:58:18 -0500
Subject: [PATCH] Attempts to make JustInTimeActor sequence-point aware.

With the objective of chopping out a lot of future boilerplate.
---
 ClockReceiver/JustInTime.hpp           | 38 ++++++++++++++++++++------
 Machines/Apple/AppleIIgs/AppleIIgs.cpp |  5 ++--
 Machines/Atari/ST/AtariST.cpp          | 11 +++-----
 3 files changed, 36 insertions(+), 18 deletions(-)

diff --git a/ClockReceiver/JustInTime.hpp b/ClockReceiver/JustInTime.hpp
index b1de4a0d8..6929bf3de 100644
--- a/ClockReceiver/JustInTime.hpp
+++ b/ClockReceiver/JustInTime.hpp
@@ -21,6 +21,9 @@
 
 	Machines that accumulate HalfCycle time but supply to a Cycle-counted device may supply a
 	separate @c TargetTimeScale at template declaration.
+
+	If the held object implements get_next_sequence_point() then it'll be used to flush implicitly
+	as and when sequence points are hit. Callers can use will_flush() to predict these.
 */
 template <class T, int multiplier = 1, int divider = 1, class LocalTimeScale = HalfCycles, class TargetTimeScale = LocalTimeScale> class JustInTimeActor {
 	public:
@@ -28,13 +31,20 @@ template <class T, int multiplier = 1, int divider = 1, class LocalTimeScale = H
 		template<typename... Args> JustInTimeActor(Args&&... args) : object_(std::forward<Args>(args)...) {}
 
 		/// Adds time to the actor.
-		forceinline void operator += (const LocalTimeScale &rhs) {
+		forceinline void operator += (LocalTimeScale rhs) {
 			if constexpr (multiplier != 1) {
 				time_since_update_ += rhs * multiplier;
 			} else {
 				time_since_update_ += rhs;
 			}
 			is_flushed_ = false;
+
+			if constexpr (has_sequence_points<T>::value) {
+				time_until_event_ -= rhs;
+				if(time_until_event_ < LocalTimeScale(0)) {
+					flush();
+				}
+			}
 		}
 
 		/// Flushes all accumulated time and returns a pointer to the included object.
@@ -67,23 +77,35 @@ template <class T, int multiplier = 1, int divider = 1, class LocalTimeScale = H
 					if(duration > TargetTimeScale(0))
 						object_.run_for(duration);
 				}
+
+				if constexpr (has_sequence_points<T>::value) {
+					time_until_event_ = object_.get_next_sequence_point();
+					assert(time_until_event_ > LocalTimeScale(0));
+				}
 			}
 		}
 
-		/// Flushes only if the next window of size @c threshold (measured in LocalTimeScale) has been crossed since the last flush.
-		template<int threshold> forceinline bool check_flush_threshold() {
-			if(time_since_update_.as_integral() >= threshold) {
-				flush();
-				return true;
-			} else {
+		/// Indicates whether a sequence-point-caused flush will occur if the specified period is added.
+		forceinline bool will_flush(LocalTimeScale rhs) const {
+			if constexpr (!has_sequence_points<T>::value) {
 				return false;
 			}
+			return rhs >= time_until_event_;
+		}
+
+		/// @returns the number of cycles until the next sequence-point-based flush, if the embedded object
+		/// supports sequence points; @c LocalTimeScale() otherwise.
+		LocalTimeScale cycles_until_implicit_flush() const {
+			return time_until_event_;
 		}
 
 	private:
 		T object_;
-		LocalTimeScale time_since_update_;
+		LocalTimeScale time_since_update_, time_until_event_;
 		bool is_flushed_ = true;
+
+		template <typename S, typename = void> struct has_sequence_points : std::false_type {};
+		template <typename S> struct has_sequence_points<S, decltype(void(std::declval<S &>().get_next_sequence_point()))> : std::true_type {};
 };
 
 /*!
diff --git a/Machines/Apple/AppleIIgs/AppleIIgs.cpp b/Machines/Apple/AppleIIgs/AppleIIgs.cpp
index 0dd5ed223..e27842b63 100644
--- a/Machines/Apple/AppleIIgs/AppleIIgs.cpp
+++ b/Machines/Apple/AppleIIgs/AppleIIgs.cpp
@@ -688,14 +688,13 @@ class ConcreteMachine:
 				update_interrupts();
 			}
 
+			const bool will_flush_video = video_.will_flush(duration);
 			video_ += duration;
 			iwm_ += duration;
 			cycles_since_audio_update_ += duration;
 			total += decltype(total)(duration.as_integral());
 
-			// Ensure no more than a single line is enqueued for just-in-time video purposes.
-			// TODO: as implemented, check_flush_threshold doesn't actually work. Can it be made to without forcing cost to non-users, or is it a bad idea?
-			if(video_.check_flush_threshold<131>()) {
+			if(will_flush_video) {
 				update_interrupts();
 			}
 
diff --git a/Machines/Atari/ST/AtariST.cpp b/Machines/Atari/ST/AtariST.cpp
index 6ddf2dfce..6e55a6d4b 100644
--- a/Machines/Atari/ST/AtariST.cpp
+++ b/Machines/Atari/ST/AtariST.cpp
@@ -465,16 +465,14 @@ class ConcreteMachine:
 			}
 
 			// Update the video output, checking whether a sequence point has been hit.
-			while(length >= cycles_until_video_event_) {
-				length -= cycles_until_video_event_;
-				video_ += cycles_until_video_event_;
-				cycles_until_video_event_ = video_->get_next_sequence_point();
-				assert(cycles_until_video_event_ > HalfCycles(0));
+			if(video_.will_flush(length)) {
+				length -= video_.cycles_until_implicit_flush();
+				video_ += video_.cycles_until_implicit_flush();
 
 				mfp_->set_timer_event_input(1, video_->display_enabled());
 				update_interrupt_input();
 			}
-			cycles_until_video_event_ -= length;
+
 			video_ += length;
 		}
 
@@ -486,7 +484,6 @@ class ConcreteMachine:
 		HalfCycles bus_phase_;
 
 		JustInTimeActor<Video> video_;
-		HalfCycles cycles_until_video_event_;
 
 		// The MFP runs at 819200/2673749ths of the CPU clock rate.
 		JustInTimeActor<Motorola::MFP68901::MFP68901, 819200, 2673749> mfp_;