From 94e298394c2cb7f310594103c9a02f310924c537 Mon Sep 17 00:00:00 2001 From: OmniaX-Dev Date: Fri, 24 Apr 2026 03:01:43 +0200 Subject: [PATCH] Fixed a ListView dangling pointer bug --- .clangd | 1 + src/ogfx/gui/widgets/ListView.cpp | 75 +++++++++++++++++-------------- src/ogfx/gui/widgets/ListView.hpp | 14 ++++-- src/ogfx/gui/widgets/Slider.cpp | 24 +++++++++- src/ogfx/gui/widgets/Slider.hpp | 3 ++ src/test/GuiTest.cpp | 8 ++-- 6 files changed, 84 insertions(+), 41 deletions(-) diff --git a/.clangd b/.clangd index e6defe1..c60968a 100644 --- a/.clangd +++ b/.clangd @@ -1,4 +1,5 @@ CompileFlags: CompilationDatabase: bin + Add: ["--target=x86_64-w64-mingw32"] Diagnostics: MissingIncludes: None \ No newline at end of file diff --git a/src/ogfx/gui/widgets/ListView.cpp b/src/ogfx/gui/widgets/ListView.cpp index 01ac34f..f13647a 100644 --- a/src/ogfx/gui/widgets/ListView.cpp +++ b/src/ogfx/gui/widgets/ListView.cpp @@ -35,6 +35,7 @@ namespace ogfx return; m_text = text; update_dimensions(); + if (m_parent) m_parent->m_extentsDirty = true; } void ListView::Item::setFontSize(i32 fontSize) @@ -43,6 +44,7 @@ namespace ogfx return; m_fontSize = fontSize; update_dimensions(); + if (m_parent) m_parent->m_extentsDirty = true; } void ListView::Item::update_dimensions(void) @@ -108,35 +110,31 @@ namespace ogfx void ListView::onDraw(ogfx::BasicRenderer2D& gfx) { - // f32 y = 0; const auto& bounds = getGlobalBounds(); const auto& content = getContentExtents(); + const f32 scrollY = -getScrollOffset().y; + const f32 visibleH = getContentBounds().h; + const f32 visibleStart = scrollY; + const f32 visibleEnd = scrollY + visibleH; Color textColor; - // Compute the visible Y window in content space - const f32 scrollY = -getScrollOffset().y; // positive offset into content - const f32 visibleH = getContentBounds().h; - const f32 visibleStart = scrollY; - const f32 visibleEnd = scrollY + visibleH; - // Find the starting item and its Y via a linear scan... f32 y = 0; i32 startIdx = 0; for (auto& item : m_list) { if (!item.isValid()) { startIdx++; continue; } f32 itemH = item.getDimensions().y; - if (y + itemH > visibleStart) break; // this item is the first one in view + if (y + itemH > visibleStart) break; y += itemH; startIdx++; } - // ...then process only items from startIdx until we exceed visibleEnd for (i32 i = startIdx; i < (i32)m_list.size(); i++) { auto& item = m_list[i]; if (!item.isValid()) continue; f32 itemH = item.getDimensions().y; - if (y > visibleEnd) break; // past the visible window, stop + if (y > visibleEnd) break; if (item.isSelected()) { textColor = item.getSelectedTextColor(); @@ -146,24 +144,8 @@ namespace ogfx textColor = item.getTextColor(); gfx.drawLine({ Vec2 { bounds.x, bounds.y + y + itemH + 4 } + getScrollOffset(), Vec2 { bounds.x + content.w, bounds.y + y + itemH + 4 } + getScrollOffset() }, Colors::Black, 1); gfx.drawString(item, bounds.getPosition() + Vec2 { 0, y } + getScrollOffset() + item.getPadding().getPosition(), textColor, item.getFontSize()); - // draw or hit-test item at content Y = y y += itemH; } - // for (auto& item : m_list) - // { - // if (!item.isValid()) continue; - // const auto& size = item.getDimensions(); - // if (item.isSelected()) - // { - // textColor = item.getSelectedTextColor(); - // gfx.fillRect({ Vec2 { bounds.x, bounds.y + y + 4 } + getScrollOffset(), { content.w, size.y } }, item.getSelectedColor()); - // } - // else - // textColor = item.getTextColor(); - // gfx.drawLine({ Vec2 { bounds.x, bounds.y + y + size.y + 4 } + getScrollOffset(), Vec2 { bounds.x + content.w, bounds.y + y + size.y + 4 } + getScrollOffset() }, Colors::Black, 1); - // gfx.drawString(item, bounds.getPosition() + Vec2 { 0, y } + getScrollOffset() + item.getPadding().getPosition(), textColor, item.getFontSize()); - // y += size.y; - // } } void ListView::afterDraw(ogfx::BasicRenderer2D& gfx) @@ -188,10 +170,11 @@ namespace ogfx { if (!item.isValid()) continue; const f32 itemH = item.getDimensions().y; - if (!item.isSelected() && localY >= y && localY < y + itemH) + if (localY >= y && localY < y + itemH) { + bool wasSelected = item.isSelected(); item.set_selected(m_selectedList); - if (callback_onSelectionChanged) + if (!wasSelected && callback_onSelectionChanged) callback_onSelectionChanged(m_selectedList); event.handle(); break; @@ -202,6 +185,9 @@ namespace ogfx Rectangle ListView::getContentExtents(void) const { + + if (!m_extentsDirty) + return m_cachedExtents; f32 maxX = 0, maxY = 0; for (auto& item : m_list) { @@ -210,10 +196,12 @@ namespace ogfx maxX = std::max(maxX, size.x); maxY += size.y; } - return { 0, 0, maxX, maxY }; + m_cachedExtents = { 0, 0, maxX, maxY }; + m_extentsDirty = false; + return m_cachedExtents; } - ListView::Item& ListView::getItem(const String& text) + ListView::Item& ListView::getLine(const String& text) { for (auto& item : m_list) if (item.getText() == text) @@ -221,7 +209,7 @@ namespace ogfx return InvalidItem; } - ListView::Item& ListView::getItem(u32 index) + ListView::Item& ListView::getLine(u32 index) { if (index < m_list.size()) return m_list[index]; @@ -238,10 +226,31 @@ namespace ogfx item.setSelectedTextColor(m_defaultSelectionTextColor); item.setText(text); m_list.push_back(item); - if (m_list.size() == 1) - m_list.back().set_selected(m_selectedList); + if (m_list.size() > 0) + m_list[0].set_selected(m_selectedList); + m_extentsDirty = true; return m_list.back(); } + + bool ListView::removeLine(const String& text) + { + return false; //TODO: Implement + } + + bool ListView::removeLine(u32 index) + { + return false; //TODO: Implement + } + + bool ListView::hasLine(const String& text) + { + return false; //TODO: Implement + } + + bool ListView::hasLine(u32 index) + { + return false; //TODO: Implement + } } } } diff --git a/src/ogfx/gui/widgets/ListView.hpp b/src/ogfx/gui/widgets/ListView.hpp index 5af48c0..9158aa6 100644 --- a/src/ogfx/gui/widgets/ListView.hpp +++ b/src/ogfx/gui/widgets/ListView.hpp @@ -22,6 +22,7 @@ #include #include +#include namespace ogfx { @@ -78,9 +79,13 @@ namespace ogfx void afterDraw(ogfx::BasicRenderer2D& gfx) override; void onMouseReleased(const Event& event) override; Rectangle getContentExtents(void) const override; - Item& getItem(const String& text); - Item& getItem(u32 index); + Item& getLine(const String& text); + Item& getLine(u32 index); Item& addLine(const String& text); + bool removeLine(const String& text); + bool removeLine(u32 index); + bool hasLine(const String& text); + bool hasLine(u32 index); inline void addLine(const Item& line) { m_list.push_back(line); if (m_list.size() == 1) m_list.back().set_selected(m_selectedList); } inline stdvec& getSelection(void) { return m_selectedList; } inline void setSelectionChangedCallback(std::function& selection)> callback) { callback_onSelectionChanged = std::move(callback); } @@ -95,9 +100,12 @@ namespace ogfx private: Item InvalidItem { *this }; - stdvec m_list; + std::deque m_list; stdvec m_selectedList; std::function& selection)> callback_onSelectionChanged { nullptr }; + mutable Rectangle m_cachedExtents { 0, 0, 0, 0 }; + mutable bool m_extentsDirty { true }; + Color m_lineColor { 40, 40, 40 }; bool m_showLine { true }; i32 m_defaultLinefontSize { 16 }; diff --git a/src/ogfx/gui/widgets/Slider.cpp b/src/ogfx/gui/widgets/Slider.cpp index 33e6572..89145b9 100644 --- a/src/ogfx/gui/widgets/Slider.cpp +++ b/src/ogfx/gui/widgets/Slider.cpp @@ -74,6 +74,28 @@ namespace ogfx set_value(mouse_position_to_value({ event.mouse->position_x, event.mouse->position_y })); } + void Slider::onMouseScrolled(const Event& event) + { + if (!isMouseInside()) + return; + if (!isScrollWheelShortcutEnabled()) + return; + if (getStep() > 0) + { + if (event.mouse->scroll == MouseEventData::eScrollDirection::Down || + event.mouse->scroll == MouseEventData::eScrollDirection::Right) + set_value(getValue() + (getStep() * -1)); + else + set_value(getValue() + getStep()); + } + else if (event.mouse->scroll == MouseEventData::eScrollDirection::Down || + event.mouse->scroll == MouseEventData::eScrollDirection::Up) + set_value(getValue() + (0.1f * event.mouse->scrollAmount.y)); + else + set_value(getValue() + (0.1f * event.mouse->scrollAmount.x)); + event.handle(); + } + void Slider::onDraw(ogfx::BasicRenderer2D& gfx) { const auto& bounds = getGlobalContentBounds(); @@ -143,7 +165,7 @@ namespace ogfx f32 Slider::snap_to_step(f32 val) { - if (m_step <= 0.0f) return val; // full precision + if (m_step <= 0.0f) return std::clamp(val, m_min, m_max); // full precision f32 snapped = std::round((val - m_min) / m_step) * m_step + m_min; return std::clamp(snapped, m_min, m_max); } diff --git a/src/ogfx/gui/widgets/Slider.hpp b/src/ogfx/gui/widgets/Slider.hpp index bc75179..e466e45 100644 --- a/src/ogfx/gui/widgets/Slider.hpp +++ b/src/ogfx/gui/widgets/Slider.hpp @@ -36,6 +36,7 @@ namespace ogfx void applyTheme(const ostd::Stylesheet& theme) override; void onMouseReleased(const Event& event) override; void onMouseDragged(const Event& event) override; + void onMouseScrolled(const Event& event) override; void onDraw(ogfx::BasicRenderer2D& gfx) override; inline f32 getValue(void) const { return m_value; } inline void setValueChangedCallback(std::function callback) { callback_onValueChanged = std::move(callback); } @@ -48,6 +49,7 @@ namespace ogfx inline void disableHorizontal(void) { enableHorizontal(false); } OSTD_BOOL_PARAM_GETSET_E(MiddleClickShortcut, m_middleClickShortcut); + OSTD_BOOL_PARAM_GETSET_E(ScrollWheelShortcut, m_enableScrollWheel); OSTD_PARAM_GETSET(f32, MinValue, m_min); OSTD_PARAM_GETSET(f32, MaxValue, m_max); OSTD_PARAM_GETSET(f32, Step, m_step); @@ -75,6 +77,7 @@ namespace ogfx f32 m_step { 0.1f }; bool m_vertical { false }; bool m_middleClickShortcut { true }; + bool m_enableScrollWheel { true }; f32 m_trackWidth { 6 }; Color m_trackColor { "#500000FF" }; diff --git a/src/test/GuiTest.cpp b/src/test/GuiTest.cpp index 4e9da71..0c16097 100644 --- a/src/test/GuiTest.cpp +++ b/src/test/GuiTest.cpp @@ -119,12 +119,12 @@ class Window : public ogfx::gui::Window m_list.setSize(200, 300); - for (i32 i = 0; i < 100; i++) + for (i32 i = 0; i < 10000; i++) { - m_list.addLine(ostd::Random::getString(ostd::Random::getui8(0, 40))); + m_list.addLine(ostd::Random::getString(ostd::Random::getui8(1, 40))); } - m_list.getItem(10).setFontSize(40); - m_list.getItem(160).setTextColor(Colors::Crimson); + m_list.getLine(10).setFontSize(40); + m_list.getLine(160).setTextColor(Colors::Crimson); m_list.setSelectionChangedCallback([&](stdvec& selection) -> void {\ std::cout << *(selection[0]) << "\n"; });