Verilog: Test Pattern: rgb = {b,g,r} needs more explanation

[ewenmcneill]

In the Verilog "Test Pattern" example, the rgb assignment assign rgb = {b,g,r}; seems somewhat counter intuitive. https://github.com/sehugg/8bitworkshop/blob/03af8c27144380d528f524828e73c966d12ecd35/presets/verilog-vga/test_hvsync.v#L30

The variable naming implies (at least to me) a "r, g, b" ordering (ie, "r" is most significant bit), but the actual implementation appears to be a "b, g, r" ordering, at least when read as a big endian value; ie it appears in the emulated video hardware that the "r" is the least significant bit, and the "b" is the most significant bit. Page 45 of the book ("Designing Video Game Hardware in Verilog") explains that syntax as making "b" the most significant bit. But the variable naming leads me to expect "r" to be the most significant bit...

This seems to violate the Principle of Least Surprise, and I think it'd be worth at least adding a comment on that particular example (the first one where the video hardware is introduced) that explains the (emulated) video hardware is Little Endian (the "r" in "rgb" is at the least significant bit end) and Verilog is Big Endian. Hence the opposite order.

AFAICT the colour mapping is coming from:

https://github.com/sehugg/8bitworkshop/blob/03af8c27144380d528f524828e73c966d12ecd35/src/platform/verilog.ts#L292-L309

via:

https://github.com/sehugg/8bitworkshop/blob/03af8c27144380d528f524828e73c966d12ecd35/src/platform/verilog.ts#L552

and the output of RGBLOOKUP seems like it's a RGBA value (with alpha always forced to 0xff) in little endian order (ie A(lpha) at most significant end, then B, G, R). (It looks like the input is a 4-bit value, presumably RGBI with an intensity bit, like early PC graphics hardware; but ordered I,B,G,R from most significant to least significant bit.)

I'm not sure where those RGBA values go after that, and thus whether that RGBA Little Endian implementation is a Javascript/HTML/Browser implementation artifact or an internal choice (eg, for compatibility with another emulated platform).

My guess is that this implementation detail could be hidden by the RGBLOOKUP table, but that doing so would require all the other video examples -- including ones in the printed book! -- to be rewritten for the new "hardware" colour order. Hence the suggestion to add a comment into the online examples to document further what is happening. (It's not documented in the book AFAICT, at least not in the relevant chapter -- chapter 10.)

Ewen

PS: To save anyone else finding this some time, the "video display" is turned on for the Verilog platform in the IDE when hsync, vsync, and rgb are present as output signals; otherwise you get the waveform view. So you can't, eg, rename the output signal to bgr to make it "more logical", as then the video display emulation disappears... :-)

https://github.com/sehugg/8bitworkshop/blob/03af8c27144380d528f524828e73c966d12ecd35/src/platform/verilog.ts#L670

[ewenmcneill]

The other option might be to show:

  localparam RED = 0;
  localparam GREEN = 1;
  localparam BLUE = 2;
[...]
  assign rgb[RED] = r;
  assign rgb[GREEN] = g;
  assign rgb[BLUE] = b;

somewhere as an equivalent of what is actually happening, and make the underlying video "hardware" bit/colour mapping more obvious.

That seems to work in my testing, and I find it less confusing than "rgb = bgr" :-)

Ewen

[ewenmcneill]

FTR, it looks like the underlying simulation implements: intensity, Blue, Green, Red, in that order, ie ibgr, and the intensity can be set from Verilog. Eg in the racing game CPU version (in the Verilog just before the assembly for the program):

https://github.com/sehugg/8bitworkshop/blob/03af8c27144380d528f524828e73c966d12ecd35/presets/verilog/racing_game_cpu.v#L149

has a 4 bit rgb value, and if the initial 1'b0 is set to 1'b1 then the background, etc, is grey rather than black (ie, "bright black").

So the examples with rgb = {b,g,r} are not only in little endian order, but also implicitly allowing the intensity to be set to 1'b0.

Note that whether the intensity value needs to be supplied by the Verilog depends on how that rgb output is defined on the Verilog side, eg in the racing game CPU version it is defined as a 4 bit quantity:

https://github.com/sehugg/8bitworkshop/blob/03af8c27144380d528f524828e73c966d12ecd35/presets/verilog/racing_game_cpu.v#L20

but in most of the other examples it's defined as a 3 bit quantity, eg,

https://github.com/sehugg/8bitworkshop/blob/03af8c27144380d528f524828e73c966d12ecd35/presets/verilog-vga/test_hvsync.v#L12

Ewen

[sehugg]

Thanks, these are good points. I'll add more comments to the examples.