Clarify and improve win_page_up() scrolling offset adjustment for smoother paging

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.
This commit is contained in:
2025-07-01 20:53:18 +02:00
parent 6a394ae2e5
commit 40b7a12543
6 changed files with 79 additions and 41 deletions

View File

@@ -222,6 +222,7 @@ out:
void void
log_database_close(void) log_database_close(void)
{ {
log_debug("log_database_close() called");
if (g_chatlog_database) { if (g_chatlog_database) {
sqlite3_close(g_chatlog_database); sqlite3_close(g_chatlog_database);
sqlite3_shutdown(); 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 // 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 // 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 // otherwise the last ones. Flip flips the order of the results
GSList* 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) 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; sqlite3_stmt* stmt = NULL;
const Jid* myjid = connection_get_jid(); const Jid* myjid = connection_get_jid();
if (!myjid->str) if (!myjid->str) {
return NULL; log_warning("log_database_get_previous_chat() called but no connection detected.");
return DB_RESPONSE_ERROR;
}
// Flip order when querying older pages // Flip order when querying older pages
gchar* sort1 = from_start ? "ASC" : "DESC"; 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); g_date_time_unref(now);
if (!query) { if (!query) {
log_error("Could not allocate memory"); log_error("Could not allocate memory.");
return NULL; return DB_RESPONSE_ERROR;
} }
int rc = sqlite3_prepare_v2(g_chatlog_database, query, -1, &stmt, NULL); int rc = sqlite3_prepare_v2(g_chatlog_database, query, -1, &stmt, NULL);
if (rc != SQLITE_OK) { if (rc != SQLITE_OK) {
log_error("SQLite error in log_database_get_previous_chat(): %s", sqlite3_errmsg(g_chatlog_database)); log_error("SQLite error in log_database_get_previous_chat(): (error code: %d) %s", rc, sqlite3_errmsg(g_chatlog_database));
return NULL; return DB_RESPONSE_ERROR;
} }
GSList* history = NULL;
while (sqlite3_step(stmt) == SQLITE_ROW) { while (sqlite3_step(stmt) == SQLITE_ROW) {
char* message = (char*)sqlite3_column_text(stmt, 0); char* message = (char*)sqlite3_column_text(stmt, 0);
char* date = (char*)sqlite3_column_text(stmt, 1); 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->type = _get_message_type_type(type);
msg->enc = _get_message_enc_type(encryption); msg->enc = _get_message_enc_type(encryption);
history = g_slist_append(history, msg); *result = g_slist_append(*result, msg);
} }
sqlite3_finalize(stmt); sqlite3_finalize(stmt);
return history; return g_slist_length(*result) != 0 ? DB_RESPONSE_SUCCESS : DB_RESPONSE_EMPTY;
} }
static const char* static const char*

View File

@@ -40,14 +40,20 @@
#include "config/account.h" #include "config/account.h"
#include "xmpp/xmpp.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); gboolean log_database_init(ProfAccount* account);
void log_database_add_incoming(ProfMessage* message); 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_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(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); 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); ProfMessage* log_database_get_limits_info(const gchar* const contact_barejid, gboolean is_last);
void log_database_close(void); void log_database_close(void);

View File

@@ -109,6 +109,7 @@ _buffer_add(ProfBuff buffer, const char* show_char, int pad_indent, GDateTime* t
buffer->lines += e->_lines; buffer->lines += e->_lines;
while (g_slist_length(buffer->entries) >= MAX_BUFFER_SIZE) { 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); GSList* buffer_entry_to_delete = append ? buffer->entries : g_slist_last(buffer->entries);
ProfBuffEntry* entry_to_delete = (ProfBuffEntry*)buffer_entry_to_delete->data; ProfBuffEntry* entry_to_delete = (ProfBuffEntry*)buffer_entry_to_delete->data;
buffer->lines -= entry_to_delete->_lines; 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) { 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); buffer->entries = append ? g_slist_append(buffer->entries, e) : g_slist_prepend(buffer->entries, e);

View File

@@ -561,7 +561,8 @@ static void
_chatwin_history(ProfChatWin* chatwin, const char* const contact_barejid) _chatwin_history(ProfChatWin* chatwin, const char* const contact_barejid)
{ {
if (!chatwin->history_shown) { 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; GSList* curr = history;
while (curr) { 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 // 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. // first entry's timestamp in the buffer is used. Flip true to prepend to buffer.
// Timestamps should be in iso8601 // Timestamps should be in iso8601
gboolean db_history_result_t
chatwin_db_history(ProfChatWin* chatwin, const char* start_time, char* end_time, gboolean flip) chatwin_db_history(ProfChatWin* chatwin, const char* start_time, char* end_time, gboolean flip)
{ {
if (!end_time) { 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); 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); GSList* history = NULL;
gboolean has_items = g_slist_length(history) != 0; db_history_result_t result = log_database_get_previous_chat(chatwin->barejid, start_time, end_time, !flip, flip, &history);
GSList* curr = history; GSList* curr = history;
while (curr) { 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); 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); win_redraw((ProfWin*)chatwin);
return has_items; return result;
} }
static void static void

View File

@@ -43,6 +43,7 @@
#include "config/account.h" #include "config/account.h"
#include "config/preferences.h" #include "config/preferences.h"
#include "command/cmd_funcs.h" #include "command/cmd_funcs.h"
#include "database.h"
#include "ui/win_types.h" #include "ui/win_types.h"
#include "xmpp/message.h" #include "xmpp/message.h"
#include "xmpp/muc.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_unset_incoming_char(ProfChatWin* chatwin);
void chatwin_set_outgoing_char(ProfChatWin* chatwin, const char* const ch); void chatwin_set_outgoing_char(ProfChatWin* chatwin, const char* const ch);
void chatwin_unset_outgoing_char(ProfChatWin* chatwin); 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 // MUC window
ProfMucWin* mucwin_new(const char* const barejid); ProfMucWin* mucwin_new(const char* const barejid);

View File

@@ -35,6 +35,7 @@
*/ */
#include "config.h" #include "config.h"
#include "database.h"
#include "ui/window_list.h" #include "ui/window_list.h"
#include <stdlib.h> #include <stdlib.h>
@@ -628,54 +629,68 @@ win_free(ProfWin* window)
void void
win_page_up(ProfWin* window, int scroll_size) 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 = &(window->layout->y_pos);
int page_start_initial = *page_start; 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; int page_space = getmaxy(stdscr) - 4;
if (scroll_size == 0) if (scroll_size == 0)
scroll_size = page_space; scroll_size = page_space;
win_scroll_state_t* scroll_state = &window->scroll_state; win_scroll_state_t* scroll_state = &window->scroll_state;
*scroll_state = (*scroll_state == WIN_SCROLL_REACHED_BOTTOM) ? WIN_SCROLL_INNER : *scroll_state; *scroll_state = (*scroll_state == WIN_SCROLL_REACHED_BOTTOM) ? WIN_SCROLL_INNER : *scroll_state;
*page_start -= scroll_size; *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; ProfChatWin* chatwin = (ProfChatWin*)window;
ProfBuffEntry* first_entry = buffer_size(window->layout->buffer) != 0 ? buffer_get_entry(window->layout->buffer, 0) : NULL; 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 gboolean is_still_fetching_mam = first_entry && (first_entry->theme_item == THEME_ROOMINFO && g_strcmp0(first_entry->message, LOADING_MESSAGE) == 0);
if (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) { 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)) { if (*scroll_state == WIN_SCROLL_REACHED_TOP) {
win_print_loading_history(window); *page_start = 0;
iq_mam_request_older(chatwin); if (prefs_get_boolean(PREF_MAM)) {
win_print_loading_history(window);
iq_mam_request_older(chatwin);
}
} }
int buff_size = buffer_size(window->layout->buffer); // Recalculate scroll offset after loading older messages from the DB.
int offset_entry_id = buff_size > 10 ? 10 : buff_size - 1; // *page_start may become negative due to paging up beyond the current buffer.
int offset = buffer_get_entry(window->layout->buffer, offset_entry_id)->y_end_pos; // To maintain smooth scroll positioning, we adjust it relative to the visual offset
*page_start = offset - page_space; // (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) { 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; *page_start = 0;
} }
window->layout->paged = 1; // Update window location only if position has changed
// update only if position has changed
if (page_start_initial != *page_start) { if (page_start_initial != *page_start) {
win_update_virtual(window); win_update_virtual(window);
} }
// switch off page if last line and space line visible // Update scrolling state
if ((total_rows) - *page_start == page_space) { int total_rows = getcury(window->layout->win);
window->layout->paged = 0; window->layout->paged = (total_rows) - *page_start > page_space;
}
} }
void void