search

gyscos / cursive

A Text User Interface library for the Rust programming language
MIT License
4.32k stars 247 forks source link

[Bug] ScrollView panics when using mousewheel #356

Open heft2008 opened 5 years ago

heft2008 commented 5 years ago

Problem description

When using the mouse wheel on a ScrollView with visible scrollbars the program panics. This happens with the pancurses backend under Windows10. Crossterm works fine. The scrolling example can be used to reproduce this.

Environment

heft2008 commented 5 years ago

This is the stack backtrace:

thread 'main' panicked at 'attempt to add with overflow', /rustc/3c235d5600393dfe6c36eeed34042efad8d4f26e\src\libcore\ops\arith.rs:100:45 stack backtrace: 0: std::sys::windows::backtrace::set_frames at /rustc/3c235d5600393dfe6c36eeed34042efad8d4f26e\/src\libstd\sys\windows\backtrace\mod.rs:95 1: std::sys::windows::backtrace::unwind_backtrace at /rustc/3c235d5600393dfe6c36eeed34042efad8d4f26e\/src\libstd\sys\windows\backtrace\mod.rs:82 2: std::sys_common::backtrace::_print at /rustc/3c235d5600393dfe6c36eeed34042efad8d4f26e\/src\libstd\sys_common\backtrace.rs:71 3: std::sys_common::backtrace::print at /rustc/3c235d5600393dfe6c36eeed34042efad8d4f26e\/src\libstd\sys_common\backtrace.rs:59 4: std::panicking::default_hook::{{closure}} at /rustc/3c235d5600393dfe6c36eeed34042efad8d4f26e\/src\libstd\panicking.rs:197 5: std::panicking::default_hook at /rustc/3c235d5600393dfe6c36eeed34042efad8d4f26e\/src\libstd\panicking.rs:211 6: std::panicking::rust_panic_with_hook at /rustc/3c235d5600393dfe6c36eeed34042efad8d4f26e\/src\libstd\panicking.rs:474 7: std::panicking::continue_panic_fmt at /rustc/3c235d5600393dfe6c36eeed34042efad8d4f26e\/src\libstd\panicking.rs:381 8: std::panicking::rust_begin_panic at /rustc/3c235d5600393dfe6c36eeed34042efad8d4f26e\/src\libstd\panicking.rs:308 9: core::panicking::panic_fmt at /rustc/3c235d5600393dfe6c36eeed34042efad8d4f26e\/src\libcore\panicking.rs:85 10: core::panicking::panic at /rustc/3c235d5600393dfe6c36eeed34042efad8d4f26e\/src\libcore\panicking.rs:49 11: core::ops::arith::{{impl}}::add at /rustc/3c235d5600393dfe6c36eeed34042efad8d4f26e\src\libcore\ops\arith.rs:100 12: core::ops::function::Fn::call<fn(usize, usize) -> usize,(usize, usize)> at /rustc/3c235d5600393dfe6c36eeed34042efad8d4f26e\src\libcore\ops\function.rs:69 13: cursive::xy::XY::zip_map<usize,usize,usize,fn(usize, usize) -> usize> at C:\Users###.cargo\registry\src\github.com-1ecc6299db9ec823\cursive-0.12.0\src\xy.rs:331 14: cursive::vec::{{impl}}::add<usize,cursive::xy::XY> at C:\Users###.cargo\registry\src\github.com-1ecc6299db9ec823\cursive-0.12.0\src\vec.rs:418 15: cursive::view::scroll::core::Core::is_event_inside at C:\Users###.cargo\registry\src\github.com-1ecc6299db9ec823\cursive-0.12.0\src\view\scroll\core.rs:173 16: cursive::view::scroll::raw::on_event<cursive::views::scroll_view::ScrollView,fn(mut cursive::views::scroll_view::ScrollView) -> mut cursive::view::scroll::core::Core,closure,closure> at C:\Users###.cargo\registry\src\github.com-1ecc6299db9ec823\cursive-0.12.0\src\view\scroll\raw.rs:189 17: cursive::view::scroll::on_event<cursive::views::scroll_view::ScrollView,closure,closure> at C:\Users###.cargo\registry\src\github.com-1ecc6299db9ec823\cursive-0.12.0\src\view\scroll\mod.rs:52 18: cursive::views::scroll_view::{{impl}}::on_event at C:\Users###.cargo\registry\src\github.com-1ecc6299db9ec823\cursive-0.12.0\src\views\scroll_view.rs:152 19: cursive::view::view_wrapper::ViewWrapper::wrap_on_event::{{closure}} at C:\Users###.cargo\registry\src\github.com-1ecc6299db9ec823\cursive-0.12.0\src\view\view_wrapper.rs:65 20: cursive::views::view_box::{{impl}}::with_view_mut<closure,cursive::event::EventResult> at C:\Users###.cargo\registry\src\github.com-1ecc6299db9ec823\cursive-0.12.0\src\views\view_box.rs:59 21: cursive::view::view_wrapper::ViewWrapper::wrap_on_event at C:\Users###.cargo\registry\src\github.com-1ecc6299db9ec823\cursive-0.12.0\src\view\view_wrapper.rs:65 22: cursive::view::view_wrapper::{{impl}}::on_event at C:\Users###.cargo\registry\src\github.com-1ecc6299db9ec823\cursive-0.12.0\src\view\view_wrapper.rs:116 23: cursive::view::view_wrapper::ViewWrapper::wrap_on_event::{{closure}}<cursive::views::sized_view::SizedView> at C:\Users###.cargo\registry\src\github.com-1ecc6299db9ec823\cursive-0.12.0\src\view\view_wrapper.rs:65 24: cursive::views::sized_view::{{impl}}::with_view_mut<cursive::views::view_box::ViewBox,closure,cursive::event::EventResult> at C:\Users###.cargo\registry\src\github.com-1ecc6299db9ec823\cursive-0.12.0\src\view\view_wrapper.rs:182 25: cursive::view::view_wrapper::ViewWrapper::wrap_on_event<cursive::views::sized_view::SizedView> at C:\Users###.cargo\registry\src\github.com-1ecc6299db9ec823\cursive-0.12.0\src\view\view_wrapper.rs:65 26: cursive::view::view_wrapper::{{impl}}::on_event<cursive::views::sized_view::SizedView> at C:\Users###.cargo\registry\src\github.com-1ecc6299db9ec823\cursive-0.12.0\src\view\view_wrapper.rs:116 27: cursive::views::dialog::Dialog::on_event_content at C:\Users###.cargo\registry\src\github.com-1ecc6299db9ec823\cursive-0.12.0\src\views\dialog.rs:297 28: cursive::views::dialog::{{impl}}::on_event at C:\Users###.cargo\registry\src\github.com-1ecc6299db9ec823\cursive-0.12.0\src\views\dialog.rs:615 29: cursive::view::view_wrapper::ViewWrapper::wrap_on_event::{{closure}}<cursive::views::box_view::BoxView> at C:\Users###.cargo\registry\src\github.com-1ecc6299db9ec823\cursive-0.12.0\src\view\view_wrapper.rs:65 30: cursive::views::box_view::{{impl}}::with_view_mut<cursive::views::dialog::Dialog,closure,cursive::event::EventResult> at C:\Users###.cargo\registry\src\github.com-1ecc6299db9ec823\cursive-0.12.0\src\view\view_wrapper.rs:182 31: cursive::view::view_wrapper::ViewWrapper::wrap_on_event<cursive::views::box_view::BoxView> at C:\Users###.cargo\registry\src\github.com-1ecc6299db9ec823\cursive-0.12.0\src\view\view_wrapper.rs:65 32: cursive::view::view_wrapper::{{impl}}::on_event<cursive::views::box_view::BoxView> at C:\Users###.cargo\registry\src\github.com-1ecc6299db9ec823\cursive-0.12.0\src\view\view_wrapper.rs:116 33: cursive::view::view_wrapper::ViewWrapper::wrap_on_event::{{closure}} at C:\Users###.cargo\registry\src\github.com-1ecc6299db9ec823\cursive-0.12.0\src\view\view_wrapper.rs:65 34: cursive::views::view_box::{{impl}}::with_view_mut<closure,cursive::event::EventResult> at C:\Users###.cargo\registry\src\github.com-1ecc6299db9ec823\cursive-0.12.0\src\views\view_box.rs:59 35: cursive::view::view_wrapper::ViewWrapper::wrap_on_event at C:\Users###.cargo\registry\src\github.com-1ecc6299db9ec823\cursive-0.12.0\src\view\view_wrapper.rs:65 36: cursive::view::view_wrapper::{{impl}}::on_event at C:\Users###.cargo\registry\src\github.com-1ecc6299db9ec823\cursive-0.12.0\src\view\view_wrapper.rs:116 37: cursive::views::circular_focus::{{impl}}::wrap_on_event at C:\Users###.cargo\registry\src\github.com-1ecc6299db9ec823\cursive-0.12.0\src\views\circular_focus.rs:64 38: cursive::view::view_wrapper::{{impl}}::on_event<cursive::views::circular_focus::CircularFocus> at C:\Users###.cargo\registry\src\github.com-1ecc6299db9ec823\cursive-0.12.0\src\view\view_wrapper.rs:116 39: cursive::view::view_wrapper::ViewWrapper::wrap_on_event::{{closure}}<cursive::views::layer::Layer<cursive::views::circular_focus::CircularFocus>> at C:\Users###.cargo\registry\src\github.com-1ecc6299db9ec823\cursive-0.12.0\src\view\view_wrapper.rs:65 40: cursive::views::layer::{{impl}}::with_view_mut<cursive::views::circular_focus::CircularFocus,closure,cursive::event::EventResult> at C:\Users###.cargo\registry\src\github.com-1ecc6299db9ec823\cursive-0.12.0\src\view\view_wrapper.rs:182 41: cursive::view::view_wrapper::ViewWrapper::wrap_on_event<cursive::views::layer::Layer<cursive::views::circular_focus::CircularFocus>> at C:\Users###.cargo\registry\src\github.com-1ecc6299db9ec823\cursive-0.12.0\src\view\view_wrapper.rs:65 42: cursive::view::view_wrapper::{{impl}}::on_event<cursive::views::layer::Layer<cursive::views::circular_focus::CircularFocus>> at C:\Users###.cargo\registry\src\github.com-1ecc6299db9ec823\cursive-0.12.0\src\view\view_wrapper.rs:116 43: cursive::views::shadow_view::{{impl}}::wrap_on_event<cursive::views::layer::Layer<cursive::views::circular_focus::CircularFocus>> at C:\Users###.cargo\registry\src\github.com-1ecc6299db9ec823\cursive-0.12.0\src\views\shadow_view.rs:70 44: cursive::view::view_wrapper::{{impl}}::on_event<cursive::views::shadow_view::ShadowView<cursive::views::layer::Layer<cursive::views::circular_focus::CircularFocus>>> at C:\Users###.cargo\registry\src\github.com-1ecc6299db9ec823\cursive-0.12.0\src\view\view_wrapper.rs:116 45: cursive::views::stack_view::{{impl}}::on_event at C:\Users###.cargo\registry\src\github.com-1ecc6299db9ec823\cursive-0.12.0\src\views\stack_view.rs:141 46: cursive::views::stack_view::{{impl}}::on_event at C:\Users###.cargo\registry\src\github.com-1ecc6299db9ec823\cursive-0.12.0\src\views\stack_view.rs:618 47: cursive::cursive::Cursive::on_event at C:\Users###.cargo\registry\src\github.com-1ecc6299db9ec823\cursive-0.12.0\src\cursive.rs:778 48: cursive::cursive::Cursive::step at C:\Users###.cargo\registry\src\github.com-1ecc6299db9ec823\cursive-0.12.0\src\cursive.rs:878 49: cursive::cursive::Cursive::run at C:\Users###.cargo\registry\src\github.com-1ecc6299db9ec823\cursive-0.12.0\src\cursive.rs:858 50: scrolltest::main at .\src\main.rs:23 51: std::rt::lang_start::{{closure}}<()> at /rustc/3c235d5600393dfe6c36eeed34042efad8d4f26e\src\libstd\rt.rs:64 52: std::rt::lang_start_internal::{{closure}} at /rustc/3c235d5600393dfe6c36eeed34042efad8d4f26e\/src\libstd\rt.rs:49 53: std::panicking::try::do_call<closure,i32> at /rustc/3c235d5600393dfe6c36eeed34042efad8d4f26e\/src\libstd\panicking.rs:293 54: panic_unwind::__rust_maybe_catch_panic at /rustc/3c235d5600393dfe6c36eeed34042efad8d4f26e\/src\libpanic_unwind\lib.rs:87 55: std::panicking::try at /rustc/3c235d5600393dfe6c36eeed34042efad8d4f26e\/src\libstd\panicking.rs:272 56: std::panic::catch_unwind at /rustc/3c235d5600393dfe6c36eeed34042efad8d4f26e\/src\libstd\panic.rs:388 57: std::rt::lang_start_internal at /rustc/3c235d5600393dfe6c36eeed34042efad8d4f26e\/src\libstd\rt.rs:48 58: std::rt::lang_start<()> at /rustc/3c235d5600393dfe6c36eeed34042efad8d4f26e\src\libstd\rt.rs:64 59: main 60: invoke_main at d:\agent_work\2\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:78 61: __scrt_common_main_seh at d:\agent_work\2\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:288 62: BaseThreadInitThunk 63: RtlUserThreadStart

gyscos commented 5 years ago

Thanks for the report!

Wow - "add overflow"? With usize? Looks like something wrong is going on.

I can't reproduce this bug using pancurses on linux, so I suspect this may due to the way pdcurses return mouse events, but I don't have a windows machine to test this right now.

I'll investigate a bit, and come back when I can test more.

nemo-nullius commented 4 years ago

I have the same problem on my platform (Windows 10 Pro build 18363 with pancurses as backend).

After some tests, it seems that the mouse position never changes with pancurses as backend when my mouse wheel scrolls. Wherever my mouse pointer is, its position remains { x: 18,446,744,073,709,551,615, y: 18,446,744,073,709,551,615,}. However, when I click or do other jobs, the mouse position is fine.

nemo-nullius commented 4 years ago

OK, this problem comes from pancurses. When mouse wheel scrolls, its _getmouse() returns (-1, -1) rather than a real position.

nemo-nullius commented 4 years ago

Finally found a solution.

Not change the behavior of pdcurses-sys

This is a dirty workaround with the help of a global variable. In this way, the default mouse position is the top left corner (0,0). When the mouse wheel scrolls, its position will be the last mouse position stored in the variable OLD_POSITION.

However, the y position will be messed up in this way.

diff --git a/src/backend/curses/pan.rs b/src/backend/curses/pan.rs
index 747e70f..d827519 100644
--- a/src/backend/curses/pan.rs
+++ b/src/backend/curses/pan.rs
@@ -16,6 +16,12 @@ use super::split_i32;
 // Use AHash instead of the slower SipHash
 type HashMap<K, V> = std::collections::HashMap<K, V, ahash::ABuildHasher>;

+use crate::XY;
+static mut OLD_POSITION:XY<usize> = XY{
+    x:0,
+    y:0
+};
+
 /// Backend using pancurses.
 pub struct Backend {
     // Used
@@ -311,7 +317,21 @@ impl Backend {

         let make_event = |event| Event::Mouse {
             offset: Vec2::zero(),
-            position: Vec2::new(mevent.x as usize, mevent.y as usize),
+            position: {
+                match mevent.x {
+                    -1 => {
+                        unsafe {
+                            OLD_POSITION
+                        }
+                    }
+                    _ => {
+                        unsafe {
+                            OLD_POSITION = Vec2::new(mevent.x as usize, mevent.y as usize);
+                            OLD_POSITION
+                        }
+                    }
+                }
+            },
             event,
         };

Change the behavior of PDCurses used by pdcurses-sys

In this way, when the mouse wheel scrolls, the actual mouse position will be returned.

ATTENTION: The developer of PDCurses prevents this behavior. See the comments in /win32/pdckbd.c :

I think it may be that for wheel events, we return x = y = -1, rather than getting the actual mouse position. I don't like this, but I like messing up existing apps even less. 

diff --git a/win32/pdckbd.c b/win32/pdckbd.c
index a956c49..d61a935 100644
--- a/win32/pdckbd.c
+++ b/win32/pdckbd.c
@@ -565,8 +565,8 @@ static int _process_mouse_event(void)
         pdc_mouse_status.changes = (MEV.dwButtonState & 0xFF000000) ?
             PDC_MOUSE_WHEEL_DOWN : PDC_MOUSE_WHEEL_UP;

-        pdc_mouse_status.x = -1;
-        pdc_mouse_status.y = -1;
+        pdc_mouse_status.x = MEV.dwMousePosition.X;
+        pdc_mouse_status.y = MEV.dwMousePosition.Y;

         memset(&old_mouse_status, 0, sizeof(old_mouse_status));

@@ -578,8 +578,8 @@ static int _process_mouse_event(void)
         pdc_mouse_status.changes = (MEV.dwButtonState & 0xFF000000) ?
             PDC_MOUSE_WHEEL_RIGHT : PDC_MOUSE_WHEEL_LEFT;

-        pdc_mouse_status.x = -1;
-        pdc_mouse_status.y = -1;
+        pdc_mouse_status.x = MEV.dwMousePosition.X;
+        pdc_mouse_status.y = MEV.dwMousePosition.Y;

         memset(&old_mouse_status, 0, sizeof(old_mouse_status));

diff --git a/win32a/pdcscrn.c b/win32a/pdcscrn.c
index 1ec6644..a1d1343 100644
--- a/win32a/pdcscrn.c
+++ b/win32a/pdcscrn.c
@@ -651,9 +651,7 @@ static int set_mouse( const int button_index, const int button_state,
                         /* return x = y = -1,  rather than getting the    */
                         /* actual mouse position.  I don't like this, but */
                         /* I like messing up existing apps even less.     */
-            pt.x = -PDC_cxChar;
-            pt.y = -PDC_cyChar;
-/*          ScreenToClient( PDC_hWnd, &pt);      Wheel posns are in screen, */
+          ScreenToClient( PDC_hWnd, &pt);      /*Wheel posns are in screen, */
         }                         /* not client,  coords;  gotta xform them */
     }
     pdc_mouse_status.x = pt.x / PDC_cxChar;

pancurses-sys uses PDCurses at its 068b93c commit. Download PDCurses @ 068b93c and unzip its content to pancurses-sys\src\PDCurses, and change those two files of PDCurses above. Then copy the whole pancurses-sys folder into pancurses. Change pdcurses-sys="0.7" to pdcurses-sys = { path = "./pdcurses-sys" } under [target.'cfg(windows)'.dependencies]. Finally run cargo run --features win32a --example newtest or cargo run --features win32 --example newtest and you will see the result.