vroland
commented
1 week ago Hi, good catch on that last pixel, looks like a bug indeed! Regarding the modulo operator: All modern compilers optimize this away anyway, so I'd optimize for readability. Regarding the other qeustion: Previously there was an issue on the S3-based boards with LUT calculation and alignment. I'm onto fixing that, which should enable subsequent refactoring and lifting this inefficiency.
I'm in the process of adding test cases for any new changes, which we can hopefully run in the Wokwi emulator. So your example cases are a good starting point for that :)
Hi,
I think there is a bug in function 'epd_hl_update_area' in file 'highlevel.c' for updating the back framebuffer. Basically, the last pixel/byte per line is NOT copied from front to back framebuffer. Consequently, the last pixel per line does NOT change on the display/screen. This effect (last pixel per line does NOT change) started me to investigate the mechanism with front and back frame buffers and updating the back frame buffer.
Relevant lines highlevel.c: 155: diff_area.x = 0; 156: diff_area.y = 0; 157: diff_area.width = epd_width(); 158: diff_area.height = epd_height(); 159: 160: int buf_width = epd_width(); 161: 162: for (int l = diff_area.y; l < diff_area.y + diff_area.height; l++) { 163: if (state->dirty_lines[l] > 0) { 164: uint8_t lfb = state->front_fb + buf_width / 2 l; 165: uint8_t lbb = state->back_fb + buf_width / 2 l; 166: 167: int x = diff_area.x; 168: int x_last = diff_area.x + diff_area.width - 1; 169: 170: if (x % 2) { 171: (lbb + x / 2) = ((lfb + x / 2) & 0xF0) | ((lbb + x / 2) & 0x0F); 172: x += 1; 173: } 174: 175: if (x_last % 2) { 176: (lbb + x_last / 2) = ((lfb + x_last / 2) & 0x0F) | ((lbb + x_last / 2) & 0xF0); 177: x_last -= 1; 178: } 179: 180: memcpy(lbb + (x / 2), lfb + (x / 2), (x_last - x) / 2); 181: } 182: }
######### Proposals ######### My proposals are:
By the way, instead of using the modulo-operator (%) for detecting even/odd, I would use the binary-and (&). I.e., instead of 'if (x % 2)' I would use 'if( x & 2 )'. The modulo-operator usually needs a multiplication, a division and a subtraction, i.e., three assembly instructions. The binary-and will be one assembly instruction. However, I suspect that the compiler also knows about this optimization and will replace 'x % 2' by 'x & 2'.
Another question: Lines 136ff (and also 155ff) are resetting 'diff_area' to the whole area of the framebuffer for drawing the front framebuffer and updating the back framebuffer. Why not using 'diff_area' here as returned by 'epd_difference_image_cropped' in line 121?
######### Investigation ######### I investigated the update of the back framebuffer as follows, using just one line with 8 pixels.
x: coordinate. front, back: color values of pixels in front and back frame buffer. addr: address of frame buffer byte. frame buffer: lower 4 bits: even pixel (x=0,2,4), upper 4 bits: odd pixels (x=1,3,5).
Case A: All 8 pixels differ in front and back frame buffer:
x 0 1 2 3 4 5 6 7 front 0 0 | 0 0 | 0 0 | 0 0 back 15 15 | 15 15 | 15 15 | 15 15 addr 0 | 1 | 2 | 3 x_first = 0 (even) x_last = 7 (odd) N_byte = (x_last-x_first+1)/2 = (7-0+1)/2 = 4 addr_start = x_first/2 = 0 --> copy 4 bytes from front to back starting at address 0
Case B: Upper 7 pixels differ in front and back frame buffer:
x 0 1 2 3 4 5 6 7 front 15 0 | 0 0 | 0 0 | 0 0 back 15 15 | 15 15 | 15 15 | 15 15 addr 0 | 1 | 2 | 3 x_first = 1 --> copy upper 4 bits from front to back at addr x_first/2=0, and x_first ++ (x_first = 2) x_last = 7 N_byte = (x_last-x_first+1)/2 = (7-2+1)/2 = 3 addr_start = x_first/2 = 1 --> copy 3 bytes from front to back starting at address 1
Case C: Lower 7 pixels differ in front and back frame buffer:
x 0 1 2 3 4 5 6 7 front 0 0 | 0 0 | 0 0 | 0 15 back 15 15 | 15 15 | 15 15 | 15 15 addr 0 | 1 | 2 | 3 x_first = 0 x_last = 6 --> copy lower 4 bits from front to back at addr x_last/2=3, and x_last --, x_last = 5 N_byte = (x_last-x_first+1)/2 = (5-0+1)/2 = 3 addr_start = x_first/2 = 0 --> copy 3 bytes from front to back starting at address 0