Closed robin-raymond closed 1 year ago
FYI - I do not expect that you will want to accept the pull request "as is". I'm not a typescript developer by day so you may want to correct idiomatic patterns where they can be done easier/cleaner. Also there may be inconsistencies with how you like to do things . You could likely merge the Multi/Multi FLI "guess" code into one code base. However, I thought a working code example is better than nothing so you can borrow whatever aspect you feel are appropriate if you want to add Multi FLI support.
Wonderful! Glad you like it 👍Yes, the difference between the screen block size and the color block size was a bit of a headache (and a head trip at times in the calculations).
The reason I called the "guessParam()" recursively was to ensure the parameters related to the chosen block color get updated (so the screen colors aren't waste choosing a color that is already available via the color block). I agree it's not technically necessary, except that the parameters are scanned in order so technically it's possible that the color block color chooses a color that some screen color already settled on using since color block params are scanned last. So on the final run it's important that to ensure that no-updates to the color block do happen which might impact the previously scanned + chosen screen colors. The final run logic waits for there to be no more image changes so in theory this will never happen, except one small corner case when the iterations get stopped because they are never truly settling on a final image. In that case the final run could leave a color block choosing a color which the screen color settled on already. I don't know if the performance hit is worth that corner case though so I'll leave you to judge.
The other way to fix would be to let each class themselves decide the order of the scan rather than the generic class, that way they can scan their color block first before the screen colors, or pass in a scanner lambda. Again, bit of an uglification of the code for the sake of a corner case.
You could also add a "const" config value in the class that decides to recursively call or not instead of the comment (not worth exposing as a gui option). If you feel compelled...
Glad you liked + merged! The dithertron is a great and friendly little tool for the retro community so I was happy to be able to contribute something!
Nice example pictures too! Great demonstration where MCI FLI really shines!
It works pretty well so far! I made the color block parameters freeze after 50% of the max iterations. You can see it (especially with certain dither settings) as a second ripple that starts halfway through the process. This has given me more food for thought on the other formats though, like maybe there needs to be a temperature decrease somehow since many regions just end up oscillating between two values. Thanks again!
@sehugg FYI - I discovered that the "speed up" can cause an issue. I was looking at the parrot image in the dithertron preview, and when I went to view in the code screen I noticed the normally black pupil eye became grey. I was puzzled as to why that could possibly happen. I poked the background color block for the eye to black in the debugger, and sure enough it went black. But yet, the color block in the export clearly color chose "grey" in the color block, not black, yet the preview on screen showed a proper black eye. What gives?
I found out. The issue is that during the 76 rounds of "updating" the color block color toggles between choosing grey + black, back and forth because the histogram colors are so extremely close in number. The dither tips the grey or the black choice back and forth yet between grey and black. In the final run, the block color chosen is grey, and then the loop aborts. Because the pixels related to that color block were not refreshed, they last didn't choose black as their pixel color because black was already chosen, but it wasn't, it was actually grey that was chosen in the final run. The preview shows the color choice of the bitmap pixel, whereas the export uses the color choice of the color block because they get out of sync since the pixels related to the color block are not refreshed.
Bottom line, that recursion is indeed required. I'm working on other patches related to the code. The bottom of the screen is 3 pixels off, and a few other things, so I'm going to investigate further and see if there's a better fix. I'll post a patch as soon as I have something useful.
Wow, I didn't know the colors actually got out of sync. Nice catch. Does it need a final pass with recursion enabled or does every pass need it? Or since guessParam() is scanned linearly, maybe it just needs to be called every 8 lines?
I think final pass is sufficient, but every single color block in the final run needs to be synced during the final, and the index choices needs to be adjusted in another final pass of "update" even if the update thought it was already in the final pass. The trouble is only the outer routine run knows it's in the final pass about to stop iterating, so that information would need to be passed into the guess routine so it could know it's in the final pass. This will impact the index choice too so it's important that the update routine happen after the commit guessing, so if the logic told the guess routine it's in the final pass, the block routine rearranges itself during that time, but if the update routine didn't re-adjust the pixel choices after that'd be a problem.
I don't think the eight line would help because it's not really a choice that happens during a particular pixel row scan since the color blocks are entirely chosen after the pixel rows are scanned. Even if you merged the two scans into one combined pass, the and the color block information was instead processed during the 8th pixel row, it would still end up being the same iterations because there are 8x as many pixel rows as color blocks, so 1/8th of the rows = 1000 color blocks, which is the same number of color blocks that exist now the way the passes are done, i.e. the recursion would still happen exactly the same number of times so no net savings in looping.
The other way to fix this is that the color blocks are scanned BEFORE the pixels are scanned. The issue is that the loop that does the guess is generic and doesn't know which guess params to scan first then after. So that means either the loop needs to be removed from the generic class into the canvas, or the generic class needs extra logic to know the order in which to perform a "guess" loop.
One other option would be that the first pixel in a column/row call a "special" guess parameters function of itself which then modifies the parameters, and the call to the guess parameters is ignored entirely for the color block area after.
I think the easiest logic might actually be the first pixel for a column/row does the color block choice. Do you want me to modify the logic to do that? I'm in that code messing about to address the FLI bug area anyway so I could do it.
Yeah if you're in the code go for it! Sorry to be so worried about performance, but I used to develop on an old Chromebook :)
The C64 Multi FLI uses a 4x1 screen block size allowing a background color, two screen ram color choices, and a third "color block" color choice option. The color block is 8x8 in size and stored in the VIC color ram in a fixed memory location. The C64 screen ram color choices can be adjusted per scan line (as done in FLI) but the color block ram address cannot be dynamically changed). The sample code changes screen ram address banks per Y scan line but leaves the color block ram alone as insufficient remaining CPU time exists to do any meaningful changes to the color block ram.
Both C64 Multi FLI and Hires FLI have the "bad scan line" bug present in the example source code. The first 3 character columns on the PAL screen will not have color information because the VIC is stalled on CPU wait cycles and thus the character color data is not available when the scan line starts to draw (and the VIC uses pixel colors that happen to remain in the registers at the time of the bad scan line is invoked). This is a known bug with the FLI technique without a solution. Most developers work around thus limitation by filling the screen character ram with a 00 pixel pattern so the pixels are transparent and display the standard background color instead. Emulators (and SCPU) however can "fix" this bug and display the full 160x200 picture but this fix is outside the scope of tool and example code.