search

lv2 / pugl

A minimal portable API for embeddable GUIs
https://gitlab.com/lv2/pugl/
ISC License
179 stars 34 forks source link

macOS event loop fixes #57

Closed reuk closed 1 year ago

reuk commented 3 years ago

When running pugl as a PUGL_MODULE, it's dangerous to call sendEvent on arbitrary events, as processing such events could cause the UI instance to be destroyed. Then, when statements later on in puglUpdate are executed, they may attempt to access resources that have been freed, crashing the process. This is currently seen in LV2 UIs which execute puglUpdate from the idle callback.

This change just posts events to the CFRunLoop directly, which seems a bit cleaner. This way, there's no need to convert the event to an NSEvent and back, as we can just capture the event in a block instead.

I also noticed that the test_redisplay app asserts on Big Sur because the OS invalidates the entire window in response to the invalidate request, so I've updated the test a little to be slightly more forgiving about mismatches between the requested and actual repainted areas.

Finally, I noticed that the meson build currently doesn't work out-of-the-box due to undefined references to c_warnings and objc_warnings. Perhaps this is a configuration issue on my system, so I haven't pushed a fix. That said, removing the references allowed the build to succeed.

See also #56 and https://github.com/DISTRHO/DPF/issues/299.

drobilla commented 1 year ago

Well, there's a rebased version at https://github.com/lv2/pugl/tree/nested-event-loop-fix and it doesn't break the tests, but the tests don't currently cover modules.

I don't particularly understand this, but it still seems like an elaborate way to do "make client events not work in modules in MacOS" to me. Apparently not, so I'll take your word for it.

That said, this is also essentially "make puglUpdate do nothing in modules on MacOS" which is really not how the API is supposed to work. If it's literally impossible to dispatch events within a specific scope on MacOS... okay I guess? That seems unlikely though?

If so, I'll at least need a good enough understanding of this to make the documentation not a lie. The time that events are dispatched is a critically important part of the API semantics; if it has to be wildly different on MacOS, so be it, but that needs to be documented.

falkTX commented 1 year ago

if it is worth anything I have been using pugl with this patch/change applied for as long as it has been originally reported, and have seen no issues (contrary to before the patch).

drobilla commented 1 year ago

@falkTX You snuck in before the edit, I hadn't noticed that @reuk said it has been tested.

Client events are only used for... well, code that explicitly uses client events, so pugl would generally work aside from that either way.

falkTX commented 1 year ago

right I never make use of those for the moment.

drobilla commented 1 year ago

Merged as 28d75f7, thanks.

I had to change the run loop mode to kCFRunLoopBeforeSources to get reliable behaviour for programs. Hopefully that doesn't cause any new issues...