From ae6bdc4e29a6e38d0305d620b2301de6328dde0e Mon Sep 17 00:00:00 2001 From: Daniel Bertalan Date: Wed, 2 Jun 2021 15:36:31 +0200 Subject: [PATCH] LibVT+Kernel: Clean up scroll API This commit cleans up some of the `#ifdef`-ed code smell in `Terminal`, by extending the scroll APIs to take a range of lines as a parameter. This makes it possible to use the same code for `IL`/`DL` as for scrolling. Note that the current scrolling implementation is very naive, and does many insertions/deletions in the middle of arrays, whereas swaps should be enough. This optimization will come in a later commit. The `linefeed` override was removed from `VirtualConsole`. Previously, it exhibited incorrect behavior by moving to column 0. Now that we use the method defined in `Terminal`, code which relied on this behavior stopped working. We go instead go through the TTY layer which handles the various output flags. Passing the input character-by-character seems a bit excessive, so a fix for it will come in another PR. --- Kernel/TTY/TTY.h | 5 +- Kernel/TTY/VirtualConsole.cpp | 82 +++++++++-------- Kernel/TTY/VirtualConsole.h | 13 +-- Userland/Libraries/LibVT/Terminal.cpp | 122 +++++++++++++++----------- Userland/Libraries/LibVT/Terminal.h | 32 ++++--- 5 files changed, 141 insertions(+), 113 deletions(-) diff --git a/Kernel/TTY/TTY.h b/Kernel/TTY/TTY.h index 9e5ee61bfd..7813b0abf9 100644 --- a/Kernel/TTY/TTY.h +++ b/Kernel/TTY/TTY.h @@ -58,7 +58,7 @@ protected: TTY(unsigned major, unsigned minor); void emit(u8, bool do_evaluate_block_conditions = false); - virtual void echo(u8) = 0; + void echo_with_processing(u8); bool can_do_backspace() const; void do_backspace(); @@ -80,7 +80,8 @@ protected: private: // ^CharacterDevice virtual bool is_tty() const final override { return true; } - inline void echo_with_processing(u8); + + virtual void echo(u8) = 0; template void process_output(u8, Functor put_char); diff --git a/Kernel/TTY/VirtualConsole.cpp b/Kernel/TTY/VirtualConsole.cpp index ceef83cd0a..2684783ab1 100644 --- a/Kernel/TTY/VirtualConsole.cpp +++ b/Kernel/TTY/VirtualConsole.cpp @@ -48,6 +48,9 @@ void ConsoleImpl::set_size(u16 determined_columns, u16 determined_rows) m_columns = determined_columns; m_rows = determined_rows; + m_scroll_region_top = 0; + m_scroll_region_bottom = determined_rows - 1; + m_current_state.cursor.clamp(rows() - 1, columns() - 1); m_normal_saved_state.cursor.clamp(rows() - 1, columns() - 1); m_alternate_saved_state.cursor.clamp(rows() - 1, columns() - 1); @@ -59,52 +62,34 @@ void ConsoleImpl::set_size(u16 determined_columns, u16 determined_rows) m_horizontal_tabs[determined_columns - 1] = 1; m_client.terminal_did_resize(m_columns, m_rows); } -void ConsoleImpl::scroll_up() +void ConsoleImpl::scroll_up(u16 region_top, u16 region_bottom, size_t count) { // NOTE: We have to invalidate the cursor first. m_client.invalidate_cursor(cursor_row()); - m_client.scroll_up(); + m_client.scroll_up(region_top, region_bottom, count); } -void ConsoleImpl::scroll_down() + +void ConsoleImpl::scroll_down(u16 region_top, u16 region_bottom, size_t count) { + m_client.invalidate_cursor(cursor_row()); + m_client.scroll_down(region_top, region_bottom, count); } -void ConsoleImpl::linefeed() -{ - u16 new_row = cursor_row(); - u16 max_row = rows() - 1; - if (new_row == max_row) { - // NOTE: We have to invalidate the cursor first. - m_client.invalidate_cursor(new_row); - m_client.scroll_up(); - } else { - ++new_row; - } - set_cursor(new_row, 0); -} + void ConsoleImpl::put_character_at(unsigned row, unsigned column, u32 ch) { m_client.put_character_at(row, column, ch, m_current_state.attribute); m_last_code_point = ch; } -void ConsoleImpl::set_window_title(const String&) -{ -} + void ConsoleImpl::ICH(Parameters) { // FIXME: Implement this } -void ConsoleImpl::IL(Parameters) -{ - // FIXME: Implement this -} + void ConsoleImpl::DCH(Parameters) { // FIXME: Implement this } -void ConsoleImpl::DL(Parameters) -{ - // FIXME: Implement this -} void VirtualConsole::set_graphical(bool graphical) { @@ -196,8 +181,11 @@ UNMAP_AFTER_INIT VirtualConsole::VirtualConsole(const unsigned index, const Circ , m_console_impl(*this) { initialize(); - for (auto& ch : log) { - echo(ch); + // HACK: We have to go through the TTY layer for correct newline handling. + // It would be nice to not have to make all these calls, but we can't get the underlying data pointer + // and head index. If we did that, we could reduce this to at most 2 calls. + for (auto ch : log) { + emit_char(ch); } } @@ -305,7 +293,9 @@ void VirtualConsole::set_active(bool active) void VirtualConsole::emit_char(char ch) { - echo(ch); + // Since we are standards-compliant by not moving to column 1 on '\n', we have to add an extra carriage return to + // do newlines properly. The `TTY` layer handles adding it. + echo_with_processing(static_cast(ch)); } void VirtualConsole::flush_dirty_lines() @@ -381,10 +371,7 @@ String VirtualConsole::device_name() const void VirtualConsole::echo(u8 ch) { - if (should_echo_input()) { - auto buffer = UserOrKernelBuffer::for_kernel_buffer(&ch); - on_tty_write(buffer, 1); - } + m_console_impl.on_input(ch); } VirtualConsole::Cell& VirtualConsole::cell_at(size_t x, size_t y) @@ -407,11 +394,30 @@ void VirtualConsole::clear() m_console_impl.set_cursor(0, 0); } -void VirtualConsole::scroll_up() +void VirtualConsole::scroll_up(u16 region_top, u16 region_bottom, size_t count) { - memmove(m_cells->vaddr().as_ptr(), m_cells->vaddr().offset(columns() * sizeof(Cell)).as_ptr(), ((rows() - 1) * columns() * sizeof(Cell))); - clear_line(rows() - 1); - m_console_impl.m_need_full_flush = true; + VERIFY(region_top <= region_bottom); + size_t region_size = region_bottom - region_top + 1; + count = min(count, region_size); + size_t line_bytes = (columns() * sizeof(Cell)); + memmove(m_cells->vaddr().offset(line_bytes * region_top).as_ptr(), m_cells->vaddr().offset(line_bytes * (region_top + count)).as_ptr(), line_bytes * (region_size - count)); + for (size_t i = 0; i < count; ++i) + clear_line(region_bottom - i); + for (u16 row = region_top; row <= region_bottom; ++row) + m_lines[row].dirty = true; +} + +void VirtualConsole::scroll_down(u16 region_top, u16 region_bottom, size_t count) +{ + VERIFY(region_top <= region_bottom); + size_t region_size = region_bottom - region_top + 1; + count = min(count, region_size); + size_t line_bytes = (columns() * sizeof(Cell)); + memmove(m_cells->vaddr().offset(line_bytes * (region_top + count)).as_ptr(), m_cells->vaddr().offset(line_bytes * region_top).as_ptr(), line_bytes * (region_size - count)); + for (size_t i = 0; i < count; ++i) + clear_line(region_top + i); + for (u16 row = region_top; row <= region_bottom; ++row) + m_lines[row].dirty = true; } void VirtualConsole::clear_line(size_t y_index) diff --git a/Kernel/TTY/VirtualConsole.h b/Kernel/TTY/VirtualConsole.h index fe0b62a23c..69f6eecf90 100644 --- a/Kernel/TTY/VirtualConsole.h +++ b/Kernel/TTY/VirtualConsole.h @@ -37,17 +37,12 @@ private: virtual void clear() override; virtual void clear_including_history() override; - virtual void scroll_up() override; - virtual void scroll_down() override; - virtual void linefeed() override; + virtual void scroll_up(u16 region_top, u16 region_bottom, size_t count) override; + virtual void scroll_down(u16 region_top, u16 region_bottom, size_t count) override; virtual void put_character_at(unsigned row, unsigned column, u32 ch) override; - virtual void set_window_title(const String&) override; virtual void ICH(Parameters) override; - - virtual void IL(Parameters) override; virtual void DCH(Parameters) override; - virtual void DL(Parameters) override; }; class VirtualConsole final : public TTY @@ -140,8 +135,8 @@ private: void on_code_point(u32); - void scroll_down(); - void scroll_up(); + void scroll_down(u16 region_top, u16 region_bottom, size_t count); + void scroll_up(u16 region_top, u16 region_bottom, size_t count); void clear_line(size_t index); void put_character_at(unsigned row, unsigned column, u32 ch, const VT::Attribute&); diff --git a/Userland/Libraries/LibVT/Terminal.cpp b/Userland/Libraries/LibVT/Terminal.cpp index 5d3700d603..413d572c90 100644 --- a/Userland/Libraries/LibVT/Terminal.cpp +++ b/Userland/Libraries/LibVT/Terminal.cpp @@ -638,8 +638,7 @@ void Terminal::SU(Parameters params) if (params.size() >= 1 && params[0] != 0) count = params[0]; - for (u16 i = 0; i < count; i++) - scroll_up(); + scroll_up(count); } void Terminal::SD(Parameters params) @@ -648,8 +647,7 @@ void Terminal::SD(Parameters params) if (params.size() >= 1 && params[0] != 0) count = params[0]; - for (u16 i = 0; i < count; i++) - scroll_down(); + scroll_down(count); } void Terminal::DECSCUSR(Parameters params) @@ -681,54 +679,36 @@ void Terminal::DECSCUSR(Parameters params) } } -#ifndef KERNEL void Terminal::IL(Parameters params) { - unsigned count = 1; + size_t count = 1; if (params.size() >= 1 && params[0] != 0) count = params[0]; - invalidate_cursor(); - for (; count > 0; --count) { - active_buffer().insert(cursor_row(), make(m_columns)); - if (m_scroll_region_bottom + 1 < active_buffer().size()) - active_buffer().remove(m_scroll_region_bottom + 1); - else - active_buffer().remove(active_buffer().size() - 1); + if (!is_within_scroll_region(cursor_row())) { + dbgln("Shenanigans! Tried to insert line outside the scroll region"); + return; } - - m_need_full_flush = true; + scroll_down(cursor_row(), m_scroll_region_bottom, count); } -#endif void Terminal::DA(Parameters) { emit_string("\033[?1;0c"); } -#ifndef KERNEL void Terminal::DL(Parameters params) { - int count = 1; + size_t count = 1; if (params.size() >= 1 && params[0] != 0) count = params[0]; - - if (count == 1 && cursor_row() == 0) { - scroll_up(); + if (!is_within_scroll_region(cursor_row())) { + dbgln("Shenanigans! Tried to delete line outside the scroll region"); return; } - - int max_count = m_rows - (m_scroll_region_top + cursor_row()); - count = min(count, max_count); - - for (int c = count; c > 0; --c) { - active_buffer().remove(cursor_row()); - if (m_scroll_region_bottom < active_buffer().size()) - active_buffer().insert(m_scroll_region_bottom, make(m_columns)); - else - active_buffer().append(make(m_columns)); - } + scroll_up(cursor_row(), m_scroll_region_bottom, count); } +#ifndef KERNEL void Terminal::DCH(Parameters params) { int num = 1; @@ -765,33 +745,75 @@ void Terminal::linefeed() void Terminal::carriage_return() { + dbgln_if(TERMINAL_DEBUG, "Carriage return"); set_cursor(cursor_row(), 0); } -#ifndef KERNEL -void Terminal::scroll_up() +void Terminal::scroll_up(size_t count) { - dbgln_if(TERMINAL_DEBUG, "Scroll up 1 line"); - // NOTE: We have to invalidate the cursor first. - invalidate_cursor(); - if (m_scroll_region_top == 0 && !m_use_alternate_screen_buffer) { - auto line = move(active_buffer().ptr_at(m_scroll_region_top)); - add_line_to_history(move(line)); - m_client.terminal_history_changed(); - } - active_buffer().remove(m_scroll_region_top); - active_buffer().insert(m_scroll_region_bottom, make(m_columns)); - m_need_full_flush = true; + scroll_up(m_scroll_region_top, m_scroll_region_bottom, count); } -void Terminal::scroll_down() +void Terminal::scroll_down(size_t count) { - dbgln_if(TERMINAL_DEBUG, "Scroll down 1 line"); + scroll_down(m_scroll_region_top, m_scroll_region_bottom, count); +} + +#ifndef KERNEL +// Insert `count` blank lines at the bottom of the region. Text moves up, top lines get added to the scrollback. +void Terminal::scroll_up(u16 region_top, u16 region_bottom, size_t count) +{ + VERIFY(region_top <= region_bottom); + VERIFY(region_bottom < rows()); + // Only the specified region should be affected. + size_t region_size = region_bottom - region_top + 1; + count = min(count, region_size); + dbgln_if(TERMINAL_DEBUG, "Scroll up {} lines in region {}-{}", count, region_top, region_bottom); // NOTE: We have to invalidate the cursor first. invalidate_cursor(); - active_buffer().remove(m_scroll_region_bottom); - active_buffer().insert(m_scroll_region_top, make(m_columns)); - m_need_full_flush = true; + + if (!m_use_alternate_screen_buffer && max_history_size() != 0) + for (size_t i = 0; i < count; ++i) + add_line_to_history(move(active_buffer().ptr_at(region_top + i))); + + if (count == region_size) { + for (u16 row = region_top; row <= region_bottom; ++row) + active_buffer()[row].clear(Attribute()); + return; + } + + active_buffer().remove(region_top, count); + for (size_t i = 0; i < count; ++i) + active_buffer().insert(region_bottom - count + i + 1, make(m_columns)); + for (u16 row = region_top; row <= region_bottom; ++row) + active_buffer()[row].set_dirty(true); + if (!m_use_alternate_screen_buffer && max_history_size() != 0) + m_client.terminal_history_changed(); +} + +// Insert `count` blank lines at the top of the region. Text moves down. Does not affect the scrollback buffer. +void Terminal::scroll_down(u16 region_top, u16 region_bottom, size_t count) +{ + VERIFY(region_top <= region_bottom); + VERIFY(region_bottom < rows()); + // Only the specified region should be affected. + size_t region_size = region_bottom - region_top + 1; + count = min(count, region_size); + dbgln_if(TERMINAL_DEBUG, "Scroll down {} lines in region {}-{}", count, region_top, region_bottom); + // NOTE: We have to invalidate the cursor first. + invalidate_cursor(); + + if (count == region_size) { + for (u16 row = region_top; row <= region_bottom; ++row) + active_buffer()[row].set_dirty(true); + return; + } + + active_buffer().remove(region_bottom - count + 1, count); + for (size_t i = 0; i < count; ++i) + active_buffer().insert(region_top, make(m_columns)); + for (u16 row = region_top; row <= region_bottom; ++row) + active_buffer()[row].set_dirty(true); } void Terminal::put_character_at(unsigned row, unsigned column, u32 code_point) diff --git a/Userland/Libraries/LibVT/Terminal.h b/Userland/Libraries/LibVT/Terminal.h index fdc5c0e6c1..5b34a960a2 100644 --- a/Userland/Libraries/LibVT/Terminal.h +++ b/Userland/Libraries/LibVT/Terminal.h @@ -171,6 +171,11 @@ public: return m_needs_bracketed_paste; }; + bool is_within_scroll_region(u16 line) const + { + return line >= m_scroll_region_top && line <= m_scroll_region_bottom; + } + protected: // ^EscapeSequenceExecutor virtual void emit_code_point(u32) override; @@ -199,18 +204,17 @@ protected: }; void carriage_return(); -#ifndef KERNEL - void scroll_up(); - void scroll_down(); + inline void scroll_up(size_t count = 1); + inline void scroll_down(size_t count = 1); void linefeed(); +#ifndef KERNEL + void scroll_up(u16 region_top, u16 region_bottom, size_t count); + void scroll_down(u16 region_top, u16 region_bottom, size_t count); void put_character_at(unsigned row, unsigned column, u32 ch); - void set_window_title(const String&); #else - virtual void scroll_up() = 0; - virtual void scroll_down() = 0; - virtual void linefeed() = 0; + virtual void scroll_up(u16 region_top, u16 region_bottom, size_t count) = 0; + virtual void scroll_down(u16 region_top, u16 region_bottom, size_t count) = 0; virtual void put_character_at(unsigned row, unsigned column, u32 ch) = 0; - virtual void set_window_title(const String&) = 0; #endif void unimplemented_control_code(u8); @@ -307,18 +311,18 @@ protected: // SD - Scroll Down (called "Pan Up" in VT510) void SD(Parameters); -#ifndef KERNEL // IL - Insert Line void IL(Parameters); + +#ifndef KERNEL // DCH - Delete Character void DCH(Parameters); +#else + virtual void DCH(Parameters) = 0; +#endif + // DL - Delete Line void DL(Parameters); -#else - virtual void IL(Parameters) = 0; - virtual void DCH(Parameters) = 0; - virtual void DL(Parameters) = 0; -#endif // CHA - Cursor Horizontal Absolute void CHA(Parameters);