From ce5ac8ec6497ceffc0c2e91432d8b15fd4bdd0bc Mon Sep 17 00:00:00 2001 From: per1234 Date: Mon, 10 Feb 2025 19:12:43 -0800 Subject: [PATCH 1/2] Correct text scroll trail clearing coordinates calculation for left and up directions The `endText` function has the capability to scroll the printed text. In order to ensure artifacts are not left behind on the display while scrolling, the library must clear the pixels at the previous location of the text after each frame of the scrolling animation. Previously, the code to handle this clearing incorrectly calculated the clearing coordinates for the leftwards (`SCROLL_LEFT`) and upwards (`SCROLL_UP`) scroll directions, having two separate problems: * The offset was subtracted rather than added. * An offset of 1 was used, which did not consider the width/height of the text. This bug might not be immediately apparent to the user because many character bitmaps do not populate any pixels on the rightmost column or bottom row of the grid, and thus those characters provide incidental self scroll trail clearing. However, this is not the case for all characters and those would cause a trail of artifacts to be left behind on the display when scrolled. The clearing coordinates calculation code is hereby corrected. NOTE: The coordinates calculation will still be incorrect for multi-line strings. However, this is not a regression because it was also incorrect before this change. The scroll trail clearing code has never had any provisions for handling multi-line strings so addition of such support is out of scope for this commit. In addition, the text scrolling code (not the scroll trail clearing code) has never correctly handled horizontal scrolling of multi-line strings, so until that is fixed it is only the lack of correct scroll trail clearing for vertical scrolling that is impactful to users. --- src/ArduinoGraphics.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ArduinoGraphics.cpp b/src/ArduinoGraphics.cpp index 852c273..06f61da 100644 --- a/src/ArduinoGraphics.cpp +++ b/src/ArduinoGraphics.cpp @@ -458,7 +458,7 @@ void ArduinoGraphics::endText(int scrollDirection) beginDraw(); int const text_x = _textX - i; text(_textBuffer, text_x, _textY); - bitmap(_font->data[0x20], text_x - 1, _textY, 1, _font->height, _textSizeX, _textSizeY); + bitmap(_font->data[0x20], text_x + _textBuffer.length() * _font->width, _textY, 1, _font->height, _textSizeX, _textSizeY); endDraw(); delay(_textScrollSpeed); @@ -482,7 +482,7 @@ void ArduinoGraphics::endText(int scrollDirection) beginDraw(); int const text_y = _textY - i; text(_textBuffer, _textX, text_y); - bitmap(_font->data[0x20], _textX, text_y - 1, _font->width, 1, _textSizeX, _textSizeY); + bitmap(_font->data[0x20], _textX, text_y + _font->height, _font->width, 1, _textSizeX, _textSizeY); endDraw(); delay(_textScrollSpeed); From 83d6df0c238e9b39b40c3835f74897d62eba2409 Mon Sep 17 00:00:00 2001 From: per1234 Date: Mon, 10 Feb 2025 21:18:11 -0800 Subject: [PATCH 2/2] Clear trail for full string width when scrolling text vertically The `endText` function has the capability to scroll the printed text. In order to ensure artifacts are not left behind on the display while scrolling, the library must clear the pixels at the previous location of the text after each frame of the scrolling animation. Previously, the code to handle this clearing for vertical scrolling only cleared a single character's width. This meant that scroll trail clearing was only done for the first character in the string. This bug might not be immediately apparent to the user because many character bitmaps do not populate any pixels on the top or bottom row of the grid, and thus those characters provide incidental self scroll trail clearing. However, this is not the case for all characters and those would cause a trail of artifacts to be left behind on the display when scrolled. The vertical scroll trail clearing code is hereby corrected to cover the full width of the string. Previously the `ArduinoGraphics::bitmap` function was used by the clearing code. That approach was reasonable for clearing the scroll trail of a single character, but due to the function's eight pixel width limitation, it is not suitable to use with strings. In this application where a line of arbitrary length, but only one pixel thick is needed, the `ArduinoGraphics::line` function is the suitable tool. So the code is ported to using `ArduinoGraphics::line`. NOTE: The calculations of the length of the clearing line will still be incorrect for multi-line strings. However, this is not a regression because it was also incorrect before this change. The scroll trail clearing code has never had any provisions for handling multi-line strings so adding such support is out of scope for this commit. In addition, the text scrolling code (not the scroll trail clearing code) has never correctly handled horizontal scrolling of multi-line strings, so until that is fixed it is only the lack of correct scroll trail clearing for vertical scrolling that is impactful to users. --- .codespellrc | 2 +- src/ArduinoGraphics.cpp | 39 ++++++++++++++++++++++++++++++++++----- 2 files changed, 35 insertions(+), 6 deletions(-) diff --git a/.codespellrc b/.codespellrc index 33d69d0..a41b93c 100644 --- a/.codespellrc +++ b/.codespellrc @@ -1,7 +1,7 @@ # See: https://github.com/codespell-project/codespell#using-a-config-file [codespell] # In the event of a false positive, add the problematic word, in all lowercase, to a comma-separated list here: -ignore-words-list = mis +ignore-words-list = cleary,mis check-filenames = check-hidden = skip = ./.git diff --git a/src/ArduinoGraphics.cpp b/src/ArduinoGraphics.cpp index 06f61da..89083c0 100644 --- a/src/ArduinoGraphics.cpp +++ b/src/ArduinoGraphics.cpp @@ -448,17 +448,21 @@ void ArduinoGraphics::endText(int scrollDirection) uint8_t strokeG = _strokeG; uint8_t strokeB = _strokeB; - - stroke(_textR, _textG, _textB); - if (scrollDirection == SCROLL_LEFT) { int scrollLength = _textBuffer.length() * textFontWidth() + _textX; for (int i = 0; i < scrollLength; i++) { beginDraw(); + int const text_x = _textX - i; + stroke(_textR, _textG, _textB); text(_textBuffer, text_x, _textY); - bitmap(_font->data[0x20], text_x + _textBuffer.length() * _font->width, _textY, 1, _font->height, _textSizeX, _textSizeY); + + // clear previous position + const int clearX = text_x + _textBuffer.length() * _font->width; + stroke(_backgroundR, _backgroundG, _backgroundB); + line(clearX, _textY, clearX, _textY + _font->height - 1); + endDraw(); delay(_textScrollSpeed); @@ -468,9 +472,18 @@ void ArduinoGraphics::endText(int scrollDirection) for (int i = 0; i < scrollLength; i++) { beginDraw(); + int const text_x = _textX - (scrollLength - i - 1); + stroke(_textR, _textG, _textB); text(_textBuffer, text_x, _textY); + + // clear previous position + const int clearX = text_x - 1; + stroke(_backgroundR, _backgroundG, _backgroundB); + line(clearX, _textY, clearX, _textY + _font->height - 1); + bitmap(_font->data[0x20], text_x - 1, _textY, 1, _font->height, _textSizeX, _textSizeY); + endDraw(); delay(_textScrollSpeed); @@ -480,9 +493,16 @@ void ArduinoGraphics::endText(int scrollDirection) for (int i = 0; i < scrollLength; i++) { beginDraw(); + int const text_y = _textY - i; + stroke(_textR, _textG, _textB); text(_textBuffer, _textX, text_y); - bitmap(_font->data[0x20], _textX, text_y + _font->height, _font->width, 1, _textSizeX, _textSizeY); + + // clear previous position + const int clearY = text_y + _font->height; + stroke(_backgroundR, _backgroundG, _backgroundB); + line(_textX, clearY, _textX + (_font->width * _textBuffer.length()) - 1, clearY); + endDraw(); delay(_textScrollSpeed); @@ -492,8 +512,16 @@ void ArduinoGraphics::endText(int scrollDirection) for (int i = 0; i < scrollLength; i++) { beginDraw(); + int const text_y = _textY - (scrollLength - i - 1); + stroke(_textR, _textG, _textB); text(_textBuffer, _textX, text_y); + + // clear previous position + const int clearY = text_y - 1; + stroke(_backgroundR, _backgroundG, _backgroundB); + line(_textX, clearY, _textX + (_font->width * _textBuffer.length()) - 1, clearY); + bitmap(_font->data[0x20], _textX, text_y - 1, _font->width, 1, _textSizeX, _textSizeY); endDraw(); @@ -501,6 +529,7 @@ void ArduinoGraphics::endText(int scrollDirection) } } else { beginDraw(); + stroke(_textR, _textG, _textB); text(_textBuffer, _textX, _textY); endDraw(); }