Skip to content

Commit

Permalink
correct processing of signal array in pixel renderers
Browse files Browse the repository at this point in the history
all SetPixel() function now set VBLANK pixels to 'VideoBlack' via a call
to GetColor(), rather than setting the RGBA values directly. except the
video digest, which remains a special case

this complete work from the previous commit 6826399
  • Loading branch information
JetSetIlly committed Nov 23, 2024
1 parent 1ac6f33 commit b54439e
Show file tree
Hide file tree
Showing 11 changed files with 48 additions and 40 deletions.
7 changes: 4 additions & 3 deletions bots/chess/videochess.go
Original file line number Diff line number Diff line change
Expand Up @@ -131,9 +131,10 @@ func (obs *observer) SetPixels(sig []signal.SignalAttributes, last int) error {
var offset int

for i := range sig {
// handle VBLANK by setting pixels to black
if sig[i].VBlank {
col = color.RGBA{R: 0, G: 0, B: 0}
// handle VBLANK by setting pixels to black. we also manually handle
// NoSignal in the same way
if sig[i].VBlank || sig[i].Index == signal.NoSignal {
col = obs.frameInfo.Spec.GetColor(signal.VideoBlack)
} else {
col = obs.frameInfo.Spec.GetColor(sig[i].Color)
}
Expand Down
7 changes: 4 additions & 3 deletions bots/spacejockey/spacejockey.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,9 +100,10 @@ func (obs *observer) SetPixels(sig []signal.SignalAttributes, last int) error {
var offset int

for i := range sig {
// handle VBLANK by setting pixels to black
if sig[i].VBlank {
col = color.RGBA{R: 0, G: 0, B: 0}
// handle VBLANK by setting pixels to black. we also manually handle
// NoSignal in the same way
if sig[i].VBlank || sig[i].Index == signal.NoSignal {
col = obs.frameInfo.Spec.GetColor(signal.VideoBlack)
} else {
col = obs.frameInfo.Spec.GetColor(sig[i].Color)
}
Expand Down
7 changes: 4 additions & 3 deletions comparison/comparison.go
Original file line number Diff line number Diff line change
Expand Up @@ -353,9 +353,10 @@ func (cmp *Comparison) SetPixels(sig []signal.SignalAttributes, last int) error
var offset int

for i := range sig {
// handle VBLANK by setting pixels to black
if sig[i].VBlank {
col = color.RGBA{R: 0, G: 0, B: 0}
// handle VBLANK by setting pixels to black. we also manually handle
// NoSignal in the same way
if sig[i].VBlank || sig[i].Index == signal.NoSignal {
col = cmp.frameInfo.Spec.GetColor(signal.VideoBlack)
} else {
col = cmp.frameInfo.Spec.GetColor(sig[i].Color)
}
Expand Down
7 changes: 4 additions & 3 deletions comparison/driver.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,9 +109,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].VBlank {
col = color.RGBA{R: 0, G: 0, B: 0}
// handle VBLANK by setting pixels to black. we also manually handle
// NoSignal in the same way
if sig[i].VBlank || sig[i].Index == signal.NoSignal {
col = drv.frameInfo.Spec.GetColor(signal.VideoBlack)
} else {
col = drv.frameInfo.Spec.GetColor(sig[i].Color)
}
Expand Down
12 changes: 9 additions & 3 deletions digest/video.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package digest
import (
"crypto/sha1"
"fmt"
"image/color"

"github.com/jetsetilly/gopher2600/hardware/television"
"github.com/jetsetilly/gopher2600/hardware/television/signal"
Expand Down Expand Up @@ -108,13 +109,18 @@ func (dig *Video) SetPixels(sig []signal.SignalAttributes, _ int) error {
offset := len(dig.digest)

for _, s := range sig {
// ignore invalid signals
// ignore invalid signals. this has a consequence that shrunken screen
// sizes will have pixel values left over from previous frames. but for
// the purposes of the digest this is okay
if s.Index == signal.NoSignal {
continue
}

// ignore VBLANK and extract the color signal in all situations
col := dig.spec.GetColor(s.Color)
var col color.RGBA

// signal processing is unlike other SetPixel() implementations in that
// we set the underlying color even if VBLANK is set
col = dig.spec.GetColor(s.Color)

// setting every pixel regardless of vblank value
p := dig.pixels[offset : offset+3 : offset+3]
Expand Down
7 changes: 4 additions & 3 deletions gui/sdlimgui/screen.go
Original file line number Diff line number Diff line change
Expand Up @@ -488,9 +488,10 @@ func (scr *screen) SetPixels(sig []signal.SignalAttributes, last int) error {
return nil
}

// handle VBLANK by setting pixels to black
if sig[i].VBlank {
col = color.RGBA{R: 0, G: 0, B: 0}
// handle VBLANK by setting pixels to black. we also manually handle
// NoSignal in the same way
if sig[i].VBlank || sig[i].Index == signal.NoSignal {
col = scr.crit.frameInfo.Spec.GetColor(signal.VideoBlack)
} else {
col = scr.crit.frameInfo.Spec.GetColor(sig[i].Color)
}
Expand Down
14 changes: 3 additions & 11 deletions hardware/television/protocol.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,17 +61,9 @@ type PixelRenderer interface {
// Every signal from SetPixels() therefore corresponds to a pixel in the
// bitmap - the first entry always referes to the top-left pixel
//
// Setting the color of a pixel can be done by extracting the ColorSignal
// from the SignalAttributes (see signal package)
//
// The last parameter indicates the index of the array entry that was most
// recently set. This is useful to know when showing televison images when
// the emulation is paused. All entries upto and including last are from
// teh *current* frame. All entries afterwards are from the *previous*
// frame
//
// If the entry contains signal.NoSignal then that screen pixel has not
// been written to recently
// If the entry contains signal.NoSignal then that screen pixel has not been
// written to recently. However, the bitmap may still need to be updated
// with "nil" information if the size of the screen has reduced
//
// For renderers that are producing an accurate visual image, the pixel
// should always be set to video black if VBLANK is on. Some renderers
Expand Down
5 changes: 4 additions & 1 deletion hardware/television/signal/signal.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,10 @@ const VideoBlack ColorSignal = 0xff
// Index value to indicate that the signal is invalid
const NoSignal = -1

// SignalAttributes represents the data sent to the television.
// SignalAttributes represents the data sent to the television
//
// When reset the Index field should be set to NoSignal and the Color field
// should be set to VideoBlack
type SignalAttributes struct {
Index int
VSync bool
Expand Down
1 change: 1 addition & 0 deletions hardware/television/television.go
Original file line number Diff line number Diff line change
Expand Up @@ -844,6 +844,7 @@ func (tv *Television) newFrame() error {
// the entire entry
tv.signals[i] = signal.SignalAttributes{
Index: signal.NoSignal,
Color: signal.VideoBlack,
}
}

Expand Down
10 changes: 5 additions & 5 deletions thumbnailer/anim.go
Original file line number Diff line number Diff line change
Expand Up @@ -317,8 +317,6 @@ func (thmb *Anim) NewScanline(scanline int) error {

// SetPixels implements the television.PixelRenderer interface
func (thmb *Anim) SetPixels(sig []signal.SignalAttributes, last int) error {
var col color.RGBA

// this adhoc "monitor" system looks for changes in pixels and uses that
// information to insert input into the emulation
var monitorChanges bool
Expand All @@ -328,10 +326,12 @@ func (thmb *Anim) SetPixels(sig []signal.SignalAttributes, last int) error {

var offset int
for i := range sig {
var col color.RGBA

// handle VBLANK by setting pixels to black
if sig[i].VBlank {
col = color.RGBA{R: 0, G: 0, B: 0}
// handle VBLANK by setting pixels to black. we also manually handle
// NoSignal in the same way
if sig[i].VBlank || sig[i].Index == signal.NoSignal {
col = thmb.frameInfo.Spec.GetColor(signal.VideoBlack)
} else {
col = thmb.frameInfo.Spec.GetColor(sig[i].Color)
}
Expand Down
11 changes: 6 additions & 5 deletions thumbnailer/image.go
Original file line number Diff line number Diff line change
Expand Up @@ -206,13 +206,14 @@ func (thmb *Image) NewScanline(scanline int) error {

// SetPixels implements the television.PixelRenderer interface
func (thmb *Image) SetPixels(sig []signal.SignalAttributes, last int) error {
var col color.RGBA
var offset int

for i := range sig {
// handle VBLANK by setting pixels to black
if sig[i].VBlank {
col = color.RGBA{R: 0, G: 0, B: 0}
var col color.RGBA

// handle VBLANK by setting pixels to black. we also manually handle
// NoSignal in the same way
if sig[i].VBlank || sig[i].Index == signal.NoSignal {
col = thmb.frameInfo.Spec.GetColor(signal.VideoBlack)
} else {
col = thmb.frameInfo.Spec.GetColor(sig[i].Color)
}
Expand Down

0 comments on commit b54439e

Please sign in to comment.