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

Add support for impl View for Arc/Rc<impl View> #164

Closed
wants to merge 10 commits into from

Conversation

Philipp-M
Copy link
Contributor

No description provided.

crates/xilem_core/src/view/rc.rs Outdated Show resolved Hide resolved
crates/xilem_web/src/view.rs Outdated Show resolved Hide resolved
@jaredoconnell
Copy link
Contributor

Now, or later when the examples are improved, this would definitely benefit from a brief example.

@Philipp-M
Copy link
Contributor Author

Now, or later when the examples are improved, this would definitely benefit from a brief example.

You're right, I could add one here, that shows a use-case for this (it's an alternative to Memoize)

@Philipp-M
Copy link
Contributor Author

Good idea to mention the example, I've implemented it via an example for memoization and quite a few views were not yet exported... It also revealed a bomb:

View needs to be Sync to support Arc<impl View>.

I think if we want to store shared views in general in the app state this requirement will come sooner or later (e.g. by supporting a rope of views as a ViewSequence).

crates/xilem_core/src/view/rc.rs Outdated Show resolved Hide resolved
crates/xilem_core/src/view/rc.rs Outdated Show resolved Hide resolved
crates/xilem_core/src/view/rc.rs Outdated Show resolved Hide resolved
@Philipp-M
Copy link
Contributor Author

Ok, at least for View::State, Sync bounds are not necessary. I've relaxed this via adjustments in the macros.

@@ -3,22 +3,22 @@

#[macro_export]
macro_rules! generate_anyview_trait {
($anyview:ident, $viewtrait:ident, $viewmarker:ty, $cx:ty, $changeflags:ty, $anywidget:ident, $boxedview:ident; $($ss:tt)*) => {
($anyview:ident, $viewtrait:ident, $viewmarker:ty, $cx:ty, $changeflags:ty, $anywidget:ident, $boxedview:ident; ($($super_bounds:tt)*), ($($state_bounds:tt)*)) => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was going to suggest changing the input to avoid the parentheses here too, but unfortunately that runs into rust-lang/rust#18700 (last comment has a workaround, but probably not worth using that here).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe make the last two parameters optional using $()? though? The invocation with (), () looks weird.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes I think it's difficult to get rid of () in general (while covering everything that is allowed in trait bounds).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've used optionals as you said

crates/xilem_core/src/any_view.rs Outdated Show resolved Hide resolved
crates/xilem_core/src/view/mod.rs Outdated Show resolved Hide resolved
crates/xilem_core/src/view/mod.rs Outdated Show resolved Hide resolved
@Philipp-M
Copy link
Contributor Author

I have added impl View for Arc<dyn AnyView> as well (resolves the TODO in the example and makes everything a little bit cleaner).

@Philipp-M
Copy link
Contributor Author

I've added the super bounds of View to AnyView as well, to avoid Box<dyn AnyView + Send + Sync>

Philipp-M added a commit to Philipp-M/xilem that referenced this pull request Jan 25, 2024
Philipp-M added a commit to Philipp-M/xilem that referenced this pull request Feb 12, 2024
Philipp-M added a commit to Philipp-M/xilem that referenced this pull request Feb 15, 2024
@Philipp-M Philipp-M closed this Mar 13, 2024
@Philipp-M Philipp-M deleted the arc-view branch March 13, 2024 22:16
@Philipp-M Philipp-M restored the arc-view branch March 13, 2024 22:16
@Philipp-M Philipp-M reopened this Mar 13, 2024
@Philipp-M
Copy link
Contributor Author

Whoops, I was a little bit too eager with deleting old branches...

Philipp-M added a commit to Philipp-M/xilem that referenced this pull request May 5, 2024
This ports the `Memoize` view from old xilem, slightly enhances it,
by checking whether the given view callback is a non-capturing closure and
not a function pointer (by asserting `std::mem::size_of::<F>() == 0`)

It also ports the `Arc<impl View>` and `Arc<dyn AnyMasonryView>` from linebender#164
including the example there to show how these two forms of memoization can be used.
Philipp-M added a commit to Philipp-M/xilem that referenced this pull request May 12, 2024
This ports the `Memoize` view from old xilem, slightly enhances it,
by checking whether the given view callback is a non-capturing closure and
not a function pointer (by asserting `std::mem::size_of::<F>() == 0`)

It also ports the `Arc<impl View>` and `Arc<dyn AnyMasonryView>` from linebender#164
including the example there to show how these two forms of memoization can be used.
Philipp-M added a commit to Philipp-M/xilem that referenced this pull request May 12, 2024
This ports the `Memoize` view from old xilem, slightly enhances it,
by checking whether the given view callback is a non-capturing closure and
not a function pointer (by asserting `std::mem::size_of::<F>() == 0`)

It also ports the `Arc<impl View>` and `Arc<dyn AnyMasonryView>` from linebender#164
including the example there to show how these two forms of memoization can be used.
github-merge-queue bot pushed a commit that referenced this pull request May 20, 2024
This ports the `Memoize` view from old xilem, slightly enhances it, by
checking whether the given view callback is a non-capturing closure and
not a function pointer (by asserting `std::mem::size_of::<F>() == 0`)

It also ports the `Arc<impl View>` and `Arc<dyn AnyMasonryView>` from
#164 including the example there to show how these two forms of
memoization can be used.
@DJMcNab
Copy link
Member

DJMcNab commented Jun 6, 2024

I think this can be closed - this is included in #310

@Philipp-M
Copy link
Contributor Author

Yes it can be closed

@Philipp-M Philipp-M closed this Jun 6, 2024
github-merge-queue bot pushed a commit that referenced this pull request Nov 11, 2024
New tracy image:


![image](https://github.com/user-attachments/assets/94e54c89-8159-4dd4-a521-4a7122f64375)

New log tracing file:
<details><summary>An overview of the new logs</summary>
<p>

```
14:37:40.365Z TRACE update_new_widgets:RootWidget{id=#165}: masonry::passes::update: RootWidget received Update::WidgetAdded
14:37:40.365Z TRACE update_new_widgets:RootWidget{id=#165}:Flex{id=#164}: masonry::passes::update: Flex received Update::WidgetAdded
14:37:40.365Z TRACE update_new_widgets:RootWidget{id=#165}:Flex{id=#164}:Flex{id=#8}: masonry::passes::update: Flex received Update::WidgetAdded
14:37:40.365Z TRACE update_new_widgets:RootWidget{id=#165}:Flex{id=#164}:Flex{id=#8}:SizedBox{id=#7}: masonry::passes::update: SizedBox received Update::WidgetAdded
14:37:40.365Z TRACE update_new_widgets:RootWidget{id=#165}:Flex{id=#164}:Flex{id=#8}:SizedBox{id=#7}:Flex{id=#6}: masonry::passes::update: Flex received Update::WidgetAdded
14:37:40.365Z TRACE update_new_widgets:RootWidget{id=#165}:Flex{id=#164}:Flex{id=#8}:SizedBox{id=#7}:Flex{id=#6}:Flex{id=#3}: masonry::passes::update: Flex received Update::WidgetAdded
14:37:40.365Z TRACE update_new_widgets:RootWidget{id=#165}:Flex{id=#164}:Flex{id=#8}:SizedBox{id=#7}:Flex{id=#6}:Flex{id=#3}:Prose{id=#1}: masonry::passes::update: Prose received Update::WidgetAdded
14:37:40.365Z TRACE update_new_widgets:RootWidget{id=#165}:Flex{id=#164}:Flex{id=#8}:SizedBox{id=#7}:Flex{id=#6}:Flex{id=#3}:Label{id=#2}: masonry::passes::update: Label received Update::WidgetAdded
14:37:40.365Z TRACE update_new_widgets:RootWidget{id=#165}:Flex{id=#164}:Flex{id=#8}:SizedBox{id=#7}:Flex{id=#6}:Flex{id=#5}: masonry::passes::update: Flex received Update::WidgetAdded
14:37:40.365Z TRACE update_new_widgets:RootWidget{id=#165}:Flex{id=#164}:Flex{id=#8}:SizedBox{id=#7}:Flex{id=#6}:Flex{id=#5}:VariableLabel{id=#4}: masonry::passes::update: VariableLabel received Update::WidgetAdded
14:37:40.365Z TRACE update_new_widgets:RootWidget{id=#165}:Flex{id=#164}:Flex{id=#17}: masonry::passes::update: Flex received Update::WidgetAdded
14:37:40.365Z TRACE update_new_widgets:RootWidget{id=#165}:Flex{id=#164}:Flex{id=#17}:Button{id=#10}: masonry::passes::update: Button received Update::WidgetAdded
14:37:40.365Z TRACE update_new_widgets:RootWidget{id=#165}:Flex{id=#164}:Flex{id=#17}:Button{id=#10}:Label{id=#9}: masonry::passes::update: Label received Update::WidgetAdded
14:37:40.365Z TRACE update_new_widgets:RootWidget{id=#165}:Flex{id=#164}:Flex{id=#17}:Button{id=#12}: masonry::passes::update: Button received Update::WidgetAdded
14:37:40.365Z TRACE update_new_widgets:RootWidget{id=#165}:Flex{id=#164}:Flex{id=#17}:Button{id=#12}:Label{id=#11}: masonry::passes::update: Label received Update::WidgetAdded
14:37:40.365Z TRACE update_new_widgets:RootWidget{id=#165}:Flex{id=#164}:Flex{id=#17}:Button{id=#14}: masonry::passes::update: Button received Update::WidgetAdded
14:37:40.365Z TRACE update_new_widgets:RootWidget{id=#165}:Flex{id=#164}:Flex{id=#17}:Button{id=#14}:Label{id=#13}: masonry::passes::update: Label received Update::WidgetAdded
14:37:40.365Z TRACE update_new_widgets:RootWidget{id=#165}:Flex{id=#164}:Flex{id=#17}:Button{id=#16}: masonry::passes::update: Button received Update::WidgetAdded
14:37:40.365Z TRACE update_new_widgets:RootWidget{id=#165}:Flex{id=#164}:Flex{id=#17}:Button{id=#16}:Label{id=#15}: masonry::passes::update: Label received Update::WidgetAdded
14:37:40.365Z TRACE update_new_widgets:RootWidget{id=#165}:Flex{id=#164}:Portal{id=#163}: masonry::passes::update: Portal received Update::WidgetAdded
14:37:40.365Z TRACE update_new_widgets:RootWidget{id=#165}:Flex{id=#164}:Portal{id=#163}:Flex{id=#160}: masonry::passes::update: Flex received Update::WidgetAdded
14:37:40.365Z TRACE update_new_widgets:RootWidget{id=#165}:Flex{id=#164}:Portal{id=#163}:Flex{id=#160}:SizedBox{id=#24}: masonry::passes::update: SizedBox received Update::WidgetAdded
14:37:40.365Z TRACE update_new_widgets:RootWidget{id=#165}:Flex{id=#164}:Portal{id=#163}:Flex{id=#160}:SizedBox{id=#24}:Flex{id=#23}: masonry::passes::update: Flex received Update::WidgetAdded
14:37:40.365Z TRACE update_new_widgets:RootWidget{id=#165}:Flex{id=#164}:Portal{id=#163}:Flex{id=#160}:SizedBox{id=#24}:Flex{id=#23}:Flex{id=#20}: masonry::passes::update: Flex received Update::WidgetAdded
14:37:40.365Z TRACE update_new_widgets:RootWidget{id=#165}:Flex{id=#164}:Portal{id=#163}:Flex{id=#160}:SizedBox{id=#24}:Flex{id=#23}:Flex{id=#20}:Prose{id=#18}: masonry::passes::update: Prose received Update::WidgetAdded
14:37:40.365Z TRACE update_new_widgets:RootWidget{id=#165}:Flex{id=#164}:Portal{id=#163}:Flex{id=#160}:SizedBox{id=#24}:Flex{id=#23}:Flex{id=#20}:Label{id=#19}: masonry::passes::update: Label received Update::WidgetAdded
14:37:40.365Z TRACE update_new_widgets:RootWidget{id=#165}:Flex{id=#164}:Portal{id=#163}:Flex{id=#160}:SizedBox{id=#24}:Flex{id=#23}:Flex{id=#22}: masonry::passes::update: Flex received Update::WidgetAdded
14:37:40.365Z TRACE update_new_widgets:RootWidget{id=#165}:Flex{id=#164}:Portal{id=#163}:Flex{id=#160}:SizedBox{id=#24}:Flex{id=#23}:Flex{id=#22}:VariableLabel{id=#21}: masonry::passes::update: VariableLabel received Update::WidgetAdded
14:37:40.365Z TRACE update_new_widgets:RootWidget{id=#165}:Flex{id=#164}:Portal{id=#163}:Flex{id=#160}:SizedBox{id=#31}: masonry::passes::update: SizedBox received Update::WidgetAdded
```

</p>
</details> 

This was originally an experiment into caching spans, but I determined
that was non-viable due to the pass names.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants