jpcima / spectacle

Realtime graphical spectrum analyzer
ISC License
60 stars 5 forks source link

Give events to subwidgets first if needed; Use onResize on UI #21

Closed falkTX closed 3 years ago

falkTX commented 3 years ago

Initial fixup, main issue was on DPF side. Some comments:

The reshape/resize change makes the main toolbar not appear on the correct position at the start, dont know why yet..

Also, something is still wrong on DPF side regarding position of sub-sub widgets. This makes the slider not work here. But note that you dont need to compare coordinates yourself, each widget has a contains helper method for this, ie:

bool MyWidget::onMouse(const MouseEvent& ev)
{
    if (ev.press && contains(ev.pos))
    {
    }
...

I will have to see what is wrong with the sub-sub widget positioning, I thought I handled that already...

jpcima commented 3 years ago

Oh nice findings, thanks for looking into it. The crash on exit is something new. I've had carla updates since, which always put me in doubt whether it's a host or plugin thing. It looks even more broken on my end, the tool bar isn't even appearing.

falkTX commented 3 years ago

It looks even more broken on my end, the tool bar isn't even appearing.

It appears after a resize.

I am pretty sure there is a plugin problem of sorts because even the jack standalone crashes on exit. Doing the sub-sub widget positioning fixes now.

falkTX commented 3 years ago

Fixed the toolbar too just now, not sure if made in the most correct way or not, the toolbar just lacked height/size on init.

jpcima commented 3 years ago

he toolbar just lacked height/size on init

If I may ask, why does MainToolbar require height to be set on init, in order to work? it would be given its height just a little later, as part of doResize.

falkTX commented 3 years ago

he toolbar just lacked height/size on init

If I may ask, why does MainToolbar require height to be set on init, in order to work? it would be given its height just a little later, as part of doResize.

need to check again, but you have some logic that skips setting something if size is not set. Previously you would call uiReshape yourself, and then DPF would call it too, so this was skipped since the height was set from the 1st uiReshape.

jpcima commented 3 years ago

This still has issues however. It crashes in valgrind, not always identical locations. It has NanoTopLevelWidget::onDisplay()->NanoVG::reset()->nvgReset->memset writing invalid memory.

falkTX commented 3 years ago

the reset I added as a test. not sure if needed, it was in vcv but I think now it doesnt make sense. try to remove it. one thing I learned with vcv is that we cannot have DEBUG defined as a macro in plugin header files if not building the whole DPF as DEBUG, because then the size and position of data is not going to match.