Faster decode

[andrews05]

This makes two changes to improve decode performance:

• px is no longer hashed and added to the index on index or run ops (with the exception of the first pixel in the image for a run - see Spec clarifications #253 (comment)).
• The bytes and pixels arrays are accessed by incrementing the pointer rather than using a separate counter (this one gave the bigger improvement for me).

Together these changes showed a 20% improvement to decode speed on a set of test images I used. If the changes are all good, I could follow up with a similar change for encode.

[amstan]

While it might make stuff faster I feel like we've lost some of the defensive programming and simplicity of the original design.

It was nice having the symmetric way to else if ((b1 & QOI_MASK_2) == ____) { everywhere, what if an OP was missed?

index[QOI_COLOR_HASH(px) % 64] = px; only appearing once at the end was pretty nice, not duplicated in a bunch of places.

The pointer changes might work if perfectly done, but the moment there's a misalignment anywhere you have pretty big problems debugging and random bytes might be interpreted as OPs, whereas before you always had the for loop +=channels. This one is could go either way though, I do see some improvements.

[andrews05]

It was nice having the symmetric way to else if ((b1 & QOI_MASK_2) == ____) { everywhere, what if an OP was missed?

This was a fairly minor speed gain so I won't mind reverting it. On the other hand, I'm not sure favouring a minor visual symmetry over performance is a good goal 😆

index[QOI_COLOR_HASH(px) % 64] = px; only appearing once at the end was pretty nice, not duplicated in a bunch of places.

Yeah, but the encoder already does it this way. The main downside here is having to deal with the (purely theoretical) edge case with a starting run.

Ideally, the spec could just be clarified to rule out the edge case. It wouldn't really be a breaking change as the current encoder doesn't trigger it and there probably aren't any other encoders that do. I did browse over a number of other implementations and spotted a few decoders that already don't handle the edge case, so ruling it out in the spec would really just be affirming that those existing decoders are actually compliant.

[phoboslab]

As mentioned in #256 I want to maintain the readability of this implementation. If you (and/or @ProkopRandacek) want to fork this repo and maintain a faster version of this library, I'm more than happy to link it in the Readme.

I did browse over a number of other implementations and spotted a few decoders that already don't handle the edge case

That's... troubling. I'm hesitant to change the spec now, as it spells out the current behavior (every pixel is put into the index). I think it's more reasonable to fix these non-compliant decoders.

[andrews05]

As mentioned in #256 I want to maintain the readability of this implementation. If you (and/or @ProkopRandacek) want to fork this repo and maintain a faster version of this library, I'm more than happy to link it in the Readme.

Sure, I understand.

That's... troubling. I'm hesitant to change the spec now, as it spells out the current behavior (every pixel is put into the index). I think it's more reasonable to fix these non-compliant decoders.

The problem is that the edge case is very easy to overlook and it's very easy/tempting to write a decoder that doesn't deal with it, especially as it won't fail any tests nor on any images the encoder would ever produce. If you're sure you want this to be spec and make sure people write compliant decoders, I think you'll need to:

1. Explicitly point out this edge case in the spec. Simply saying that each pixel that is seen goes into the index isn't enough to make people realise there's an edge case they may need to deal with. Case in point: the current encoder doesn't strictly adhere to this rule (I think you weren't aware of the edge case until I pointed it out?).
2. Add an image to the test suite that contains the edge case (of course you'd need to temporarily modify the encoder to produce such an image).

Regardless of whether you want to keep the edge case or rule it out though, I do think the spec needs to be explicit about it either way.