BlitBuffer: Make sure we *never* update a color reference

[NiLuJe]

In the vast majority if cases, in blitbuffer, when we're passed a Color? object, it's a direct reference (e.g., Color8 &) to the buffer itself. As such, if we update it, we're writing to the buffer.

This is obviously a terrible idea (except in the cases where we actually want to write to the buffer ^^), and shit starts getting really scary when you remember that we most often do that via the getColor? methods, which do pixel-format conversions. Ouch.

I'm mildly amazed this has never blown up anywhere in fun and interesting ways before, but, this was at least exposed by @poire-z's testcase in koreader/koreader#5916 (comment)
ImageViewer starts by showing the image full-screen, then a tap scales it in order to show the buttons. Something was modifying the buffer in-between those two states, which meant that the scaled buffer was slightly off. This was made more obvious when dithering enters the fray, as this generated a different, more obvious dither pattern.

---

Also, compute the dither diffusion based on the source coordinates, not the target's. (Much like the C blitter).

---

This doesn't happen with the C blitter, because, there, the distinction between what's a pointer/ref and what's a value is much clearer in plain C. (Also, the compiler would shout at you very sternly when this kind of thing happens and involves type-punning. Also, const exists in C ^^).

---

This change is 

[poire-z]

Tested (as of 0e87556) on my Kobo:

• now works without issue (no crash) when [x] Disable C blitter [x] Disable SW dithering.
• no crash either when [x] Disable C blitter [ ] Disable SW dithering, but exact same behaviour as with [ ] Disable C blitter [ ] Disable SW dithering : crappy full-refresh, dithering grid, degradation of the grid and colors, remanence everywhere, and lateral solid black stripes over the book when quitting the ImageViewer. Then, there, a full refresh (small diagonal swipe) restores everything cleanly.

So, the SW dithering causes same issues with both C/!C blitting.
What piece of code is doing the dithering? just different methods in blitbuffer.c and blitbuffer.lua ? Nothing "Neon" specific that could cause issues on older arm devices?
Or may be some byte alignment issue that would only trigger on my 1072 x 1448 GloHD and not on other multiple of 16 sized screens (1448 is not /16)?

Oh, but no issue if, in ImageViewer, I pinch to zoom and make the image smaller than the ImageViewer area ! Only when Scale or Original size (both make the images touch the top/bottomborders - with initially white stripes on left/right - these white stripes becomes black or grey on each further refreshes).
So, may be some issue in ImageViewer blitbuffer scaling w/ dithering ? The scaling was initialy my code, a bit crappy by making a huge BB and cropping it on display I guess, so I wouldn't be surprised if it has bugs :)

[NiLuJe]

Same dithering code on both sides, with the grid now aligned the exact same way. (And, remember, I'm testing this on an H2O, which technically uses an older SoC than your Glo HD ;)). And it's plain C/LuaFFI byte-by-byte, so, no alignment constraints.
(... on our end. The EPDC might be doing copies on its end and/or doing weird things that exacerbate the potential for "I'm allergic to grid patterns" ^^).

It's just that a dithered image is extremely prone to ghosting and weird eInk shenanigans.

IIRC, ImageViewer now defers scaling to MuPDF, so, stuff should be sane on that front, I think?

---

Nice to know that this did fix a crash/hang, though ^^.

[poire-z]

The dither code is way over my head - but something I don't get: what are x and y in there:

koreader-base/ffi/blitbuffer.lua

Lines 230 to 247 in d980ec9

	local function dither_o8x8(x, y, v)
	-- Constants:
	-- Quantum = 8; Levels = 16; map Divisor = 65
	-- QuantumRange = 0xFF
	-- QuantumScale = 1.0 / QuantumRange
	--
	-- threshold = QuantumScale * v * ((L-1) * (D-1) + 1)
	-- NOTE: The initial computation of t (specifically, what we pass to DIV255) would overflow an uint8_t.
	-- So jump to shorts, and do it signed to be extra careful, although I don't *think* we can ever underflow here.
	local t = div255(v * (lshift(15, 6) + 1))
	-- level = t / (D-1);
	local l = rshift(t, 6)
	-- t -= l * (D-1);
	t = t - lshift(l, 6)
	-- map width & height = 8
	-- c = ClampToQuantum((l+(t >= map[(x % mw) + mw * (y % mh)])) * QuantumRange / (L-1));
	local q = (l + (t >= threshold_map_o8x8[band(x, 7) + (8 * band(y, 7))] and 1 or 0)) * 17

They seem to be coordinates of the input pixel we get the original color from as v.
But how x/y (not even peek'ing at the color of pixels neighbours of x/y) affect the resulting color? Just them being %8 either 0,1,2,3,4,5,6,7 tweaks the resulting color, no matter the surrounding pixels ? expecting this will work just fine on a sufficiently large same-color area ?

[NiLuJe]

Yep, hence "ordered" dither and the grid pattern (as opposed to error diffusion algorithms which do push error into surrounding pixels).

[poire-z]

No issue with [x] Disable C blitter [ ] Disable SW dithering and:

 local function dither_o8x8(x, y, v)
+    if true then return v end
     -- Constants:

No issue if I put this line at the end of the function (to still have the computation done).

So I guess our code is fine, and it's indeed down the hardware that things get bad because of the patterns of color values :/

[poire-z]

I know the x/y may not be real screen x/y coordinates (or are they when we do dithering? only to the final framebuffer?) but:

local function dither_o8x8(x, y, v)
    if x < 10 or y < 10 then return v end
    if x > 1061 or y > 1437 then return v end
    [...]

limits a lot the issues I had!

Scratch that... what limited the issues was having my Kobo plugged into USB (so I could do these edits and quickly test)...

Plugged in/charging : no issue (even without the above hack)
Not plugged in: issues... :/

[NiLuJe]

Huh. Might affect the voltages applied by the waveform? Gremlins!

FWIW, makes no difference on my end (and neither does toggling Wi-Fi, which is extra-funky on the H2O because of the whole DVFS thingy).