From 5f192d5dc7c1cb090a05ef23627fb988a3537660 Mon Sep 17 00:00:00 2001
From: SparkyTD <45818400+SparkyTD@users.noreply.github.com>
Date: Fri, 10 May 2024 12:20:10 -0700
Subject: [PATCH] impr: Don't move hex editor scroll position when jumping to
 address that's on-screen (#1660)

### Problem description
This PR offers two improvements:
1) When selecting / jumping to an offset that falls within the current
viewport of the scroll view, the scroll offset will no longer force the
selected byte to the top of the view. Instead, the scroll offset will
only be changed if the selected byte is outside the current view.

2) In case a wrong offset is entered into the Select or Goto dialog
(e.g. and offset beyond EoF), the dialog's button will be disabled.

### Implementation description
For the first change, I modified the logic that recalculates the
`m_scrollPosition ` based on the current byte offset.

For the second change, I added validation logic to both popups to ensure
that the entered offsets are valid (using `provider->getActualSize()`).
In case of the Select popup, I wrapped the button into an
`ImGui::Begin/EndDisabled` to enforce the validation check.
---
 .../source/content/views/view_hex_editor.cpp  | 20 ++++++++++++++-----
 plugins/ui/source/ui/hex_editor.cpp           | 15 ++++++++++++--
 2 files changed, 28 insertions(+), 7 deletions(-)

diff --git a/plugins/builtin/source/content/views/view_hex_editor.cpp b/plugins/builtin/source/content/views/view_hex_editor.cpp
index 6394e6c6e..11d8c1c74 100644
--- a/plugins/builtin/source/content/views/view_hex_editor.cpp
+++ b/plugins/builtin/source/content/views/view_hex_editor.cpp
@@ -88,12 +88,14 @@ namespace hex::plugin::builtin {
                     }
                 }
 
+                bool isOffsetValid = m_newAddress <= ImHexApi::Provider::get()->getActualSize();
+
                 bool executeGoto = false;
-                if (ImGui::IsItemFocused() && ImGui::IsKeyPressed(ImGuiKey_Enter)) {
+                if (isOffsetValid && ImGui::IsItemFocused() && ImGui::IsKeyPressed(ImGuiKey_Enter)) {
                     executeGoto = true;
                 }
 
-                ImGui::BeginDisabled(!m_newAddress.has_value());
+                ImGui::BeginDisabled(!m_newAddress.has_value() || !isOffsetValid);
                 {
                     const auto label = hex::format("{} {}", "hex.builtin.view.hex_editor.menu.file.goto"_lang, m_newAddress.has_value() ? hex::format("0x{:08X}", *m_newAddress) : "???");
                     const auto buttonWidth = ImGui::GetWindowWidth() - ImGui::GetStyle().WindowPadding.x * 2;
@@ -163,10 +165,18 @@ namespace hex::plugin::builtin {
                     ImGui::EndTabItem();
                 }
 
-                if (ImGui::Button("hex.builtin.view.hex_editor.select.select"_lang) || (ImGui::IsItemFocused() && (ImGui::IsKeyPressed(ImGuiKey_Enter) || ImGui::IsKeyPressed(ImGuiKey_Enter)))) {
-                    editor->setSelection(m_region.getStartAddress(), m_region.getEndAddress());
-                    editor->jumpToSelection();
+                const auto provider = ImHexApi::Provider::get();
+                bool isOffsetValid = m_region.getStartAddress() <= m_region.getEndAddress() &&
+                        m_region.getEndAddress() < provider->getActualSize();
+
+                ImGui::BeginDisabled(!isOffsetValid);
+                {
+                    if (ImGui::Button("hex.builtin.view.hex_editor.select.select"_lang) || (ImGui::IsItemFocused() && (ImGui::IsKeyPressed(ImGuiKey_Enter) || ImGui::IsKeyPressed(ImGuiKey_Enter)))) {
+                        editor->setSelection(m_region.getStartAddress(), m_region.getEndAddress());
+                        editor->jumpToSelection();
+                    }
                 }
+                ImGui::EndDisabled();
 
                 ImGui::EndTabBar();
             }
diff --git a/plugins/ui/source/ui/hex_editor.cpp b/plugins/ui/source/ui/hex_editor.cpp
index f67692365..c619d7718 100644
--- a/plugins/ui/source/ui/hex_editor.cpp
+++ b/plugins/ui/source/ui/hex_editor.cpp
@@ -837,9 +837,20 @@ namespace hex::ui {
                         m_provider->setCurrentPage(m_provider->getPageOfAddress(newSelection.address).value_or(0));
 
                         const auto pageAddress = m_provider->getCurrentPageAddress() + m_provider->getBaseAddress();
+                        const auto targetRowNumber = (newSelection.getStartAddress() - pageAddress) / m_bytesPerRow;
 
-                        m_scrollPosition = (newSelection.getStartAddress() - pageAddress) / m_bytesPerRow;
-                        m_scrollPosition -= m_visibleRowCount * m_jumpPivot;
+                        // Calculate the current top and bottom row numbers of the viewport
+                        ImS64 currentTopRow = m_scrollPosition;
+                        ImS64 currentBottomRow = m_scrollPosition + m_visibleRowCount - 3;
+
+                        // Check if the targetRowNumber is outside the current visible range
+                        if (ImS64(targetRowNumber) < currentTopRow) {
+                            // If target is above the current view, scroll just enough to bring it into view at the top
+                            m_scrollPosition = targetRowNumber - m_visibleRowCount * m_jumpPivot;
+                        } else if (ImS64(targetRowNumber) > currentBottomRow) {
+                            // If target is below the current view, scroll just enough to bring it into view at the bottom
+                            m_scrollPosition = targetRowNumber - (m_visibleRowCount - 3);
+                        }
 
                         m_jumpPivot = 0.0F;
                     }