improved VSYNC counting

number of scanlines of VSYNC is now part of the FramInfo type. it is
used in the screen implementation to help decide whether to
desynchronise the screen

added sync sensitivity preference and additional control in the
preferences window

coprocessor/developer/developer.go

@@ -345,7 +345,7 @@ func (dev *Developer) newFrame_source(frameInfo television.FrameInfo) {
 	// only update FrameCycles if new frame was caused by a VSYNC or we've
 	// waited long enough since the last update
 	dev.framesSinceLastUpdate++
-	if !frameInfo.VSynced || dev.framesSinceLastUpdate > maxWaitUpdateTime {
+	if !frameInfo.VSync || dev.framesSinceLastUpdate > maxWaitUpdateTime {
 		return
 	}
 	dev.framesSinceLastUpdate = 0

gui/crt/preferences.go

@@ -54,8 +54,9 @@ type Preferences struct {
 
 	PixelPerfectFade prefs.Float
 
-	SyncSpeed   prefs.Int
-	SyncPowerOn prefs.Bool
+	SyncPowerOn     prefs.Bool
+	SyncSpeed       prefs.Int
+	SyncSensitivity prefs.Int
 
 	IntegerScaling prefs.Bool
 }
@@ -94,6 +95,7 @@ const (
 	pixelPerfectFade     = 0.4
 	syncSpeed            = 2
 	syncPowerOn          = true
+	syncSensitivity      = 2
 	integerScaling       = false
 )
 
@@ -220,11 +222,15 @@ func NewPreferences() (*Preferences, error) {
 	if err != nil {
 		return nil, err
 	}
+	err = p.dsk.Add("crt.syncPowerOn", &p.SyncPowerOn)
+	if err != nil {
+		return nil, err
+	}
 	err = p.dsk.Add("crt.syncSpeed", &p.SyncSpeed)
 	if err != nil {
 		return nil, err
 	}
-	err = p.dsk.Add("crt.syncPowerOn", &p.SyncPowerOn)
+	err = p.dsk.Add("crt.syncSensitivity", &p.SyncSensitivity)
 	if err != nil {
 		return nil, err
 	}
@@ -270,8 +276,9 @@ func (p *Preferences) SetDefaults() {
 	p.Sharpness.Set(sharpness)
 	p.BlackLevel.Set(blackLevel)
 	p.PixelPerfectFade.Set(pixelPerfectFade)
-	p.SyncSpeed.Set(syncSpeed)
 	p.SyncPowerOn.Set(syncPowerOn)
+	p.SyncSpeed.Set(syncSpeed)
+	p.SyncSensitivity.Set(syncSensitivity)
 	p.IntegerScaling.Set(integerScaling)
 }
 

gui/sdlimgui/playscr.go

@@ -188,7 +188,7 @@ func (win *playScr) drawFPS() bool {
 	imgui.Text(win.img.screen.crit.frameInfo.Spec.ID)
 	imgui.SameLine()
 	imgui.Text(win.hz)
-	if !win.scr.crit.frameInfo.VSynced {
+	if !win.scr.crit.frameInfo.VSync {
 		imgui.SameLine()
 		imgui.Text(string(fonts.NoVSYNC))
 	}

gui/sdlimgui/screen.go

@@ -334,7 +334,7 @@ func (scr *screen) NewFrame(frameInfo television.FrameInfo) error {
 		if scr.img.crtPrefs.Enabled.Get().(bool) {
 			syncSpeed := scr.img.crtPrefs.SyncSpeed.Get().(int)
 
-			if !frameInfo.VSynced {
+			if !frameInfo.VSync || frameInfo.VSyncScanlines < scr.img.crtPrefs.SyncSensitivity.Get().(int) {
 				scr.crit.vsyncCount = 0
 			} else if scr.crit.vsyncCount <= syncSpeed {
 				scr.crit.vsyncCount++

gui/sdlimgui/win_dbgscr.go

@@ -384,7 +384,7 @@ func (win *winDbgScr) drawCoordsLine() {
 	imgui.Text(win.img.lz.TV.LastSignal.String())
 
 	// unsynced indicator
-	if !win.scr.crit.frameInfo.VSynced {
+	if !win.scr.crit.frameInfo.VSync {
 		imgui.SameLineV(0, 20)
 		imgui.Text("UNSYNCED")
 	}

gui/sdlimgui/win_prefs_crt.go

@@ -86,11 +86,14 @@ func (win *winPrefs) drawCRT() {
 	imgui.Separator()
 	imgui.Spacing()
 
-	imgui.PushItemWidth(-1)
-	win.drawSyncSpeed()
-	imgui.Spacing()
-	win.drawSyncPowerOn()
-	imgui.PopItemWidth()
+	if imgui.CollapsingHeader("VSYNC") {
+		imgui.Spacing()
+		win.drawSyncSpeed()
+		imgui.SameLineV(0, 15)
+		win.drawSyncSensitivity()
+		imgui.Spacing()
+		win.drawSyncPowerOn()
+	}
 }
 
 func (win *winPrefs) drawCurve() {
@@ -459,7 +462,9 @@ available when 'Pixel Perfect' mode is disabled.`)
 }
 
 func (win *winPrefs) drawSyncSpeed() {
-	imgui.Text("Synchronisation Speed")
+	imgui.AlignTextToFramePadding()
+	imgui.Text("Speed")
+	imgui.SameLine()
 
 	t := int32(win.img.crtPrefs.SyncSpeed.Get().(int))
 	var label string
@@ -472,11 +477,42 @@ func (win *winPrefs) drawSyncSpeed() {
 	if imgui.SliderIntV("##syncSpeed", &t, 0, 10, label, 1.0) {
 		win.img.crtPrefs.SyncSpeed.Set(t)
 	}
+	imguiTooltipSimple(`The number of consecutive frames with
+a valid VSYNC signal for the screen to be
+considered stable.
+
+When the screen is not stable, the screen
+will visibly 'roll'.`)
+}
+
+func (win *winPrefs) drawSyncSensitivity() {
+	imgui.AlignTextToFramePadding()
+	imgui.Text("Sensitivity")
+	imgui.SameLine()
+
+	t := int32(win.img.crtPrefs.SyncSensitivity.Get().(int))
+	var label string
+	if t == 1 {
+		label = fmt.Sprintf("%d scanline", t)
+	} else {
+		label = fmt.Sprintf("%d scanlines", t)
+	}
+
+	if imgui.SliderIntV("##syncSensitivity", &t, 0, 4, label, 1.0) {
+		win.img.crtPrefs.SyncSensitivity.Set(t)
+	}
+
+	imguiTooltipSimple(`The number of complete scanlines of VSYNC
+for the VSYNC signal to be considered valid.
+
+Atari recommended a value 3 to ensure maximum
+compatibility.`)
 }
 
 func (win *winPrefs) drawSyncPowerOn() {
 	b := win.img.crtPrefs.SyncPowerOn.Get().(bool)
-	if imgui.Checkbox("Syncronise On Power##poweron", &b) {
+	if imgui.Checkbox("Syncronise On Powern##poweron", &b) {
 		win.img.crtPrefs.SyncPowerOn.Set(b)
 	}
+	imguiTooltipSimple(`Whether the emulated TV visibly synchronises when powered on.`)
 }

hardware/television/frameinfo.go

@@ -36,10 +36,14 @@ type FrameInfo struct {
 	// the refresh rate. calculated from the TotalScanlines value
 	RefreshRate float32
 
-	// a VSynced frame is one which was generated from a valid VSYNC/VBLANK
+	// a VSync frame is one which was generated from a valid VSYNC/VBLANK
 	// sequence and which hasn't cause the update frequency of the television
 	// to change.
-	VSynced bool
+	VSync bool
+
+	// the number of scanlines in the VSync. value is not meaningful if VSync
+	// is false
+	VSyncScanlines int
 
 	// Stable is true once the television frame has been consistent for N frames
 	// after reset. This is useful for pixel renderers so that they don't show
@@ -71,7 +75,7 @@ func (info *FrameInfo) reset() {
 	info.VisibleBottom = info.Spec.AtariSafeVisibleBottom
 	info.TotalScanlines = info.Spec.ScanlinesTotal
 	info.RefreshRate = info.Spec.RefreshRate
-	info.VSynced = false
+	info.VSync = false
 	info.Stable = false
 }
 

hardware/television/resizer.go

@@ -108,7 +108,7 @@ func (sr *resizer) examine(tv *Television, sig signal.SignalAttributes) {
 	// the best example of this is Andrew Davie's chess which simply does
 	// not care about frames during the computer's thinking time - we don't
 	// want to resize during these frames.
-	if !tv.state.frameInfo.VSynced {
+	if !tv.state.frameInfo.VSync {
 		// reset any pending changes on an unsynced frame
 		sr.pendingCt = 0
 		sr.pendingTop = sr.vblankTop

hardware/television/television.go

@@ -72,9 +72,11 @@ type State struct {
 	// record of signal attributes from the last call to Signal()
 	lastSignal signal.SignalAttributes
 
-	// vsyncCount records the number of consecutive clocks the VSYNC signal
-	// has been sustained. we use this to help correctly implement vsync.
-	vsyncCount int
+	// vsync control
+	vsyncActive       bool
+	vsyncStartOnClock int
+	vsyncScanlines    int
+	vsyncClocks       int
 
 	// frame resizer
 	resizer resizer
@@ -223,7 +225,10 @@ func (tv *Television) Reset(keepFrameNum bool) error {
 	tv.state.clock = 0
 	tv.state.scanline = 0
 	tv.state.stableFrames = 0
-	tv.state.vsyncCount = 0
+	tv.state.vsyncActive = false
+	tv.state.vsyncStartOnClock = 0
+	tv.state.vsyncScanlines = 0
+	tv.state.vsyncClocks = 0
 	tv.state.lastSignal = signal.NoSignal
 
 	for i := range tv.signals {
@@ -420,7 +425,18 @@ func (tv *Television) Signal(sig signal.SignalAttributes) error {
 		//
 		// (06/01/21) another example is the Artkaris NTSC version of Lili
 		if tv.state.scanline >= specification.AbsoluteMaxScanlines {
-			if tv.state.vsyncCount == 0 {
+			// (20/10/22) I'm no longer sure if this test for an active vsync
+			// is necessary. it might be better / more accurate if the test is
+			// removed and the screen allowed to flyback regardless of the
+			// VSYNC state
+			//
+			// however, without testing I'm no longer sure what the effect of
+			// that be. in particular how the results appear in the debugging
+			// screen and specically, how it affects the debugging screen's
+			// onion skinning
+			//
+			// I'll leave it in place for now until further testing can be done
+			if !tv.state.vsyncActive {
 				err := tv.newFrame(false)
 				if err != nil {
 					return err
@@ -437,19 +453,43 @@ func (tv *Television) Signal(sig signal.SignalAttributes) error {
 		}
 	}
 
-	// check vsync signal at the time of the flyback
-	if sig&signal.VSync == signal.VSync && tv.state.lastSignal&signal.VSync != signal.VSync {
-		tv.state.vsyncCount = 0
-	} else if sig&signal.VSync == signal.VSync && tv.state.lastSignal&signal.VSync == signal.VSync {
-		tv.state.vsyncCount++
-	} else if sig&signal.VSync != signal.VSync && tv.state.lastSignal&signal.VSync == signal.VSync {
-		if tv.state.vsyncCount > 10 {
-			err := tv.newFrame(true)
-			if err != nil {
-				return err
-			}
+	// count VSYNC clocks and scanlines
+	if tv.state.vsyncActive {
+		if tv.state.clock == tv.state.vsyncStartOnClock {
+			tv.state.vsyncScanlines++
 		}
-		tv.state.vsyncCount = 0
+		tv.state.vsyncClocks++
+	}
+
+	// check for change of VSYNC signal
+	if sig&signal.VSync != tv.state.lastSignal&signal.VSync {
+		if sig&signal.VSync == signal.VSync {
+			// VSYNC has started
+			tv.state.vsyncActive = true
+			tv.state.vsyncScanlines = 0
+			tv.state.vsyncStartOnClock = tv.state.clock
+		} else {
+			// VSYNC has ended but we don't want to trigger a new frame unless
+			// the VSYNC signal has been present for a minimum number of
+			// clocks
+			//
+			// there's no real empirical reason for the value used here except
+			// that it seems right in practice. it certainly doesn't seem to
+			// cause any harm.
+			//
+			// it's worth noting that without this minimum threshold the
+			// smoothscrolling demos (mentioned below) don't work as expected.
+			// so maybe there's a subtle interaction with RSYNC here that's
+			// worth exploring
+			if tv.state.vsyncClocks > 10 {
+				err := tv.newFrame(true)
+				if err != nil {
+					return err
+				}
+			}
+			tv.state.vsyncActive = false
+		}
+		tv.state.vsyncClocks = 0
 	}
 
 	// we've "faked" the flyback signal above when clock reached
@@ -569,7 +609,8 @@ func (tv *Television) newFrame(fromVsync bool) error {
 	tv.state.frameInfo.FrameNum = tv.state.frameNum
 
 	// note whether newFrame() was the result of a valid VSYNC or a "natural" flyback
-	tv.state.frameInfo.VSynced = fromVsync
+	tv.state.frameInfo.VSync = fromVsync
+	tv.state.frameInfo.VSyncScanlines = tv.state.vsyncScanlines
 
 	// commit any resizing that maybe pending
 	err := tv.state.resizer.commit(tv)