diff options
author | William Wilgus <wilgus.william@gmail.com> | 2020-10-31 00:18:57 -0400 |
---|---|---|
committer | William Wilgus <wilgus.william@gmail.com> | 2020-10-31 01:11:30 -0400 |
commit | 60f642ba4f6a9bf2d976f133b85b8f6a7502c14c (patch) | |
tree | a37547c0972899968a4a2ff148ded098d40f0f36 | |
parent | 202f9df0c1e6132631e9e1372d50fe8dc8e87f20 (diff) | |
download | rockbox-60f642ba4f6a9bf2d976f133b85b8f6a7502c14c.tar.gz rockbox-60f642ba4f6a9bf2d976f133b85b8f6a7502c14c.zip |
lcd_framebuffer bugfixes, sanity checks
several issues I saw that could pontentially cause problems
scroll engine doesn't take text height into account when checking bounds
NBELEMS was one whole row too large hopefully I got them right this time
Change-Id: If303da8320429c3964fc675351cb088d46303745
-rw-r--r-- | firmware/drivers/lcd-16bit-common.c | 8 | ||||
-rw-r--r-- | firmware/drivers/lcd-16bit-vert.c | 3 | ||||
-rw-r--r-- | firmware/drivers/lcd-16bit.c | 3 | ||||
-rw-r--r-- | firmware/drivers/lcd-24bit.c | 3 | ||||
-rw-r--r-- | firmware/drivers/lcd-bitmap-common.c | 42 | ||||
-rw-r--r-- | firmware/export/lcd-remote.h | 6 | ||||
-rw-r--r-- | firmware/export/lcd.h | 14 |
7 files changed, 55 insertions, 24 deletions
diff --git a/firmware/drivers/lcd-16bit-common.c b/firmware/drivers/lcd-16bit-common.c index 7c766dab8a..49e515f59f 100644 --- a/firmware/drivers/lcd-16bit-common.c +++ b/firmware/drivers/lcd-16bit-common.c | |||
@@ -81,11 +81,12 @@ void lcd_clear_viewport(void) | |||
81 | } | 81 | } |
82 | else | 82 | else |
83 | { | 83 | { |
84 | if (!lcd_backdrop) | 84 | if (lcd_backdrop && lcd_current_viewport->buffer == &lcd_framebuffer_default) |
85 | { | 85 | { |
86 | do | 86 | do |
87 | { | 87 | { |
88 | memset16(dst, lcd_current_viewport->bg_pattern, len); | 88 | memcpy(dst, PTR_ADD(dst, lcd_backdrop_offset), |
89 | len * sizeof(fb_data)); | ||
89 | dst += step; | 90 | dst += step; |
90 | } | 91 | } |
91 | while (dst <= dst_end); | 92 | while (dst <= dst_end); |
@@ -94,8 +95,7 @@ void lcd_clear_viewport(void) | |||
94 | { | 95 | { |
95 | do | 96 | do |
96 | { | 97 | { |
97 | memcpy(dst, PTR_ADD(dst, lcd_backdrop_offset), | 98 | memset16(dst, lcd_current_viewport->bg_pattern, len); |
98 | len * sizeof(fb_data)); | ||
99 | dst += step; | 99 | dst += step; |
100 | } | 100 | } |
101 | while (dst <= dst_end); | 101 | while (dst <= dst_end); |
diff --git a/firmware/drivers/lcd-16bit-vert.c b/firmware/drivers/lcd-16bit-vert.c index 8631271482..4422fdea50 100644 --- a/firmware/drivers/lcd-16bit-vert.c +++ b/firmware/drivers/lcd-16bit-vert.c | |||
@@ -38,6 +38,9 @@ | |||
38 | #include "bidi.h" | 38 | #include "bidi.h" |
39 | #include "scroll_engine.h" | 39 | #include "scroll_engine.h" |
40 | 40 | ||
41 | /*#define LOGF_ENABLE*/ | ||
42 | #include "logf.h" | ||
43 | |||
41 | #define ROW_INC 1 | 44 | #define ROW_INC 1 |
42 | #define COL_INC lcd_current_viewport->buffer->stride | 45 | #define COL_INC lcd_current_viewport->buffer->stride |
43 | 46 | ||
diff --git a/firmware/drivers/lcd-16bit.c b/firmware/drivers/lcd-16bit.c index 6a76ba48ab..d6bf5a500d 100644 --- a/firmware/drivers/lcd-16bit.c +++ b/firmware/drivers/lcd-16bit.c | |||
@@ -38,6 +38,9 @@ | |||
38 | #include "bidi.h" | 38 | #include "bidi.h" |
39 | #include "scroll_engine.h" | 39 | #include "scroll_engine.h" |
40 | 40 | ||
41 | // #define LOGF_ENABLE | ||
42 | #include "logf.h" | ||
43 | |||
41 | #define ROW_INC lcd_current_viewport->buffer->stride | 44 | #define ROW_INC lcd_current_viewport->buffer->stride |
42 | #define COL_INC 1 | 45 | #define COL_INC 1 |
43 | 46 | ||
diff --git a/firmware/drivers/lcd-24bit.c b/firmware/drivers/lcd-24bit.c index 5ee290efe3..c3aa27f7ce 100644 --- a/firmware/drivers/lcd-24bit.c +++ b/firmware/drivers/lcd-24bit.c | |||
@@ -39,6 +39,9 @@ | |||
39 | #include "bidi.h" | 39 | #include "bidi.h" |
40 | #include "scroll_engine.h" | 40 | #include "scroll_engine.h" |
41 | 41 | ||
42 | /*#define LOGF_ENABLE*/ | ||
43 | #include "logf.h" | ||
44 | |||
42 | #define ROW_INC lcd_current_viewport->buffer->stride | 45 | #define ROW_INC lcd_current_viewport->buffer->stride |
43 | #define COL_INC 1 | 46 | #define COL_INC 1 |
44 | 47 | ||
diff --git a/firmware/drivers/lcd-bitmap-common.c b/firmware/drivers/lcd-bitmap-common.c index d195fd8ebe..170ad374ea 100644 --- a/firmware/drivers/lcd-bitmap-common.c +++ b/firmware/drivers/lcd-bitmap-common.c | |||
@@ -32,6 +32,10 @@ | |||
32 | #include "string-extra.h" | 32 | #include "string-extra.h" |
33 | #include "diacritic.h" | 33 | #include "diacritic.h" |
34 | 34 | ||
35 | #ifdef LOGF_ENABLE | ||
36 | #include "panic.h" | ||
37 | #endif | ||
38 | |||
35 | #ifndef LCDFN /* Not compiling for remote - define macros for main LCD. */ | 39 | #ifndef LCDFN /* Not compiling for remote - define macros for main LCD. */ |
36 | #define LCDFN(fn) lcd_ ## fn | 40 | #define LCDFN(fn) lcd_ ## fn |
37 | #define FBFN(fn) fb_ ## fn | 41 | #define FBFN(fn) fb_ ## fn |
@@ -77,7 +81,10 @@ struct viewport* LCDFN(init_viewport)(struct viewport* vp) | |||
77 | { | 81 | { |
78 | struct frame_buffer_t *fb_default = &LCDFN(framebuffer_default); | 82 | struct frame_buffer_t *fb_default = &LCDFN(framebuffer_default); |
79 | if (!vp) /* NULL vp grabs default viewport */ | 83 | if (!vp) /* NULL vp grabs default viewport */ |
84 | { | ||
80 | vp = &default_vp; | 85 | vp = &default_vp; |
86 | vp->buffer = fb_default; | ||
87 | } | ||
81 | 88 | ||
82 | /* use defaults if no buffer is provided */ | 89 | /* use defaults if no buffer is provided */ |
83 | if (vp->buffer == NULL || vp->buffer->elems == 0) | 90 | if (vp->buffer == NULL || vp->buffer->elems == 0) |
@@ -92,6 +99,19 @@ struct viewport* LCDFN(init_viewport)(struct viewport* vp) | |||
92 | 99 | ||
93 | if (vp->buffer->get_address_fn == NULL) | 100 | if (vp->buffer->get_address_fn == NULL) |
94 | vp->buffer->get_address_fn = fb_default->get_address_fn; | 101 | vp->buffer->get_address_fn = fb_default->get_address_fn; |
102 | |||
103 | #ifdef LOGF_ENABLE | ||
104 | if ((size_t)LCD_NBELEMS(vp->width, vp->height) > vp->buffer->elems) | ||
105 | { | ||
106 | if (vp->buffer != fb_default) | ||
107 | panicf("viewport %d x %d > buffer", vp->width, vp->height); | ||
108 | logf("viewport %d x %d, %d x %d [%lu] > buffer [%lu]", vp->x, vp->y, | ||
109 | vp->width, vp->height, | ||
110 | (unsigned long) LCD_NBELEMS(vp->width, vp->height), | ||
111 | (unsigned long) vp->buffer->elems); | ||
112 | |||
113 | } | ||
114 | #endif | ||
95 | } | 115 | } |
96 | return vp; | 116 | return vp; |
97 | } | 117 | } |
@@ -474,32 +494,34 @@ static bool LCDFN(puts_scroll_worker)(int x, int y, const unsigned char *string, | |||
474 | int width, height; | 494 | int width, height; |
475 | int w, h, cwidth; | 495 | int w, h, cwidth; |
476 | bool restart; | 496 | bool restart; |
497 | struct viewport * vp = LCDFN(current_viewport); | ||
477 | 498 | ||
478 | if (!string) | 499 | if (!string) |
479 | return false; | 500 | return false; |
480 | 501 | ||
481 | /* prepare rectangle for scrolling. x and y must be calculated early | 502 | /* prepare rectangle for scrolling. x and y must be calculated early |
482 | * for find_scrolling_line() to work */ | 503 | * for find_scrolling_line() to work */ |
483 | cwidth = font_get(LCDFN(current_viewport)->font)->maxwidth; | 504 | |
484 | height = font_get(LCDFN(current_viewport)->font)->height; | 505 | cwidth = font_get(vp->font)->maxwidth; |
506 | /* get width (pixels) of the string */ | ||
507 | LCDFN(getstringsize)(string, &w, &h); | ||
508 | height = h; | ||
509 | |||
485 | y = y * (linebased ? height : 1); | 510 | y = y * (linebased ? height : 1); |
486 | x = x * (linebased ? cwidth : 1); | 511 | x = x * (linebased ? cwidth : 1); |
487 | width = LCDFN(current_viewport)->width - x; | 512 | width = vp->width - x; |
488 | 513 | ||
489 | if (y >= LCDFN(current_viewport)->height) | 514 | if (y >= vp->height || (height + y) > (vp->height)) |
490 | return false; | 515 | return false; |
491 | 516 | ||
492 | s = find_scrolling_line(x, y); | 517 | s = find_scrolling_line(x, y); |
493 | restart = !s; | 518 | restart = !s; |
494 | 519 | ||
495 | /* get width (pixeks) of the string */ | ||
496 | LCDFN(getstringsize)(string, &w, &h); | ||
497 | |||
498 | /* Remove any previously scrolling line at the same location. If | 520 | /* Remove any previously scrolling line at the same location. If |
499 | * the string width is too small to scroll the scrolling line is | 521 | * the string width is too small to scroll the scrolling line is |
500 | * cleared as well */ | 522 | * cleared as well */ |
501 | if (w < width || restart) { | 523 | if (w < width || restart) { |
502 | LCDFN(scroll_stop_viewport_rect)(LCDFN(current_viewport), x, y, width, height); | 524 | LCDFN(scroll_stop_viewport_rect)(vp, x, y, width, height); |
503 | LCDFN(putsxyofs)(x, y, x_offset, string); | 525 | LCDFN(putsxyofs)(x, y, x_offset, string); |
504 | /* nothing to scroll, or out of scrolling lines. Either way, get out */ | 526 | /* nothing to scroll, or out of scrolling lines. Either way, get out */ |
505 | if (w < width || LCDFN(scroll_info).lines >= LCDM(SCROLLABLE_LINES)) | 527 | if (w < width || LCDFN(scroll_info).lines >= LCDM(SCROLLABLE_LINES)) |
@@ -512,7 +534,7 @@ static bool LCDFN(puts_scroll_worker)(int x, int y, const unsigned char *string, | |||
512 | strlcpy(s->linebuffer, string, sizeof(s->linebuffer)); | 534 | strlcpy(s->linebuffer, string, sizeof(s->linebuffer)); |
513 | /* scroll bidirectional or forward only depending on the string width */ | 535 | /* scroll bidirectional or forward only depending on the string width */ |
514 | if ( LCDFN(scroll_info).bidir_limit ) { | 536 | if ( LCDFN(scroll_info).bidir_limit ) { |
515 | s->bidir = w < (LCDFN(current_viewport)->width) * | 537 | s->bidir = w < (vp->width) * |
516 | (100 + LCDFN(scroll_info).bidir_limit) / 100; | 538 | (100 + LCDFN(scroll_info).bidir_limit) / 100; |
517 | } | 539 | } |
518 | else | 540 | else |
@@ -526,7 +548,7 @@ static bool LCDFN(puts_scroll_worker)(int x, int y, const unsigned char *string, | |||
526 | s->y = y; | 548 | s->y = y; |
527 | s->width = width; | 549 | s->width = width; |
528 | s->height = height; | 550 | s->height = height; |
529 | s->vp = LCDFN(current_viewport); | 551 | s->vp = vp; |
530 | s->start_tick = current_tick + LCDFN(scroll_info).delay; | 552 | s->start_tick = current_tick + LCDFN(scroll_info).delay; |
531 | LCDFN(scroll_info).lines++; | 553 | LCDFN(scroll_info).lines++; |
532 | } else { | 554 | } else { |
diff --git a/firmware/export/lcd-remote.h b/firmware/export/lcd-remote.h index 030b01c736..93ca1fac7e 100644 --- a/firmware/export/lcd-remote.h +++ b/firmware/export/lcd-remote.h | |||
@@ -63,13 +63,13 @@ | |||
63 | #define LCD_REMOTE_STRIDE(w, h) (h) | 63 | #define LCD_REMOTE_STRIDE(w, h) (h) |
64 | #define LCD_REMOTE_FBSTRIDE(w, h) ((h+7)/8) | 64 | #define LCD_REMOTE_FBSTRIDE(w, h) ((h+7)/8) |
65 | #define LCD_REMOTE_FBHEIGHT LCD_REMOTE_FBSTRIDE(LCD_REMOTE_WIDTH, LCD_REMOTE_HEIGHT) | 65 | #define LCD_REMOTE_FBHEIGHT LCD_REMOTE_FBSTRIDE(LCD_REMOTE_WIDTH, LCD_REMOTE_HEIGHT) |
66 | #define LCD_REMOTE_NBELEMS(w, h) (((w*LCD_REMOTE_FBSTRIDE(w, h)) + h) / sizeof(fb_remote_data)) | 66 | #define LCD_REMOTE_NBELEMS(w, h) ((((w-1)*LCD_REMOTE_FBSTRIDE(w, h)) + h) / sizeof(fb_remote_data)) |
67 | #elif LCD_REMOTE_DEPTH == 2 | 67 | #elif LCD_REMOTE_DEPTH == 2 |
68 | #if LCD_REMOTE_PIXELFORMAT == VERTICAL_INTERLEAVED | 68 | #if LCD_REMOTE_PIXELFORMAT == VERTICAL_INTERLEAVED |
69 | #define LCD_REMOTE_STRIDE(w, h) (h) | 69 | #define LCD_REMOTE_STRIDE(w, h) (h) |
70 | #define LCD_REMOTE_FBSTRIDE(w, h) ((h+7)/8) | 70 | #define LCD_REMOTE_FBSTRIDE(w, h) ((h+7)/8) |
71 | #define LCD_REMOTE_FBHEIGHT LCD_REMOTE_FBSTRIDE(LCD_REMOTE_WIDTH, LCD_REMOTE_HEIGHT) | 71 | #define LCD_REMOTE_FBHEIGHT LCD_REMOTE_FBSTRIDE(LCD_REMOTE_WIDTH, LCD_REMOTE_HEIGHT) |
72 | #define LCD_REMOTE_NBELEMS(w, h) (((w*LCD_REMOTE_FBSTRIDE(w, h)) + h) / sizeof(fb_remote_data)) | 72 | #define LCD_REMOTE_NBELEMS(w, h) ((((w-1)*LCD_REMOTE_FBSTRIDE(w, h)) + h) / sizeof(fb_remote_data)) |
73 | #endif | 73 | #endif |
74 | #endif /* LCD_REMOTE_DEPTH */ | 74 | #endif /* LCD_REMOTE_DEPTH */ |
75 | 75 | ||
@@ -84,7 +84,7 @@ | |||
84 | 84 | ||
85 | #ifndef LCD_REMOTE_NBELEMS | 85 | #ifndef LCD_REMOTE_NBELEMS |
86 | /* At this time (2020) known remote screens only have vertical stride */ | 86 | /* At this time (2020) known remote screens only have vertical stride */ |
87 | #define LCD_REMOTE_NBELEMS(w, h) ((w*STRIDE_REMOTE(w, h)) + h) / sizeof(fb_remote_data)) | 87 | #define LCD_REMOTE_NBELEMS(w, h) (((w-1)*STRIDE_REMOTE(w, h)) + h) / sizeof(fb_remote_data)) |
88 | #define LCD_REMOTE_STRIDE(w, h) STRIDE_REMOTE(w, h) | 88 | #define LCD_REMOTE_STRIDE(w, h) STRIDE_REMOTE(w, h) |
89 | #define LCD_REMOTE_FBSTRIDE(w, h) STRIDE_REMOTE(w, h) | 89 | #define LCD_REMOTE_FBSTRIDE(w, h) STRIDE_REMOTE(w, h) |
90 | #endif | 90 | #endif |
diff --git a/firmware/export/lcd.h b/firmware/export/lcd.h index f5a3b3f1f4..ffaf1a63d2 100644 --- a/firmware/export/lcd.h +++ b/firmware/export/lcd.h | |||
@@ -456,26 +456,26 @@ typedef void lcd_blockfunc_type(fb_data *address, unsigned mask, unsigned bits); | |||
456 | #if LCD_PIXELFORMAT == HORIZONTAL_PACKING | 456 | #if LCD_PIXELFORMAT == HORIZONTAL_PACKING |
457 | #define LCD_FBSTRIDE(w, h) ((w+7)/8) | 457 | #define LCD_FBSTRIDE(w, h) ((w+7)/8) |
458 | #define LCD_FBWIDTH LCD_FBSTRIDE(LCD_WIDTH, LCD_HEIGHT) | 458 | #define LCD_FBWIDTH LCD_FBSTRIDE(LCD_WIDTH, LCD_HEIGHT) |
459 | #define LCD_NBELEMS(w, h) (((h*LCD_FBSTRIDE(w, h)) + w) / sizeof(fb_data)) | 459 | #define LCD_NBELEMS(w, h) ((((h-1)*LCD_FBSTRIDE(w, h)) + w) / sizeof(fb_data)) |
460 | #else /* LCD_PIXELFORMAT == VERTICAL_PACKING */ | 460 | #else /* LCD_PIXELFORMAT == VERTICAL_PACKING */ |
461 | #define LCD_FBSTRIDE(w, h) ((h+7)/8) | 461 | #define LCD_FBSTRIDE(w, h) ((h+7)/8) |
462 | #define LCD_FBHEIGHT LCD_FBSTRIDE(LCD_WIDTH, LCD_HEIGHT) | 462 | #define LCD_FBHEIGHT LCD_FBSTRIDE(LCD_WIDTH, LCD_HEIGHT) |
463 | #define LCD_NBELEMS(w, h) (((w*LCD_FBSTRIDE(w, h)) + h) / sizeof(fb_data)) | 463 | #define LCD_NBELEMS(w, h) ((((w-1)*LCD_FBSTRIDE(w, h)) + h) / sizeof(fb_data)) |
464 | #endif /* LCD_PIXELFORMAT */ | 464 | #endif /* LCD_PIXELFORMAT */ |
465 | #elif LCD_DEPTH == 2 | 465 | #elif LCD_DEPTH == 2 |
466 | #if LCD_PIXELFORMAT == HORIZONTAL_PACKING | 466 | #if LCD_PIXELFORMAT == HORIZONTAL_PACKING |
467 | #define LCD_FBSTRIDE(w, h) ((w+3)>>2) | 467 | #define LCD_FBSTRIDE(w, h) ((w+3)>>2) |
468 | #define LCD_NATIVE_STRIDE(s) LCD_FBSTRIDE(s, s) | 468 | #define LCD_NATIVE_STRIDE(s) LCD_FBSTRIDE(s, s) |
469 | #define LCD_FBWIDTH LCD_FBSTRIDE(LCD_WIDTH, LCD_HEIGHT) | 469 | #define LCD_FBWIDTH LCD_FBSTRIDE(LCD_WIDTH, LCD_HEIGHT) |
470 | #define LCD_NBELEMS(w, h) (((h*LCD_FBSTRIDE(w, h)) + w) / sizeof(fb_data)) | 470 | #define LCD_NBELEMS(w, h) ((((h-1)*LCD_FBSTRIDE(w, h)) + w) / sizeof(fb_data)) |
471 | #elif LCD_PIXELFORMAT == VERTICAL_PACKING | 471 | #elif LCD_PIXELFORMAT == VERTICAL_PACKING |
472 | #define LCD_FBSTRIDE(w, h) ((h+3)/4) | 472 | #define LCD_FBSTRIDE(w, h) ((h+3)/4) |
473 | #define LCD_FBHEIGHT LCD_FBSTRIDE(LCD_WIDTH, LCD_HEIGHT) | 473 | #define LCD_FBHEIGHT LCD_FBSTRIDE(LCD_WIDTH, LCD_HEIGHT) |
474 | #define LCD_NBELEMS(w, h) (((w*LCD_FBSTRIDE(w, h)) + h) / sizeof(fb_data)) | 474 | #define LCD_NBELEMS(w, h) ((((w-1)*LCD_FBSTRIDE(w, h)) + h) / sizeof(fb_data)) |
475 | #elif LCD_PIXELFORMAT == VERTICAL_INTERLEAVED | 475 | #elif LCD_PIXELFORMAT == VERTICAL_INTERLEAVED |
476 | #define LCD_FBSTRIDE(w, h) ((h+7)/8) | 476 | #define LCD_FBSTRIDE(w, h) ((h+7)/8) |
477 | #define LCD_FBHEIGHT LCD_FBSTRIDE(LCD_WIDTH, LCD_HEIGHT) | 477 | #define LCD_FBHEIGHT LCD_FBSTRIDE(LCD_WIDTH, LCD_HEIGHT) |
478 | #define LCD_NBELEMS(w, h) (((w*LCD_FBSTRIDE(w, h)) + h) / sizeof(fb_data)) | 478 | #define LCD_NBELEMS(w, h) ((((w-1)*LCD_FBSTRIDE(w, h)) + h) / sizeof(fb_data)) |
479 | #endif /* LCD_PIXELFORMAT */ | 479 | #endif /* LCD_PIXELFORMAT */ |
480 | #endif /* LCD_DEPTH */ | 480 | #endif /* LCD_DEPTH */ |
481 | /* Set defaults if not defined different yet. The defaults apply to both | 481 | /* Set defaults if not defined different yet. The defaults apply to both |
@@ -494,9 +494,9 @@ typedef void lcd_blockfunc_type(fb_data *address, unsigned mask, unsigned bits); | |||
494 | 494 | ||
495 | #ifndef LCD_NBELEMS | 495 | #ifndef LCD_NBELEMS |
496 | #if defined(LCD_STRIDEFORMAT) && LCD_STRIDEFORMAT == VERTICAL_STRIDE | 496 | #if defined(LCD_STRIDEFORMAT) && LCD_STRIDEFORMAT == VERTICAL_STRIDE |
497 | #define LCD_NBELEMS(w, h) ((w*STRIDE_MAIN(w, h)) + h) | 497 | #define LCD_NBELEMS(w, h) (((w-1)*STRIDE_MAIN(w, h)) + h) |
498 | #else | 498 | #else |
499 | #define LCD_NBELEMS(w, h) ((h*STRIDE_MAIN(w, h)) + w) | 499 | #define LCD_NBELEMS(w, h) (((h-1)*STRIDE_MAIN(w, h)) + w) |
500 | #endif | 500 | #endif |
501 | #define LCD_FBSTRIDE(w, h) STRIDE_MAIN(w, h) | 501 | #define LCD_FBSTRIDE(w, h) STRIDE_MAIN(w, h) |
502 | #endif | 502 | #endif |