Closed robin-raymond closed 11 months ago
From what I see in the unit tests (and I might have missed something), is that the unit tests compare a known output against the dither. As I had to fix the "commit" bug:
https://github.com/sehugg/dithertron/issues/29
I would expect failures if it's a image comparison match. Why they aren't all failing I'm not sure, but the old code would do a commit after the update, and now it does a commit before the update. The init() routines do not commit anymore (because that commit always happens already before update so it'd be a double commit). I did verify the update/getValid routines, and they appear correct. I'm not sure though how to prove they aren't busted. For example, if you compare C64 Hires, the new code produces a bit more splotchiness, but that's because the old code went through +1 more times of commit than the new code. Doing that extra +1 iteration makes the difference (at least from my tests), but I'm not sure the best way to keep doing that +1 iteration round, keep the bug fix, and not touch any of the default values.
What are your thoughts?
The unit tests just test for low-resolution similarity, so minor differences should not trigger a failure. I'll take a look, the tests may be messed up.
I don't know much about Intellivision, excited to take a look!
From what I see, the c64 tests are timing out after 30 sec (?) and the vic20 has regressed somewhat, there are spotchy blocks in vic20.multi.
Hmm. I'm not seeing that, which image(s)?
I was just comparing the Seurat default image with the previous build, saw some artifacts. I'll take a closer look during the holidays.
There is going to be some differences because I had to rearrange the commit() order with the update due to a bug I found: https://github.com/sehugg/dithertron/issues/29
But it's a big change so it's possible there's a bug somewhere in there too. I tried to validate everything though but who knows... If you see anything that seems off please let me know.
I'm comparing with the current build of Dithertron, which looks great: https://sehugg.github.io/dithertron/#sys=vic20.multi&image=seurat.jpg
If you look @ the same image in the PR branch, there are a few solid blocks of color, but none in the previous build. This is probably what the unit tests (npm run test-unit) are seeing. They compare the dithered image to the original image, and expect the differences to be under a certain threshold.
So there were two issues: 1) the commit vs update order does impact the result 2) the default color pre-fill on the index was set to the background color (and most prominent color) instead of the top pick of the available foreground color.
The Intellivision has two interesting modes. The foreground and background mode allows the choice of two colors for an 8x8 area but the downside is that the characters are restricted to gram, of which there are only 64 of 8x8 pixels. This makes a rather small image.
The modes use a color stack, which allows for up to 256 characters (but only 240 are needed to fill the screen) and requires GROM image replacement by overriding the ROM bus. The foreground color is restricted but registers are used to chose the current background color in a rotating wheel of colors. The choice of the color is not selectable, as each character has the choice of picking the current color in the stack, or choosing the next color in the stack. Thus the value of the color stack is diminished.
The color stack code figures out the most useful colors to use for the color stack (and the most likely to be chosen), and then figures out the best order combination to use for the color stack. An interesting extension might be to force only a single color to be used, like a global background color (TBD if that's helpful).
Changes to code:
Related issues: https://github.com/sehugg/dithertron/issues/8 https://github.com/sehugg/dithertron/issues/29