reverted SignalAttributes to a struct

profiling shows that the performance penalty is no longer that great. the struct is preferable because the code is clearer
JetSetIlly · Nov 23, 2024 · e4dea5c · e4dea5c
1 parent 00497b6
commit e4dea5c
Show file tree

Hide file tree

Showing 22 changed files with 109 additions and 164 deletions.
diff --git a/bots/chess/videochess.go b/bots/chess/videochess.go
@@ -88,7 +88,7 @@ const audioSilence = 15
 
 func (o *observer) SetAudio(sig []signal.SignalAttributes) error {
 	for _, s := range sig {
-		if uint8((s&signal.AudioChannel0)>>signal.AudioChannel0Shift) != audioSilence {
+		if s.AudioChannel0 != audioSilence {
 			select {
 			case o.audioFeedback <- true:
 			default:
@@ -132,11 +132,10 @@ func (obs *observer) SetPixels(sig []signal.SignalAttributes, last int) error {
 
 	for i := range sig {
 		// handle VBLANK by setting pixels to black
-		if sig[i]&signal.VBlank == signal.VBlank {
+		if sig[i].VBlank {
 			col = color.RGBA{R: 0, G: 0, B: 0}
 		} else {
-			px := signal.ColorSignal((sig[i] & signal.Color) >> signal.ColorShift)
-			col = obs.frameInfo.Spec.GetColor(px)
+			col = obs.frameInfo.Spec.GetColor(sig[i].Color)
 		}
 
 		// small cap improves performance, see https://golang.org/issue/27857

diff --git a/bots/spacejockey/spacejockey.go b/bots/spacejockey/spacejockey.go
@@ -57,7 +57,7 @@ const audioSilence = 15
 
 func (o *observer) SetAudio(sig []signal.SignalAttributes) error {
 	for _, s := range sig {
-		if uint8((s&signal.AudioChannel0)>>signal.AudioChannel0Shift) != audioSilence {
+		if s.AudioChannel0 != audioSilence {
 			select {
 			case o.audioFeedback <- true:
 			default:
@@ -101,11 +101,10 @@ func (obs *observer) SetPixels(sig []signal.SignalAttributes, last int) error {
 
 	for i := range sig {
 		// handle VBLANK by setting pixels to black
-		if sig[i]&signal.VBlank == signal.VBlank {
+		if sig[i].VBlank {
 			col = color.RGBA{R: 0, G: 0, B: 0}
 		} else {
-			px := signal.ColorSignal((sig[i] & signal.Color) >> signal.ColorShift)
-			col = obs.frameInfo.Spec.GetColor(px)
+			col = obs.frameInfo.Spec.GetColor(sig[i].Color)
 		}
 
 		// small cap improves performance, see https://golang.org/issue/27857

diff --git a/comparison/comparison.go b/comparison/comparison.go
@@ -354,11 +354,10 @@ func (cmp *Comparison) SetPixels(sig []signal.SignalAttributes, last int) error
 
 	for i := range sig {
 		// handle VBLANK by setting pixels to black
-		if sig[i]&signal.VBlank == signal.VBlank {
+		if sig[i].VBlank {
 			col = color.RGBA{R: 0, G: 0, B: 0}
 		} else {
-			px := signal.ColorSignal((sig[i] & signal.Color) >> signal.ColorShift)
-			col = cmp.frameInfo.Spec.GetColor(px)
+			col = cmp.frameInfo.Spec.GetColor(sig[i].Color)
 		}
 
 		// small cap improves performance, see https://golang.org/issue/27857
@@ -407,9 +406,7 @@ func (cmp *Comparison) EndRendering() error {
 // SetAudio implements the television.AudioMixer interface.
 func (cmp *Comparison) SetAudio(sig []signal.SignalAttributes) error {
 	for _, s := range sig {
-		v0 := uint8((s & signal.AudioChannel0) >> signal.AudioChannel0Shift)
-		v1 := uint8((s & signal.AudioChannel1) >> signal.AudioChannel1Shift)
-		cmp.audio = append(cmp.audio, v0, v1)
+		cmp.audio = append(cmp.audio, s.AudioChannel0, s.AudioChannel1)
 	}
 	return nil
 }

diff --git a/comparison/driver.go b/comparison/driver.go
@@ -110,11 +110,10 @@ func (drv *driver) SetPixels(sig []signal.SignalAttributes, last int) error {
 
 	for i := range sig {
 		// handle VBLANK by setting pixels to black
-		if sig[i]&signal.VBlank == signal.VBlank {
+		if sig[i].VBlank {
 			col = color.RGBA{R: 0, G: 0, B: 0}
 		} else {
-			px := signal.ColorSignal((sig[i] & signal.Color) >> signal.ColorShift)
-			col = drv.frameInfo.Spec.GetColor(px)
+			col = drv.frameInfo.Spec.GetColor(sig[i].Color)
 		}
 
 		// small cap improves performance, see https://golang.org/issue/27857
@@ -156,9 +155,7 @@ func (drv *driver) SetAudio(sig []signal.SignalAttributes) error {
 		idx = 1
 	}
 	for _, s := range sig {
-		v0 := uint8((s & signal.AudioChannel0) >> signal.AudioChannel0Shift)
-		v1 := uint8((s & signal.AudioChannel1) >> signal.AudioChannel1Shift)
-		drv.audio[idx] = append(drv.audio[idx], v0, v1)
+		drv.audio[idx] = append(drv.audio[idx], s.AudioChannel0, s.AudioChannel1)
 	}
 	return nil
 }

diff --git a/coprocessor/developer/profiling.go b/coprocessor/developer/profiling.go
@@ -20,7 +20,6 @@ import (
 
 	"github.com/jetsetilly/gopher2600/coprocessor"
 	"github.com/jetsetilly/gopher2600/coprocessor/developer/profiling"
-	"github.com/jetsetilly/gopher2600/hardware/television/signal"
 )
 
 // Profiling implements the coprocessor.CartCoProcDeveloper interface.
@@ -186,13 +185,13 @@ func (dev *Developer) ProcessProfiling() {
 
 	if frameInfo.Stable {
 		if coords.Scanline == frameInfo.VisibleTop {
-			if dev.tv.GetLastSignal()&signal.VBlank == signal.VBlank {
+			if dev.tv.GetLastSignal().VBlank {
 				accumulate(profiling.FocusVBLANK)
 			} else {
 				accumulate(profiling.FocusScreen)
 			}
 		} else if coords.Scanline == frameInfo.VisibleBottom {
-			if dev.tv.GetLastSignal()&signal.VBlank == signal.VBlank {
+			if dev.tv.GetLastSignal().VBlank {
 				accumulate(profiling.FocusOverscan)
 			} else {
 				accumulate(profiling.FocusScreen)

diff --git a/digest/audio.go b/digest/audio.go
@@ -76,16 +76,15 @@ func (dig *Audio) ResetDigest() {
 func (dig *Audio) SetAudio(sig []signal.SignalAttributes) error {
 	for _, s := range sig {
 		// ignore invalid signals
-		if s == signal.NoSignal {
+		if s.Index == signal.NoSignal {
 			continue
 		}
 
 		// ignore signals with no audio update
-		if s&signal.AudioUpdate != signal.AudioUpdate {
+		if !s.AudioUpdate {
 			continue
 		}
-
-		dig.buffer[dig.bufferCt] = uint8((s & signal.AudioChannel0) >> signal.AudioChannel0Shift)
+		dig.buffer[dig.bufferCt] = s.AudioChannel0
 
 		dig.bufferCt++
 		if dig.bufferCt >= audioBufferLength {

diff --git a/digest/video.go b/digest/video.go
@@ -109,13 +109,12 @@ func (dig *Video) SetPixels(sig []signal.SignalAttributes, _ int) error {
 
 	for _, s := range sig {
 		// ignore invalid signals
-		if s == signal.NoSignal {
+		if s.Index == signal.NoSignal {
 			continue
 		}
 
 		// ignore VBLANK and extract the color signal in all situations
-		px := signal.ColorSignal((s & signal.Color) >> signal.ColorShift)
-		col := dig.spec.GetColor(px)
+		col := dig.spec.GetColor(s.Color)
 
 		// setting every pixel regardless of vblank value
 		p := dig.pixels[offset : offset+3 : offset+3]

diff --git a/gui/sdlaudio/audio.go b/gui/sdlaudio/audio.go
@@ -123,12 +123,12 @@ func (aud *Audio) MoreAudio() bool {
 // SetAudio implements the protocol.AudioMixer interface.
 func (aud *Audio) SetAudio(sig []signal.SignalAttributes) error {
 	for _, s := range sig {
-		if s&signal.AudioUpdate != signal.AudioUpdate {
+		if !s.AudioUpdate {
 			continue
 		}
 
-		v0 := uint8((s & signal.AudioChannel0) >> signal.AudioChannel0Shift)
-		v1 := uint8((s & signal.AudioChannel1) >> signal.AudioChannel1Shift)
+		v0 := s.AudioChannel0
+		v1 := s.AudioChannel1
 
 		aud.stereoCh0Buffer = aud.stereoCh0Buffer[1:]
 		aud.stereoCh0Buffer = append(aud.stereoCh0Buffer, v0)

diff --git a/gui/sdlimgui/screen.go b/gui/sdlimgui/screen.go
@@ -489,11 +489,10 @@ func (scr *screen) SetPixels(sig []signal.SignalAttributes, last int) error {
 		}
 
 		// handle VBLANK by setting pixels to black
-		if sig[i]&signal.VBlank == signal.VBlank {
+		if sig[i].VBlank {
 			col = color.RGBA{R: 0, G: 0, B: 0}
 		} else {
-			px := signal.ColorSignal((sig[i] & signal.Color) >> signal.ColorShift)
-			col = scr.crit.frameInfo.Spec.GetColor(px)
+			col = scr.crit.frameInfo.Spec.GetColor(sig[i].Color)
 		}
 
 		// small cap improves performance, see https://golang.org/issue/27857
@@ -571,10 +570,10 @@ func (scr *screen) plotOverlay() {
 func (scr *screen) reflectionColor(ref *reflection.ReflectedVideoStep) color.RGBA {
 	switch scr.crit.overlay {
 	case reflection.OverlayLabels[reflection.OverlayVBLANK_VSYNC]:
-		if ref.Signal&signal.VBlank == signal.VBlank {
+		if ref.Signal.VBlank {
 			return reflectionColors[reflection.VBLANK]
 		}
-		if ref.Signal&signal.VSync == signal.VSync {
+		if ref.Signal.VSync {
 			return reflectionColors[reflection.VSYNC]
 		}
 	case reflection.OverlayLabels[reflection.OverlayWSYNC]:

diff --git a/gui/sdlimgui/win_dbgscr.go b/gui/sdlimgui/win_dbgscr.go
@@ -635,8 +635,8 @@ func (win *winDbgScr) drawReflectionTooltip() {
 
 		// pixel swatch. using black swatch if pixel is HBLANKed or VBLANKed
 		_, _, pal, _ := win.img.imguiTVPalette()
-		px := signal.ColorSignal((ref.Signal & signal.Color) >> signal.ColorShift)
-		if (ref.IsHblank || ref.Signal&signal.VBlank == signal.VBlank || px == signal.VideoBlack) && !win.elements {
+		px := ref.Signal.Color
+		if (ref.IsHblank || ref.Signal.VBlank || px == signal.VideoBlack) && !win.elements {
 			imguiColorLabelSimple("No color signal", pal[0])
 		} else {
 			// not using GetColor() function. arguably we should but we've

diff --git a/gui/sdlimgui/win_dbgscr_paint.go b/gui/sdlimgui/win_dbgscr_paint.go
@@ -38,7 +38,7 @@ func (win *winDbgScr) paintDragAndDrop() {
 
 				ff := floodFill{
 					win:  win,
-					from: uint8((ref.Signal&signal.Color)>>signal.ColorShift) & 0xfe,
+					from: uint8(ref.Signal.Color) & 0xfe,
 					to:   payload[0] & 0xfe,
 				}
 
@@ -79,18 +79,17 @@ type floodFill struct {
 
 func (ff *floodFill) build(target floodFillTarget) {
 	ref := ff.startingReflection[target.offset]
-	if ref.Signal&signal.VBlank == signal.VBlank {
+	if ref.Signal.VBlank {
 		return
 	}
 
-	px := uint8((ref.Signal&signal.Color)>>signal.ColorShift) & 0xfe
+	px := uint8(ref.Signal.Color) & 0xfe
 	if px != ff.from {
 		return
 	}
 
 	ff.targets = append(ff.targets, target)
-	ref.Signal &= ^signal.Color
-	ref.Signal |= signal.SignalAttributes(ff.to) << signal.ColorShift
+	ref.Signal.Color = signal.ColorSignal(ff.to)
 	ff.startingReflection[target.offset] = ref
 
 	// down
@@ -134,7 +133,7 @@ func (ff *floodFill) resolve(offsetIdx int) {
 	ff.win.img.dbg.PushFunctionImmediate(func() {
 		target := ff.targets[offsetIdx]
 		ref := ff.win.scr.crit.reflection[target.offset]
-		px := uint8((ref.Signal&signal.Color)>>signal.ColorShift) & 0xfe
+		px := uint8(ref.Signal.Color) & 0xfe
 		if px != ff.from {
 			ff.resolve(offsetIdx + 1)
 			return

diff --git a/gui/sdlimgui/win_oscilloscope.go b/gui/sdlimgui/win_oscilloscope.go
@@ -145,12 +145,12 @@ func (win *winOscilloscope) drawTVLabel(label string) {
 // SetAudio implements protocol.AudioMixer.
 func (win *winOscilloscope) SetAudio(sig []signal.SignalAttributes) error {
 	for _, s := range sig {
-		if s&signal.AudioUpdate != signal.AudioUpdate {
+		if !s.AudioUpdate {
 			continue
 		}
 
-		v0 := uint8((s & signal.AudioChannel0) >> signal.AudioChannel0Shift)
-		v1 := uint8((s & signal.AudioChannel1) >> signal.AudioChannel1Shift)
+		v0 := s.AudioChannel0
+		v1 := s.AudioChannel1
 		m := mix.Mono(v0, v1)
 
 		s0, s1 := mix.Stereo(v0, v1)

diff --git a/hardware/television/resizer.go b/hardware/television/resizer.go
@@ -156,10 +156,10 @@ func (rz *Resizer) examine(state *State, sig signal.SignalAttributes) {
 	// some ROMs never set VBLANK but we still want to do our best and frame the
 	// screen nicely. this flag controls whether we use vblank top/bottom values
 	// or "black" top/bottom values
-	rz.usingVBLANK = rz.usingVBLANK || sig&signal.VBlank == signal.VBlank
+	rz.usingVBLANK = rz.usingVBLANK || sig.VBlank
 
 	// if VBLANK is off then update the top/bottom values note
-	if sig&signal.VBlank != signal.VBlank {
+	if !sig.VBlank {
 		if state.frameInfo.Stable {
 			if state.scanline < rz.vblankTop &&
 				state.scanline >= state.frameInfo.Spec.ExtendedVisibleTop {
@@ -197,9 +197,8 @@ func (rz *Resizer) examine(state *State, sig signal.SignalAttributes) {
 	// some ROMs never set VBLANK. for these cases we also record the extent of
 	// non-black pixels. these values are using in the commit() function in the
 	// event that usingVBLANK is false.
-	if state.clock > specification.ClksHBlank && sig&signal.VBlank != signal.VBlank {
-		px := signal.ColorSignal((sig & signal.Color) >> signal.ColorShift)
-		col := state.frameInfo.Spec.GetColor(px)
+	if state.clock > specification.ClksHBlank && !sig.VBlank {
+		col := state.frameInfo.Spec.GetColor(sig.Color)
 		if col.R != 0x00 || col.G != 0x00 || col.B != 0x00 {
 			if state.frameInfo.Stable {
 				if state.scanline < rz.blackTop &&

diff --git a/hardware/television/signal/signal.go b/hardware/television/signal/signal.go
@@ -27,51 +27,34 @@ type ColorSignal uint8
 // VideoBlack is the ColorSignal value that indicates no pixel is being output.
 const VideoBlack ColorSignal = 0xff
 
-// SignalAttributes represents the data sent to the television.
-type SignalAttributes uint64
-
-// List of bit masks to be used to with the SignalAttribute type.
-const (
-	VSync         SignalAttributes = 0b0000000000000000000000000000000000000000000001
-	VBlank        SignalAttributes = 0b0000000000000000000000000000000000000000000010
-	CBurst        SignalAttributes = 0b0000000000000000000000000000000000000000000100
-	HSync         SignalAttributes = 0b0000000000000000000000000000000000000000001000
-	AudioUpdate   SignalAttributes = 0b0000000000000000000000000000000000000000010000
-	AudioChannel0 SignalAttributes = 0b0000000000000000000000000000000001111111100000 // 8 bits
-	AudioChannel1 SignalAttributes = 0b0000000000000000000000000111111110000000000000 // 8 bits
-	Color         SignalAttributes = 0b0000000000000000011111111000000000000000000000 // 8 bits
-
-	// Index is additional information relating to the position of the signal
-	// in relation to the top left of the screen. It can mostly be ignored but
-	// it is useful for synchronisation between the television and reflection
-	// packages.
-	Index SignalAttributes = 0b1111111111111111100000000000000000000000000000 // 17 bits
-)
+// Index value to indicate that the signal is invalid
+const NoSignal = -1
 
-// List of shift amounts to be used to access the corresponding bits in a
-// SignalAttributes value.
-const (
-	AudioChannel0Shift = 5
-	AudioChannel1Shift = 13
-	ColorShift         = 21
-	IndexShift         = 29
-)
-
-// NoSignal is the null value of the SignalAttributes type.
-const NoSignal = Index
+// SignalAttributes represents the data sent to the television.
+type SignalAttributes struct {
+	Index         int
+	VSync         bool
+	VBlank        bool
+	CBurst        bool
+	HSync         bool
+	AudioUpdate   bool
+	AudioChannel0 uint8
+	AudioChannel1 uint8
+	Color         ColorSignal
+}
 
 func (a SignalAttributes) String() string {
 	s := strings.Builder{}
-	if a&VSync == VSync {
+	if a.VSync {
 		s.WriteString("VSYNC ")
 	}
-	if a&VBlank == VBlank {
+	if a.VBlank {
 		s.WriteString("VBLANK ")
 	}
-	if a&CBurst == CBurst {
+	if a.CBurst {
 		s.WriteString("CBURST ")
 	}
-	if a&HSync == HSync {
+	if a.HSync {
 		s.WriteString("HSYNC ")
 	}
 	return s.String()

diff --git a/hardware/television/simple.go b/hardware/television/simple.go
@@ -99,8 +99,8 @@ func (tv *Television) signalSimple(sig signal.SignalAttributes) {
 	}
 
 	// check for change of VSYNC signal
-	if sig&signal.VSync != tv.state.lastSignal&signal.VSync {
-		if sig&signal.VSync == signal.VSync {
+	if sig.VSync != tv.state.lastSignal.VSync {
+		if sig.VSync {
 			// VSYNC has started
 			tv.simple.vsyncActive = true
 			tv.simple.vsyncScanlines = 0
@@ -134,7 +134,7 @@ func (tv *Television) signalSimple(sig signal.SignalAttributes) {
 	// is to enable the RSYNC smooth scrolling trick to be displayed correctly.
 	//
 	// https://atariage.com/forums/topic/224946-smooth-scrolling-playfield-i-think-ive-done-it
-	if sig&signal.HSync == signal.HSync && tv.state.lastSignal&signal.HSync != signal.HSync {
+	if sig.HSync && !tv.state.lastSignal.HSync {
 		if tv.state.clock < 13 || tv.state.clock > 22 {
 			tv.state.clock = 16
 		}
@@ -157,8 +157,7 @@ func (tv *Television) signalSimple(sig signal.SignalAttributes) {
 	}
 
 	// augment television signal before storing and sending to pixel renderers
-	sig &= ^signal.Index
-	sig |= signal.SignalAttributes(tv.currentSignalIdx << signal.IndexShift)
+	sig.Index = tv.currentSignalIdx
 
 	// write the signal into the correct index of the signals array.
 	tv.signals[tv.currentSignalIdx] = sig
@@ -239,7 +238,7 @@ func (tv *Television) newFrameSimple(fromVsync bool) error {
 
 	// nullify unused signals at end of frame
 	for i := tv.currentSignalIdx; i < len(tv.signals); i++ {
-		tv.signals[i] = signal.NoSignal
+		tv.signals[i].Index = signal.NoSignal
 	}
 
 	// set pending pixels