Add AppCtx for shared app-wide services.#1766
Conversation
xStrom
added
masonry
|
I need to review this more in-depth, but some scattered thoughts:
|
|
Regarding the name of I'm not sure I understand your point about centralizing RenderRoot's inputs and outputs. Doesn't having a per-app context do just that? What do you mean by centralizing? |
Mhhh... maybe |
|
Something worth noting as well is that one of the benefits of app_ctx: &mut AppCtx
rstate: &mut SharedRootState
sas: &mut SharedAppStateOf those two you provided, if I had to choose I'd go with Some other options could include going even shorter like |
|
(Am currently on vacation, will reply in a day or two.) |
|
(Well, "a day or two" was optimistic, but I haven't forgotten about this.) |
PoignardAzur
left a comment
PoignardAzur
left a comment
There was a problem hiding this comment.
LGTM.
I have misgivings about the proliferation of context types (in one code path you have a DriverCtx that gives you an AppCtx that gives you two text contexts) and I still think we should come up with a better name than AppCtx, but I don't want to block on naming.
This does seem like a thing we'll want sooner or later, and I don't have any better alternatives.
| /// Creates a new [`AppCtx`] suitable for testing. | ||
| pub fn create_app_ctx() -> AppCtx { | ||
| let clipboard = SimpleClipboard::new(); | ||
| AppCtx::new(Box::new(clipboard)) | ||
| } |
There was a problem hiding this comment.
(Optional) This function should probably be inlined. (It's used in tests/paint.rs which is a special case, but even there you could copy the function.)
| struct SimpleClipboard { | ||
| contents: String, | ||
| } |
There was a problem hiding this comment.
(Optional) I think this type should be called MockClipboard and moved to its own module.
|
@xStrom Any blocker left? |
2 participants
For some state it makes sense to have only one copy per app. Until now we haven't had a mechanism to store and provide access to such shared app state. Instead we've hacked around it in various ways. Let's look at two examples.
Clipboard access. Deciding what to get from the clipboard, if anything, is a widget level concern. Different widgets might want different formats like plain text, rich text, or images. Some widget might want to implement its own custom in-app copy/paste mechanism for some app data and ignore the platform clipboard. However, the current hack we have is that
masonry_winitdetects Ctrl+V and replaces the key event with a plain text paste event. This does not allow for the aforementioned variety of paste logic. Widgets need to be able to decide what to do with the key event or they might want to access the clipboard even from a pointer event.Font database/cache access. This is currently implemented at the
RenderRootlevel, meaning that font resolution, loading, and caching is duplicated for each root. It works but is needlessly inefficient.This PR introduces a new
AppCtxtype that encapsulates these kind of app-wide services. It does require a fair amount of plumbing at the framework level. I think the benefit of synchronous mutable access at the widget level makes it worth it.Because the plumbing changes are already quite noisy I migrated only the clipboard access in this PR. Font context migration would be follow-up work.