Last pixel per line does not change on display

[dapeda42]

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:

1. Line 175 should be 'if (!(x_last % 2)) {', i.e. considering even values in x_last, i.e. x_last=0,2,4.
2. Line 180 should be 'memcpy(lbb + (x / 2), lfb + (x / 2), (x_last - x + 1) / 2);', i.e. adding 1 at inner last bracket for byte count.

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

[vroland]

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 :)

[dapeda42]

I think I found another issue: the whole framebuffer is shifted by one row with respect to the actual pixels of the display.
In other words: the first line of the framebuffer does not appear in the first row of the display, but in the second row.
Consequently, the last line of the framebuffer does NOT appear on the display at all (shifted by one row and this row of pixel does not exist).
Finally, using the framebuffer, the first row of pixels can not be changed or made black.

My guess is that the two render tasks and the framebuffer are somehow not aligned (its hard for me to understand the complex approach with the line queue, render tasks and so on).

With a modified version of "epd_clear_area_cycles" (i.e., not using the framebuffer but the LCD driver only) I can make ALL pixels of the display black, even those in the first row.

Testcase:

1. Display: ED097TC2 with 1200x825 pixels
2. C-Code (I hope this works, because I actually use another API):

// epd_clear_area_cycles_black: derived from epd_clear_area_cycles, but make all pixels BLACK
void epd_clear_area_cycles_black(EpdRect area, int cycles, int cycle_time) {
    const short white_time = cycle_time;
    const short dark_time = cycle_time;

    for (int c = 0; c < cycles; c++) {
        for (int i = 0; i < 10; i++) {
            epd_push_pixels(area, white_time, 1);  // original dark_time,0);
        }
        for (int i = 0; i < 10; i++) {
            epd_push_pixels(area, dark_time, 0); // original white_time,1);
        }
        for (int i = 0; i < 2; i++) {
            epd_push_pixels(area, white_time, 2);
        }
    }
}
uint16_t w=1200,h=825;
EpdRect rect_0 = {.x=0,.y=0,.width=w,  .height=h);
EpdRect rect_1 = {.x=1,.y=1,.width=w-2,.height=h-2);
EpdRect rect_2 = {.x=2,.y=2,.width=w-4,.height=h-4);
EpdiyHighlevelState hl;

epd_poweron();

// Make all pixels black (directly, WITHOUT framebuffer, LUT and render tasks)
epd_clear_area_cycles_black(); // own function, derived from epd_clear_area_cycles, see above
// Effect: ALL pixels of display are black, even those in the first row.
vTaskDelay(1000);

// Make all pixels white (directly, WITHOUT framebuffer, LUT and render tasks)
epd_clear_area_cycles();  // original function
// Effect: ALL pixels of display are white, even those in the first row.
vTaskDelay(1000);

int8_t temp = epd_ambient_temperature();
uint8_t *fb = epd_hl_get_framebuffer(&hl);

epd_hl_set_all_white(&hl);       // Clear framebuffer
epd_draw_rect(rect_0, 0, fb);    // Draw outer rectangle
// Effects:
//  - ONE row of white pixels above rectangle (not OK: this white row should NOT exist)
//  - No white pixels at left and right side of rectangle (OK)
//  - Bottom line of rectangle NOT visible (not OK: this line should be visible)
epd_hl_update_screen(&hl, MODE_GL16, temp);
vTaskDelay(1000);

epd_hl_set_all_white(&hl);     // Clear framebuffer
epd_draw_rect(rect_1, 0, fb);  // Draw first inner rectangle 
// Effects:
//  - TWO rows of white pixels above rectangle (not OK: this should be ONE row only)
//  - One white pixel at left and right side of rectangle (OK)
//  - Bottom line of rectangle visible, but NO white row below (not OK: there should be a white row below)
epd_hl_update_screen(&hl, MODE_GL16, temp);
vTaskDelay(1000);

epd_hl_set_all_white(&hl);     // Clear framebuffer
epd_draw_rect(rect_2, 0, fb);  // Draw second inner rectangle 
// Effects:
//  - THREE rows of white pixels above rectangle (not OK: this should be TWO rows only)
//  - Two white pixels at left and right side of rectangle (OK)
//  - One row of white pixels below rectangle (not OK: there should be a TWO white rows below)
epd_hl_update_screen(&hl, MODE_GL16, temp);

epd_poweroff();

[martinberlin]

Great work. I would like to see that issues are so well documented and explained like this. That’s the way to do it.
After this “sabbatical month” of august where most of us are on holidays we will hopefully be able to update this. Since you know where is the issue, would you like to propose a fix, so you get more into epdiy internals?

[dapeda42]

I'm also on holidays now. Maybe I have some time around mid of September to find the root cause of the issue.Am 16.08.2024 23:57 schrieb "Martin F." ***@***.***>: Great work. I would like to see that issues are so well documented and explained like this. That’s the way to do it. After this “sabbatical month” of august where most of us are on holidays we will hopefully be able to update this. Since you know where is the issue, would you like to propose a fix, so you get more into epdiy internals? —Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you authored the thread.Message ID: ***@***.***>