Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

separate cairo event loop #1052

Closed
wants to merge 7 commits into from
Closed

separate cairo event loop #1052

wants to merge 7 commits into from

Conversation

@tehn
Copy link
Member

@tehn tehn commented Mar 30, 2020

TODO

  • mutex lock
  • rest of events
  • alloc/dealloc string data
  • evaluate cairo doubles
  • check for head/tail overwriting

reposting review from @catfact

it looks pretty straightforward and functional to me.- yeah the Q needs a lock. not just so that you can use a condvar(btw since sleep takes integer seconds, i think sleep(0.01) is just sleep(0) so this is a hot loop explaining the 10% probably..) but becase event posting and handling aren't atomic. since they are single-producer, single-consumer, maybe it would actually be ok with a little care, but you need a mutex for a condvar anyway and they're cheap. (but we should strucure it s/t actual cairo calls aren't made while holding the mutex or the whole thing is kinda pointless.)- i like the simplicity of just adding static data structure to the queue. this might be a good idea for the main even loop too (only allocating for strings and blobs?)- on that note: you will need to add memory mgmt for strings from lua. currently, the weaver function just calls directly down to cairo with the string on the stack. with the new version, you'll have to allocate a new string b/c as soon as you exit the weaver func, lua's copy can be GC'd. so it follows that you will also have to free matron's copy when handling the event. (that is, free the memory pointed to by *c fields in the event data union)- does cairo really require us to use doubles for everything?

actually now that i think of it, the data structure is a little sketchy from a synchronization standpoint. in the main event loop, we can handle the event outside of the mutex, because we know that the event memory will never be needed by the producer again and we can free it in the handler.in new scheme, a new event could be posted which rolls around and writes over the event data while we're handling it. (granted, this should only happen if the Q is getting full, but it would cause crazy havoc so there would need to be strong safeguards... not sufficient to check the queue size when posting just by looking at the head/tail indices within a lock, b/c real work hasn't been done with the data yet.)

@tehn tehn added the do not merge label Mar 30, 2020
@tehn
Copy link
Member Author

@tehn tehn commented May 18, 2020

further improvements that should be done in parallel:

  • screen_init should be split up. if it's done carefully, different implementations of screen_init_surface (or whatever) could address the framebuffer, or a GL context or whatever.
  • put any io_ctl stuff in a separate source module that "hides" the fb device mgmt stuff. (but i know you tend to prefer more monolithic source modules than i do.)
  • with PR 1052, event handlers become another logical place for abstraction. (norns drawing events correspond to cairo commands, but there's no actual need for them to.)
@tehn tehn changed the base branch from master to main Jul 2, 2020
@tehn
Copy link
Member Author

@tehn tehn commented Dec 18, 2020

@catfact closing this in favor of your from-scratch implementation

@tehn tehn closed this Dec 18, 2020
@tehn tehn deleted the cairo-event branch Jan 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

1 participant