Screen not reflective of output and chosen color is bad

[robin-raymond]

I noticed when I was running some FLI examples through the code what I saw on screen looked way better than what I was getting in my FLI code. At first I thought it was my FLI code was just bad/wrong. But nope, it's the generated output is bad, and I tracked it down to this routine as being the culprit: update(offset: number) in export class BaseDitheringCanvas.

        // set new pixel rgb
        const errmag = (Math.abs(err[0]) + Math.abs(err[1]*2) + Math.abs(err[2])) / (256 * 4);
        if (this.indexed[offset] != palidx) {
            let shouldChange = (errmag >= this.errorThreshold);
            if (!shouldChange) {
                let existingValue = this.indexed[offset];
                // double check the old value is still legal since changing the value is not desired
                shouldChange = (valid.find((x) => existingValue === x) === undefined);
            }
            if (shouldChange) {
                this.indexed[offset] = palidx;
                this.changes++;
            }
        }
        this.img[offset] = rgbimg;

In this routine, there's a bug/glitch. The screen display does not match the chosen color if the shouldChange value turns out to be false. This means what you see visually may not match the output, and can drastically not match the output too.

Take this asset and run it through c64 multi fli (bug or no bug doesn't matter):

Choose the image, default display window, and look at the dark patch near the left middle of the image. The conversion is okay.

Now "fix" the code and look at the same image:

        // set new pixel rgb
        const errmag = (Math.abs(err[0]) + Math.abs(err[1]*2) + Math.abs(err[2])) / (256 * 4);
        if (this.indexed[offset] != palidx) {
            let shouldChange = (errmag >= this.errorThreshold);
            if (!shouldChange) {
                let existingValue = this.indexed[offset];
                // double check the old value is still legal since changing the value is not desired
                shouldChange = (valid.find((x) => existingValue === x) === undefined);
            }
            if (shouldChange) {
                this.indexed[offset] = palidx;
                this.changes++;
                this.img[offset] = rgbimg;   // screen is ONLY updated when pixel is actually changed
            }
        }

Now compare the "good" screen display:

With the "fixed" code screen display:

Notice the difference?

The issue is that let shouldChange = (errmag >= this.errorThreshold); often is false which does not update the actual indexed output. With the unfixed code what you see looks good, but what you get is not so good. With the fixed code what you see and what you get are both bad. If you change let shouldChange = true; and allow it to always change then the output looks great (but at the side effect of always changing the value).

I'm not sure about this errorThreshold stuff so I wanted to point out what the bug is so you can let me know the correct solution.