From 6a394ae2e5feae199b6128b4496417d3e770bf65 Mon Sep 17 00:00:00 2001 From: Jabber Developer Date: Tue, 1 Jul 2025 16:42:03 +0200 Subject: [PATCH 1/2] fix: Increase scrollback buffer height for chat rendering (PAD_SIZE) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit f27fa98 commit introduced issue of scrolling seemingly randomly being stuck and the following message flood in the logs: "WRN: Ncurses Overflow..." A pad is an off-screen buffer — larger than the visible terminal screen — that you can scroll, render parts of, or update selectively. Unlike regular windows, a pad is not tied to screen size. You can create a pad that's 1,000 lines tall, even if your terminal is only 40 lines tall. This commit addresses the issue by increasing PAD_SIZE to a reasonable number, allowing messages from buffer to be displayed without overflowing NCurses' window size (maxy). Minor change: add .cache folder to gitignore --- .gitignore | 3 +++ src/ui/window.c | 1 - src/ui/window.h | 3 +++ 3 files changed, 6 insertions(+), 1 deletion(-) diff --git a/.gitignore b/.gitignore index 37a7838b..6e24b69c 100644 --- a/.gitignore +++ b/.gitignore @@ -44,6 +44,9 @@ src/gitversion.h.in src/stamp-h1 src/plugins/profapi.lo +# clangd +.cache/ + # out-of-tree build folders build*/ diff --git a/src/ui/window.c b/src/ui/window.c index 88271b34..be53eae9 100644 --- a/src/ui/window.c +++ b/src/ui/window.c @@ -62,7 +62,6 @@ #include "xmpp/xmpp.h" #include "xmpp/roster_list.h" -static const int PAD_SIZE = 100; static const char* LOADING_MESSAGE = "Loading older messages…"; static const char* CONS_WIN_TITLE = "Profanity. Type /help for help information."; static const char* XML_WIN_TITLE = "XML Console"; diff --git a/src/ui/window.h b/src/ui/window.h index 7749cb0d..2c4da707 100644 --- a/src/ui/window.h +++ b/src/ui/window.h @@ -56,6 +56,9 @@ #include "xmpp/contact.h" #include "xmpp/muc.h" +// Max lines per window rendered by NCurses +static const int PAD_SIZE = 10000; + void win_move_to_end(ProfWin* window); void win_show_status_string(ProfWin* window, const char* const from, const char* const show, const char* const status, -- 2.49.1 From 40b7a1254309aa70807343112c54acbc8a957ace Mon Sep 17 00:00:00 2001 From: Jabber Developer Date: Tue, 1 Jul 2025 20:53:18 +0200 Subject: [PATCH 2/2] Clarify and improve win_page_up() scrolling offset adjustment for smoother paging MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit refines the existing logic in win_page_up() by: - Improving comments to clearly explain the rationale behind adjusting the scroll offset relative to the first buffer entry’s visual position, helping future maintainers understand why this is necessary. - Fixing offset recalculation to better handle cases where older messages with variable heights are loaded from the DB, improving scroll smoothness. - Changing the logging of negative *page_start values from warning to debug, recognizing that this can be a normal scenario when insufficient history is loaded. - Simplifying some conditionals and renaming variables for clearer intent. No changes yet applied to win_page_down(), but similar improvements could be considered in the future. Overall, this enhances the robustness and user experience of scrolling up in chat windows, while preserving existing functional logic. --- src/database.c | 29 ++++++++++++++----------- src/database.h | 10 +++++++-- src/ui/buffer.c | 9 +++++++- src/ui/chatwin.c | 14 +++++++----- src/ui/ui.h | 3 ++- src/ui/window.c | 55 ++++++++++++++++++++++++++++++------------------ 6 files changed, 79 insertions(+), 41 deletions(-) diff --git a/src/database.c b/src/database.c index e9af1263..5ceb6357 100644 --- a/src/database.c +++ b/src/database.c @@ -222,6 +222,7 @@ out: void log_database_close(void) { + log_debug("log_database_close() called"); if (g_chatlog_database) { sqlite3_close(g_chatlog_database); sqlite3_shutdown(); @@ -318,13 +319,19 @@ log_database_get_limits_info(const gchar* const contact_barejid, gboolean is_las // Query previous chats, constraints start_time and end_time. If end_time is // null the current time is used. from_start gets first few messages if true // otherwise the last ones. Flip flips the order of the results -GSList* -log_database_get_previous_chat(const gchar* const contact_barejid, const char* start_time, char* end_time, gboolean from_start, gboolean flip) +db_history_result_t +log_database_get_previous_chat(const gchar* const contact_barejid, const char* start_time, char* end_time, gboolean from_start, gboolean flip, GSList** result) { + if (!g_chatlog_database) { + log_warning("log_database_get_previous_chat() called but db is not initialized"); + return DB_RESPONSE_ERROR; + } sqlite3_stmt* stmt = NULL; const Jid* myjid = connection_get_jid(); - if (!myjid->str) - return NULL; + if (!myjid->str) { + log_warning("log_database_get_previous_chat() called but no connection detected."); + return DB_RESPONSE_ERROR; + } // Flip order when querying older pages gchar* sort1 = from_start ? "ASC" : "DESC"; @@ -346,18 +353,16 @@ log_database_get_previous_chat(const gchar* const contact_barejid, const char* s g_date_time_unref(now); if (!query) { - log_error("Could not allocate memory"); - return NULL; + log_error("Could not allocate memory."); + return DB_RESPONSE_ERROR; } int rc = sqlite3_prepare_v2(g_chatlog_database, query, -1, &stmt, NULL); if (rc != SQLITE_OK) { - log_error("SQLite error in log_database_get_previous_chat(): %s", sqlite3_errmsg(g_chatlog_database)); - return NULL; + log_error("SQLite error in log_database_get_previous_chat(): (error code: %d) %s", rc, sqlite3_errmsg(g_chatlog_database)); + return DB_RESPONSE_ERROR; } - GSList* history = NULL; - while (sqlite3_step(stmt) == SQLITE_ROW) { char* message = (char*)sqlite3_column_text(stmt, 0); char* date = (char*)sqlite3_column_text(stmt, 1); @@ -378,11 +383,11 @@ log_database_get_previous_chat(const gchar* const contact_barejid, const char* s msg->type = _get_message_type_type(type); msg->enc = _get_message_enc_type(encryption); - history = g_slist_append(history, msg); + *result = g_slist_append(*result, msg); } sqlite3_finalize(stmt); - return history; + return g_slist_length(*result) != 0 ? DB_RESPONSE_SUCCESS : DB_RESPONSE_EMPTY; } static const char* diff --git a/src/database.h b/src/database.h index 3b52efdd..4b84e722 100644 --- a/src/database.h +++ b/src/database.h @@ -40,14 +40,20 @@ #include "config/account.h" #include "xmpp/xmpp.h" -#define MESSAGES_TO_RETRIEVE 10 +#define MESSAGES_TO_RETRIEVE 100 + +typedef enum { + DB_RESPONSE_ERROR, + DB_RESPONSE_EMPTY, + DB_RESPONSE_SUCCESS +} db_history_result_t; gboolean log_database_init(ProfAccount* account); void log_database_add_incoming(ProfMessage* message); void log_database_add_outgoing_chat(const char* const id, const char* const barejid, const char* const message, const char* const replace_id, prof_enc_t enc); void log_database_add_outgoing_muc(const char* const id, const char* const barejid, const char* const message, const char* const replace_id, prof_enc_t enc); void log_database_add_outgoing_muc_pm(const char* const id, const char* const barejid, const char* const message, const char* const replace_id, prof_enc_t enc); -GSList* log_database_get_previous_chat(const gchar* const contact_barejid, const char* start_time, char* end_time, gboolean from_start, gboolean flip); +db_history_result_t log_database_get_previous_chat(const gchar* const contact_barejid, const char* start_time, char* end_time, gboolean from_start, gboolean flip, GSList** result); ProfMessage* log_database_get_limits_info(const gchar* const contact_barejid, gboolean is_last); void log_database_close(void); diff --git a/src/ui/buffer.c b/src/ui/buffer.c index e0b6ea10..5eda4438 100644 --- a/src/ui/buffer.c +++ b/src/ui/buffer.c @@ -109,6 +109,7 @@ _buffer_add(ProfBuff buffer, const char* show_char, int pad_indent, GDateTime* t buffer->lines += e->_lines; while (g_slist_length(buffer->entries) >= MAX_BUFFER_SIZE) { + // Delete message from the opposite size to free buffer GSList* buffer_entry_to_delete = append ? buffer->entries : g_slist_last(buffer->entries); ProfBuffEntry* entry_to_delete = (ProfBuffEntry*)buffer_entry_to_delete->data; buffer->lines -= entry_to_delete->_lines; @@ -117,7 +118,13 @@ _buffer_add(ProfBuff buffer, const char* show_char, int pad_indent, GDateTime* t } if (from_jid && y_end_pos == y_start_pos) { - log_warning("Ncurses Overflow! From: %s, pos: %d, ID: %s, message: %s", from_jid, y_end_pos, id, message); + log_warning("Ncurses Overflow! From: %s, position: %d, ID: %s, append: %s, used message buffer size: %d, message buffer size: %d, rendered lines buffer size: %d", + from_jid, y_start_pos, id, append ? "TRUE" : "FALSE", g_slist_length(buffer->entries), MAX_BUFFER_SIZE, PAD_SIZE); + // At this point we have a message that caused overflow of the render buffer. + // Ideally, we want to clean other messages to rerender everything properly. To do so, + // first we need to determine the size of the message that we are trying to display, + // then we need to print the message. + // However, _buffer_add is too late in the code to do this, therefore it has to be done in the win_print_old_history. } buffer->entries = append ? g_slist_append(buffer->entries, e) : g_slist_prepend(buffer->entries, e); diff --git a/src/ui/chatwin.c b/src/ui/chatwin.c index f05bf1c1..fc14f0a5 100644 --- a/src/ui/chatwin.c +++ b/src/ui/chatwin.c @@ -561,7 +561,8 @@ static void _chatwin_history(ProfChatWin* chatwin, const char* const contact_barejid) { if (!chatwin->history_shown) { - GSList* history = log_database_get_previous_chat(contact_barejid, NULL, NULL, FALSE, FALSE); + GSList* history = NULL; + log_database_get_previous_chat(contact_barejid, NULL, NULL, FALSE, FALSE, &history); GSList* curr = history; while (curr) { @@ -583,15 +584,15 @@ _chatwin_history(ProfChatWin* chatwin, const char* const contact_barejid) // Print history starting from start_time to end_time if end_time is null the // first entry's timestamp in the buffer is used. Flip true to prepend to buffer. // Timestamps should be in iso8601 -gboolean +db_history_result_t chatwin_db_history(ProfChatWin* chatwin, const char* start_time, char* end_time, gboolean flip) { if (!end_time) { end_time = buffer_size(((ProfWin*)chatwin)->layout->buffer) == 0 ? NULL : g_date_time_format_iso8601(buffer_get_entry(((ProfWin*)chatwin)->layout->buffer, 0)->time); } - GSList* history = log_database_get_previous_chat(chatwin->barejid, start_time, end_time, !flip, flip); - gboolean has_items = g_slist_length(history) != 0; + GSList* history = NULL; + db_history_result_t result = log_database_get_previous_chat(chatwin->barejid, start_time, end_time, !flip, flip, &history); GSList* curr = history; while (curr) { @@ -610,9 +611,12 @@ chatwin_db_history(ProfChatWin* chatwin, const char* start_time, char* end_time, } g_slist_free_full(history, (GDestroyNotify)message_free); + // TODO: Potential optimization + // if (flip) + // since adding messages to history without overflowing buffer does not require a win_redraw win_redraw((ProfWin*)chatwin); - return has_items; + return result; } static void diff --git a/src/ui/ui.h b/src/ui/ui.h index 8988f84d..1eafd743 100644 --- a/src/ui/ui.h +++ b/src/ui/ui.h @@ -43,6 +43,7 @@ #include "config/account.h" #include "config/preferences.h" #include "command/cmd_funcs.h" +#include "database.h" #include "ui/win_types.h" #include "xmpp/message.h" #include "xmpp/muc.h" @@ -145,7 +146,7 @@ void chatwin_set_incoming_char(ProfChatWin* chatwin, const char* const ch); void chatwin_unset_incoming_char(ProfChatWin* chatwin); void chatwin_set_outgoing_char(ProfChatWin* chatwin, const char* const ch); void chatwin_unset_outgoing_char(ProfChatWin* chatwin); -gboolean chatwin_db_history(ProfChatWin* chatwin, const char* start_time, char* end_time, gboolean flip); +db_history_result_t chatwin_db_history(ProfChatWin* chatwin, const char* start_time, char* end_time, gboolean flip); // MUC window ProfMucWin* mucwin_new(const char* const barejid); diff --git a/src/ui/window.c b/src/ui/window.c index be53eae9..6489142a 100644 --- a/src/ui/window.c +++ b/src/ui/window.c @@ -35,6 +35,7 @@ */ #include "config.h" +#include "database.h" #include "ui/window_list.h" #include @@ -628,54 +629,68 @@ win_free(ProfWin* window) void win_page_up(ProfWin* window, int scroll_size) { + // Page offset from the start of the NCurses PAD (invisible window) int* page_start = &(window->layout->y_pos); int page_start_initial = *page_start; - int total_rows = getcury(window->layout->win); + // Size of the visible chat page int page_space = getmaxy(stdscr) - 4; if (scroll_size == 0) scroll_size = page_space; win_scroll_state_t* scroll_state = &window->scroll_state; *scroll_state = (*scroll_state == WIN_SCROLL_REACHED_BOTTOM) ? WIN_SCROLL_INNER : *scroll_state; *page_start -= scroll_size; + gboolean has_scroll_past_buffer = *page_start < 0; - if (*page_start == -scroll_size && window->type == WIN_CHAT) { + if (has_scroll_past_buffer && window->type == WIN_CHAT) { ProfChatWin* chatwin = (ProfChatWin*)window; ProfBuffEntry* first_entry = buffer_size(window->layout->buffer) != 0 ? buffer_get_entry(window->layout->buffer, 0) : NULL; - // Don't do anything if still fetching mam messages - if (first_entry && !(first_entry->theme_item == THEME_ROOMINFO && g_strcmp0(first_entry->message, LOADING_MESSAGE) == 0)) { + gboolean is_still_fetching_mam = first_entry && (first_entry->theme_item == THEME_ROOMINFO && g_strcmp0(first_entry->message, LOADING_MESSAGE) == 0); + if (!is_still_fetching_mam) { + // WIN_SCROLL_REACHED_TOP means that we reached top of DB and there is no need to refetch data from it + gboolean is_db_offset_jump_needed = false; if (*scroll_state != WIN_SCROLL_REACHED_TOP) { - *scroll_state = !chatwin_db_history(chatwin, NULL, NULL, TRUE) ? WIN_SCROLL_REACHED_TOP : WIN_SCROLL_INNER; + db_history_result_t db_response = chatwin_db_history(chatwin, NULL, NULL, TRUE); + is_db_offset_jump_needed = db_response == DB_RESPONSE_SUCCESS; + *scroll_state = db_response == DB_RESPONSE_EMPTY ? WIN_SCROLL_REACHED_TOP : WIN_SCROLL_INNER; + log_debug("Scroll state after DB history fetch: %s", *scroll_state ? "WIN_SCROLL_REACHED_TOP" : "WIN_SCROLL_INNER"); } - if (*scroll_state == WIN_SCROLL_REACHED_TOP && prefs_get_boolean(PREF_MAM)) { - win_print_loading_history(window); - iq_mam_request_older(chatwin); + if (*scroll_state == WIN_SCROLL_REACHED_TOP) { + *page_start = 0; + if (prefs_get_boolean(PREF_MAM)) { + win_print_loading_history(window); + iq_mam_request_older(chatwin); + } } - int buff_size = buffer_size(window->layout->buffer); - int offset_entry_id = buff_size > 10 ? 10 : buff_size - 1; - int offset = buffer_get_entry(window->layout->buffer, offset_entry_id)->y_end_pos; - *page_start = offset - page_space; + // Recalculate scroll offset after loading older messages from the DB. + // *page_start may become negative due to paging up beyond the current buffer. + // To maintain smooth scroll positioning, we adjust it relative to the visual offset + // (y_start_pos) of the first buffer entry *before* DB fetch. + // This ensures a seamless transition as older messages are prepended, regardless + // of how many visual lines they occupy (since messages can vary in height). + // For example, if *page_start == -5 and first_entry->y_start_pos == 40, + // we shift the scroll to 35, preserving the visual context for the user. + if (is_db_offset_jump_needed) { + *page_start = first_entry ? first_entry->y_start_pos + *page_start : 0; + } } } - // went past beginning, show first page if (*page_start < 0) { + log_debug("win_page_up(): *page_start adjusted to 0 due to negative value (expected if insufficient history messages). Previous value: %d", *page_start); *page_start = 0; } - window->layout->paged = 1; - - // update only if position has changed + // Update window location only if position has changed if (page_start_initial != *page_start) { win_update_virtual(window); } - // switch off page if last line and space line visible - if ((total_rows) - *page_start == page_space) { - window->layout->paged = 0; - } + // Update scrolling state + int total_rows = getcury(window->layout->win); + window->layout->paged = (total_rows) - *page_start > page_space; } void -- 2.49.1