masonry: Refactor local transform as Property#1374
masonry: Refactor local transform as Property#1374Philipp-M wants to merge 1 commit intolinebender:mainfrom
Property#1374Conversation
| child_state.transform_changed = true; | ||
| changed_properties.mark_as_changed::<Transform>(); |
There was a problem hiding this comment.
Not sure whether it's the right thing to change this. Alternatively we could leave the transform_changed flag within WidgetState additionally, and check both in the compose pass (or set transform_changed as well in WidgetMut::property_changed).
There was a problem hiding this comment.
I think this alternative is the better way around. My understanding is that the semantics of prop_has_changed are very clear, which is that they mark whether the transform has changed during the lifetime of this WidgetMut, right?
DJMcNab
left a comment
There was a problem hiding this comment.
I'm finding this a bit hard to reason about.
| } | ||
| } | ||
|
|
||
| /// When properties of a widget were modified this should also be reflected by [`ChangedProperties::mark_as_changed`] |
There was a problem hiding this comment.
| /// When properties of a widget were modified this should also be reflected by [`ChangedProperties::mark_as_changed`] | |
| /// When properties of a widget were modified this should also be reflected by [`ChangedProperties::mark_as_changed`]. |
| } | ||
|
|
||
| impl ChangedProperties { | ||
| /// Mark that the property `P` has changed |
There was a problem hiding this comment.
| /// Mark that the property `P` has changed | |
| /// Mark that the property `P` has changed. |
etc.
|
|
||
| use crate::core::{GlobalProperty, Property}; | ||
|
|
||
| /// The local transform of a widget |
There was a problem hiding this comment.
| /// The local transform of a widget | |
| /// The local transform of a widget. |
| child_state.transform_changed = true; | ||
| changed_properties.mark_as_changed::<Transform>(); |
There was a problem hiding this comment.
I think this alternative is the better way around. My understanding is that the semantics of prop_has_changed are very clear, which is that they mark whether the transform has changed during the lifetime of this WidgetMut, right?
This got a little bit larger than I would've hoped, and I'm not entirely sure about it (more code without immediate benefit, apart from making the transform a property, which possibly allows other APIs e.g. in Xilem).
It introduces a new marker trait
GlobalPropertywithimpl<P: GlobalProperty, W: Widget> HasProperty<P> for W {}as suggested in #1278And a core property
Transformwhich is a new type wrapper aroundkurbo::Affine, it could reasonably also be a directlyAffine(less boilerplate for sure), but I guess a new type probably is worth it non-the-less.This unfortunately disallows other things I had in mind, due to orphan rules (this would result in a similar behavior as xilem_webs
OneOf):So another case why I'm not sure about merging this. But as it's basically finished, it's at the very least worth opening this.