From 537cf2e79beac113ed9b1ba2aa5d4205fb298c1c Mon Sep 17 00:00:00 2001 From: Kyle Reed <3761006+kallanreed@users.noreply.github.com> Date: Thu, 14 Sep 2023 11:20:40 -0700 Subject: [PATCH] Fix console scroll - comment on how scrolling works (#1448) * Force console scroll area to be multiple of line height. Tons of comments. * Resize POCSAG console to fill height. * Make scoll behavior comment clearer --- firmware/application/apps/pocsag_app.hpp | 3 +- firmware/application/apps/ui_afsk_rx.hpp | 6 +- firmware/common/lcd_ili9341.cpp | 13 ++--- firmware/common/lcd_ili9341.hpp | 48 ++++++++++++++-- firmware/common/ui_widget.cpp | 73 +++++++++++++++--------- firmware/common/ui_widget.hpp | 2 +- 6 files changed, 101 insertions(+), 44 deletions(-) diff --git a/firmware/application/apps/pocsag_app.hpp b/firmware/application/apps/pocsag_app.hpp index 3e93c9ebd..ae7960c80 100644 --- a/firmware/application/apps/pocsag_app.hpp +++ b/firmware/application/apps/pocsag_app.hpp @@ -281,8 +281,9 @@ class POCSAGAppView : public View { {22 * 8, 1 * 16, 8 * 8, 20}, "Config"}; + // 54 == status bar (16) + top controls (2 * 16 + 6). Console console{ - {0, 2 * 16 + 6, screen_width, screen_height - 56}}; + {0, 2 * 16 + 6, screen_width, screen_height - 54}}; MessageHandlerRegistration message_handler_packet{ Message::ID::POCSAGPacket, diff --git a/firmware/application/apps/ui_afsk_rx.hpp b/firmware/application/apps/ui_afsk_rx.hpp index 433777e78..f876ded61 100644 --- a/firmware/application/apps/ui_afsk_rx.hpp +++ b/firmware/application/apps/ui_afsk_rx.hpp @@ -94,15 +94,15 @@ class AFSKRxView : public View { false}; Text text_debug{ - {0 * 8, 12 + 2 * 16, 240, 16}, + {0 * 8, 12 + 2 * 16, screen_width, 16}, "DEBUG"}; Button button_modem_setup{ - {240 - 12 * 8, 1 * 16, 96, 24}, + {screen_width - 12 * 8, 1 * 16, 96, 24}, "Modem setup"}; Console console{ - {0, 4 * 16, 240, 240}}; + {0, 4 * 16, 240, screen_width}}; void on_data_afsk(const AFSKDataMessage& message); diff --git a/firmware/common/lcd_ili9341.cpp b/firmware/common/lcd_ili9341.cpp index 616d2018e..7196c1914 100644 --- a/firmware/common/lcd_ili9341.cpp +++ b/firmware/common/lcd_ili9341.cpp @@ -405,13 +405,12 @@ void ILI9341::drawBMP(const ui::Point p, const uint8_t* bitmap, const bool trans } } -/* - Draw BMP from SD card. - Currently supported formats: - 16bpp ARGB, RGB565 - 24bpp RGB - 32bpp ARGB -*/ +/* Draw BMP from SD card. + * Currently supported formats: + * 16bpp ARGB, RGB565 + * 24bpp RGB + * 32bpp ARGB + */ bool ILI9341::drawBMP2(const ui::Point p, const std::filesystem::path& file) { File bmpimage; size_t file_pos = 0; diff --git a/firmware/common/lcd_ili9341.hpp b/firmware/common/lcd_ili9341.hpp index 81e502ca7..158121fdd 100644 --- a/firmware/common/lcd_ili9341.hpp +++ b/firmware/common/lcd_ili9341.hpp @@ -90,14 +90,52 @@ class ILI9341 { const ui::Color foreground, const ui::Color background); + /*** Scrolling *** + * Scrolling support is implemented in the ILI9341 driver. Basically a region + * of the screen is set up to act as a circular buffer. The VSA (vertical scroll + * address) is the line that defines the "start" of the circular buffer. In our + * case, the driver is set up for "bottom-up" scrolling. In this mode, drawing + * starts at the bottom of the scroll region and draws the buffer upward. + * However, the whole display's address space is inverted (the screen is actually + * upside down in the PortaPack) so this bottom-up drawing appears to be top-down. + * + * What this means is that the line pointed to by VSA will be drawn at the top + * of the scroll region and the line at VSA - 1 (wrapped) will be the bottom. + * Consider the following screen buffers and VSA pointers. + * + * Buffer: Display: Buffer: Display: + * VSA > A A A C NB: VSA points to the "Top" + * B B B D of the rendered output. + * C C VSA > C A + * D D D B + */ + + /* Sets the top and bottom lines of the scrolling region. */ void scroll_set_area(const ui::Coord top_y, const ui::Coord bottom_y); - ui::Coord scroll_set_position(const ui::Coord position); - ui::Coord scroll(const int32_t delta); - ui::Coord scroll_area_y(const ui::Coord y) const; void scroll_disable(); - constexpr ui::Dim width() const { return 240; } - constexpr ui::Dim height() const { return 320; } + /* Sets VSA. This an offset from top_y not a screen coordinate. */ + ui::Coord scroll_set_position(const ui::Coord position); + + /* Relative adjustment to VSA. Positive value will cause output + * to scoll "down", negative values will cause the output to scroll "up". */ + ui::Coord scroll(const int32_t delta); + + /* Gets a screen coordinate from a scroll region offset. Values are wrapped + * so the offset is wrapped within the bounds of the scroll region. The + * specified offset is applied to VSA and then offset by the scroll region + * top to produce a screen coordinate. + * Consider the following screen buffers and VSA pointer. Note the offsets. + * + * VSA > A = +0 A = +2 + * B = +1 B = +3 NB: As before, VSA is always the "top" + * C = +2 VSA > C = +0 and line at VSA - 1 will be the + * D = +3 D = +1 "bottom" or the max 'y' offset. + */ + ui::Coord scroll_area_y(const ui::Coord y) const; + + constexpr ui::Dim width() const { return ui::screen_width; } + constexpr ui::Dim height() const { return ui::screen_height; } constexpr ui::Rect screen_rect() const { return {0, 0, width(), height()}; } void draw_pixels(const ui::Rect r, const ui::Color* const colors, const size_t count); diff --git a/firmware/common/ui_widget.cpp b/firmware/common/ui_widget.cpp index 8af6c4384..d80e2cb54 100644 --- a/firmware/common/ui_widget.cpp +++ b/firmware/common/ui_widget.cpp @@ -494,7 +494,9 @@ void BigFrequency::paint(Painter& painter) { const auto rect = screen_rect(); // Erase - painter.fill_rectangle({{0, rect.location().y()}, {240, 52}}, ui::Color::black()); + painter.fill_rectangle( + {{0, rect.location().y()}, {screen_width, 52}}, + ui::Color::black()); // Prepare digits if (!_frequency) { @@ -612,10 +614,10 @@ void Console::write(std::string message) { if (!hidden() && visible()) { const Style& s = style(); const Font& font = s.font; - const auto rect = screen_rect(); + auto rect = screen_rect(); ui::Color pen_color = s.foreground; - for (const auto c : message) { + for (auto c : message) { if (escape) { if (c < std::size(term_colors)) pen_color = term_colors[(uint8_t)c]; @@ -628,12 +630,13 @@ void Console::write(std::string message) { } else if (c == '\x1B') { escape = true; } else { - const auto glyph = font.glyph(c); - const auto advance = glyph.advance(); - if ((pos.x() + advance.x()) > rect.width()) { + auto glyph = font.glyph(c); + auto advance = glyph.advance(); + // Would drawing next character be off the end? Newline. + if ((pos.x() + advance.x()) > rect.width()) crlf(); - } - const Point pos_glyph{ + + Point pos_glyph{ rect.left() + pos.x(), display.scroll_area_y(pos.y())}; display.draw_glyph(pos_glyph, glyph, pen_color, s.background); @@ -649,7 +652,6 @@ void Console::write(std::string message) { void Console::writeln(std::string message) { write(message + "\n"); - // crlf(); } void Console::paint(Painter&) { @@ -659,15 +661,23 @@ void Console::paint(Painter&) { void Console::on_show() { enable_scrolling(true); clear(); - // visible = true; } bool Console::scrolling_enabled = false; void Console::enable_scrolling(bool enable) { if (enable) { - const auto screen_r = screen_rect(); - display.scroll_set_area(screen_r.top(), screen_r.bottom()); + auto sr = screen_rect(); + auto line_height = style().font.line_height(); + + // Count full lines that can fit in console's rectangle. + auto max_lines = sr.height() / line_height; // NB: int division to floor. + + // The scroll area must be a multiple of the line_height + // or some lines will end up vertically truncated. + scroll_height = max_lines * line_height; + + display.scroll_set_area(sr.top(), sr.top() + scroll_height); display.scroll_set_position(0); scrolling_enabled = true; } else { @@ -678,28 +688,37 @@ void Console::enable_scrolling(bool enable) { void Console::on_hide() { /* TODO: Clear region to eliminate brief flash of content at un-shifted - * position? - */ + * position? */ enable_scrolling(false); - // visible = false; } void Console::crlf() { if (hidden() || !visible()) return; - const Style& s = style(); - const auto sr = screen_rect(); - const auto line_height = s.font.line_height(); - pos = {0, pos.y() + line_height}; - const int32_t y_excess = pos.y() + line_height - sr.height(); - if (y_excess > 0) { - if (!scrolling_enabled) { - enable_scrolling(true); - } - display.scroll(-y_excess); - pos = {pos.x(), pos.y() - y_excess}; + const auto& s = style(); + auto sr = screen_rect(); + auto line_height = s.font.line_height(); - const Rect dirty{sr.left(), display.scroll_area_y(pos.y()), sr.width(), line_height}; + // Advance to the next line (\n) position and "carriage return" x to 0. + pos = {0, pos.y() + line_height}; + + if (pos.y() >= scroll_height) { + // Line is past off the "bottom", need to scroll. + if (!scrolling_enabled) + enable_scrolling(true); + + // See the notes in lcd_ili9341.hpp about how scrolling works. + // The gist is that VSA will be moved to scroll the "top" off the + // screen. The drawing code uses 'scroll_area_y' to get the actual + // screen coordinate based on VSA. The "bottom" line is *always* + // at 'VSA + ((max_lines - 1) * line_height)' and so is constant. + pos = {0, scroll_height - line_height}; + + // Scroll off the "top" line. + display.scroll(-line_height); + + // Clear the new line at the "bottom". + Rect dirty{sr.left(), display.scroll_area_y(pos.y()), sr.width(), line_height}; display.fill_rectangle(dirty, s.background); } } diff --git a/firmware/common/ui_widget.hpp b/firmware/common/ui_widget.hpp index a79e2bf32..b433978b8 100644 --- a/firmware/common/ui_widget.hpp +++ b/firmware/common/ui_widget.hpp @@ -332,8 +332,8 @@ class Console : public Widget { void on_hide() override; private: - // bool visible = false; Point pos{0, 0}; + Dim scroll_height = 0; std::string buffer{}; static bool scrolling_enabled;