jeremywen / JW-Modules

VCV Rack Modules
BSD 3-Clause "New" or "Revised" License
216 stars 26 forks source link

v1: Crash when selecting blank panels from module browser #37

Closed cschol closed 5 years ago

cschol commented 5 years ago

Rack v1.0.0 crashes when selecting any of the 3 blank modules ("JW Head") in the module browser.

Stack trace:

Program terminated with signal SIGSEGV, Segmentation fault.
#0  rack::widget::Widget::recursePositionEvent<void (rack::widget::Widget::*)(rack::event::Button const&), rack::event::Button> (e=..., f=&virtual table offset 72, this=0x56529fb188d0) at include/widget/Widget.hpp:122
122                             if (!e.isPropagating())
[Current thread is 1 (Thread 0x7fa8cd649180 (LWP 22220))]
(gdb) bt
#0  0x000056529ca6bf77 in rack::widget::Widget::recursePositionEvent<void (rack::widget::Widget::*)(rack::event::Button const&), rack::event::Button>(void (rack::widget::Widget::*)(rack::event::Button const&), rack::event::Button const&) (e=..., f=&virtual table offset 72, this=0x56529fb188d0)
    at include/widget/Widget.hpp:122
#1  0x000056529ca6bf77 in rack::widget::Widget::onButton(rack::event::Button const&) (this=0x56529fb188d0, e=...) at include/widget/Widget.hpp:143
#2  0x000056529cabaddd in rack::engine::Module::toJson() (this=0x56529fb188d0) at src/engine/Module.cpp:58
#3  0x000056529caa2174 in rack::app::ModuleWidget::toJson() (this=0x56529edd72e0) at src/app/ModuleWidget.cpp:498
#4  0x000056529ca34e05 in rack::history::ModuleAdd::setModule(rack::app::ModuleWidget*) (this=this@entry=0x56529ed8d490, mw=mw@entry=0x56529edd72e0) at src/history.cpp:51
#5  0x000056529ca9d052 in rack::app::ModelBox::onButton(rack::event::Button const&) (this=0x56529ed4b470, e=...) at src/app/ModuleBrowser.cpp:585
#6  0x000056529ca6bf64 in rack::widget::Widget::recursePositionEvent<void (rack::widget::Widget::*)(rack::event::Button const&), rack::event::Button>(void (rack::widget::Widget::*)(rack::event::Button const&), rack::event::Button const&) (e=..., f=&virtual table offset 72, this=0x56529ec009a0)
    at include/widget/Widget.hpp:135
#7  0x000056529ca6bf64 in rack::widget::Widget::onButton(rack::event::Button const&) (this=0x56529ec009a0, e=...) at include/widget/Widget.hpp:143
#8  0x000056529ca6bf64 in rack::widget::Widget::recursePositionEvent<void (rack::widget::Widget::*)(rack::event::Button const&), rack::event::Button>(void (rack::widget::Widget::*)(rack::event::Button const&), rack::event::Button const&) (e=..., f=&virtual table offset 72, this=0x56529ebfdcb0)
    at include/widget/Widget.hpp:135
#9  0x000056529ca6bf64 in rack::widget::Widget::onButton(rack::event::Button const&) (this=0x56529ebfdcb0, e=...) at include/widget/Widget.hpp:143
#10 0x000056529ca6bf64 in rack::widget::Widget::recursePositionEvent<void (rack::widget::Widget::*)(rack::event::Button const&), rack::event::Button>(void (rack::widget::Widget::*)(rack::event::Button const&), rack::event::Button const&) (e=..., f=&virtual table offset 72, this=0x56529ebfdbd0)
    at include/widget/Widget.hpp:135
#11 0x000056529ca6bf64 in rack::widget::Widget::onButton(rack::event::Button const&) (this=0x56529ebfdbd0, e=...) at include/widget/Widget.hpp:143
#12 0x000056529ca73914 in rack::widget::Widget::recursePositionEvent<void (rack::widget::Widget::*)(rack::event::Button const&), rack::event::Button>(void (rack::widget::Widget::*)(rack::event::Button const&), rack::event::Button const&) (e=..., f=&virtual table offset 72, this=0x56529ec00870)
    at include/widget/Widget.hpp:135
#13 0x000056529ca73914 in rack::widget::Widget::onButton(rack::event::Button const&) (e=..., this=0x56529ec00870) at include/widget/Widget.hpp:143
#14 0x000056529ca73914 in rack::ui::ScrollWidget::onButton(rack::event::Button const&) (this=0x56529ec00870, e=...) at src/ui/ScrollWidget.cpp:67
#15 0x000056529ca6f23e in rack::widget::Widget::recursePositionEvent<void (rack::widget::Widget::*)(rack::event::Button const&), rack::event::Button>(void (rack::widget::Widget::*)(rack::event::Button const&), rack::event::Button const&) (e=..., f=&virtual table offset 72, this=0x56529ec026e0)
    at include/widget/Widget.hpp:135
#16 0x000056529ca6f23e in rack::widget::Widget::onButton(rack::event::Button const&) (e=..., this=0x56529ec026e0) at include/widget/Widget.hpp:143
#17 0x000056529ca6f23e in rack::widget::OpaqueWidget::onButton(rack::event::Button const&) (this=0x56529ec026e0, e=...) at include/widget/OpaqueWidget.hpp:21
#18 0x000056529ca99a52 in rack::widget::Widget::recursePositionEvent<void (rack::widget::Widget::*)(rack::event::Button const&), rack::event::Button>(void (rack::widget::Widget::*)(rack::event::Button const&), rack::event::Button const&) (e=..., f=&virtual table offset 72, this=0x56529ebfd770)
    at include/widget/Widget.hpp:135
#19 0x000056529ca99a52 in rack::widget::Widget::onButton(rack::event::Button const&) (e=..., this=0x56529ebfd770) at include/widget/Widget.hpp:143
#20 0x000056529ca99a52 in rack::widget::OpaqueWidget::onButton(rack::event::Button const&) (e=..., this=0x56529ebfd770) at include/widget/OpaqueWidget.hpp:21
#21 0x000056529ca99a52 in rack::app::BrowserOverlay::onButton(rack::event::Button const&) (this=0x56529ebfd770, e=...) at src/app/ModuleBrowser.cpp:110
#22 0x000056529ca6f23e in rack::widget::Widget::recursePositionEvent<void (rack::widget::Widget::*)(rack::event::Button const&), rack::event::Button>(void (rack::widget::Widget::*)(rack::event::Button const&), rack::event::Button const&) (e=..., f=&virtual table offset 72, this=0x56529e7e7de0)
    at include/widget/Widget.hpp:135
#23 0x000056529ca6f23e in rack::widget::Widget::onButton(rack::event::Button const&) (e=..., this=0x56529e7e7de0) at include/widget/Widget.hpp:143
#24 0x000056529ca6f23e in rack::widget::OpaqueWidget::onButton(rack::event::Button const&) (this=0x56529e7e7de0, e=...) at include/widget/OpaqueWidget.hpp:21
#25 0x000056529ca6ab9b in rack::event::State::handleButton(rack::math::Vec, int, int, int) (this=0x56529e792350, pos=..., button=0, action=1, mods=<optimized out>) at src/event.cpp:124
#26 0x000056529cace733 in processEvent (event=0x7ffffcd9fef0) at /home/cschol/src/Rack-1.0/dep/glfw/src/x11_window.c:1391
#27 0x000056529cace733 in _glfwPlatformPollEvents () at /home/cschol/src/Rack-1.0/dep/glfw/src/x11_window.c:2698
#28 0x000056529cac5de0 in glfwPollEvents () at /home/cschol/src/Rack-1.0/dep/glfw/src/window.c:1070
#29 0x000056529ca649d7 in rack::Window::run() (this=0x56529e7e1560) at src/window.cpp:318
#30 0x000056529c9dd4ac in main(int, char**) (argc=<optimized out>, argv=<optimized out>) at src/main.cpp:174
jeremywen commented 5 years ago

ok can you try it now latest git hash: 3c6966dc5bb6e7030d1435cfd1fd89d0354c1b25

cschol commented 5 years ago

That did not fix it.

However, I found the issue:

When looking at the Blank panel code in Rack's Core I noticed that your prototype for the Widget classes, e.g. BlankPanelSmallWidget, uses the derived BlankPanel type for the *module pointer:

struct BlankPanelSmallWidget : ModuleWidget { BlankPanelSmallWidget(BlankPanel *module); };

Per the example in Andrew's code, it should be:

struct BlankPanelSmallWidget : ModuleWidget { BlankPanelSmallWidget(Module *module); };

Updating the signature in all instances in the BlankPanel.cpp source file will make it work correctly.

Now, I am a little confused why this code needs to be like this, but the other modules in your plugin are fine using the derived class in their Widget signature. Maybe @AndrewBelt can comment on that?

jeremywen commented 5 years ago

ok try it now 3fd0054 It never crashed for me but whatever.

cschol commented 5 years ago

This is on GNU/Linux (Ubuntu 18.04). Which platform do you develop on?

Still crashing with the latest change.

The following additional change is required to make it work (use base class type in createModel):

Model *modelBlankPanelSmall = createModel<Module, BlankPanelSmallWidget>("BlankPanel_SM");
Model *modelBlankPanelMedium = createModel<Module, BlankPanelMediumWidget>("BlankPanel_MD");
Model *modelBlankPanelLarge = createModel<Module, BlankPanelLargeWidget>("BlankPanel_LG");
jeremywen commented 5 years ago

ok try it now 2bcb52b I'm on a mac. Sorry I couldn't test these changes because the blank panels work for me.

cschol commented 5 years ago

No worries. That's why I test this. And thank you for providing your plugin early so we can figure these things out.

Now it works. I will integrate the new version in the library repo.