egui icon indicating copy to clipboard operation
egui copied to clipboard

Remove lock from `Context`

Open Pjottos opened this issue 3 years ago • 3 comments

This PR removes the main lock in Context and changes some parts of the API to accommodate this change, mainly necessary to satisfy the borrow checker.

This change brings the following advantages:

  • No more deadlocks, code that would create a deadlock now gives a compile time error.

  • Since Context is no longer required to be Send + Sync, callback shapes can capture !Send + !Sync types. For example the three-d context.

  • Improves performance by a couple percent.

  • It is no longer possible to use the same Context to draw UI on multiple threads at the same time, which is bound to give incorrect results.

    Closes #1399

Apologies for the huge PR, but the bulk of the changes is basically search and replace so I hope it's not too difficult to review.

Pjottos avatar Jun 13 '22 00:06 Pjottos

Just merged this into my own fork of egui, so I have a few comments as I just migrated my egui application to base off of this:

  • maybe make a note of the requirement of using memory_mut() instead of simply memory() to gain mutable access to memory
  • make a note of requiring a mutable access to context with things like on_hover_text

If this PR is to be accepted, I would recommend something of a migration guide or some deep documentation of the developer-facing changes so projects can more easily migrate.

But I can notice a performance improvement myself, and about a 1% size decrease.

Titaniumtown avatar Jun 19 '22 04:06 Titaniumtown

This is a very nice effort, but I am not so happy with the worsening ergonomics. Adding <'_> everywhere is very ugly imho, and many methods now require an extra &mut Context argument.

emilk avatar Jul 03 '22 12:07 emilk

Will any work be done on this? This was very promising.

Titaniumtown avatar Sep 07 '22 13:09 Titaniumtown

I decided to go this route instead:

  • https://github.com/emilk/egui/pull/2625

Still, thanks so much for all the work in this PR 🙏

emilk avatar Feb 04 '23 12:02 emilk