-
Notifications
You must be signed in to change notification settings - Fork 321
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
Fix depth-ordering update #3486
Conversation
@@ -851,11 +851,22 @@ impl LayerDynamicShapeInstance { | |||
// === Sublayers === | |||
// ================= | |||
|
|||
/// Unboxed callback. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd write here doc about what the callback does, not what this line is syntactically.
move || { | ||
parent_depth_order_dirty.set(); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
braces not needed
move || { | ||
for sublayers in parents.borrow().iter() { | ||
// It's safe to do it having parents borrowed, because the only possible callback called | ||
// OnElementDepthOrderDirty, which don't borrow_mut at any point. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use the [`...`]
syntax when referring to Rust entities.
There are two dirty flags in layers: depth_order_dirty and element_depth_order_dirty - one marking changed in Layer, second marking change in one of sublayers. The depth_order_dirty has a proper callback for setting element_depth_order_dirty of its parent. However, the latter did not propagate up. I fixed it by adding callback for element_depth_order_dirty which sets the depth_order_dirty of the parent. # Important Notes * The question to @wdanilo : is it possible, that I can propagate dirty directly to element_depth_order_dirty, without setting depth_order_dirty? As far as I understand the code, it would also work (and we would omit some unnecessary updates). * I tried to leave some logs, but I don't feel how to do that: the tooling I used was very specific, only the concrete ids of symbols and layers were logged, and I don't know how to generalize it.
Pull Request Description
There are two dirty flags in layers: depth_order_dirty and element_depth_order_dirty - one marking changed in Layer, second marking change in one of sublayers. The depth_order_dirty has a proper callback for setting element_depth_order_dirty of its parent. However, the latter did not propagate up.
I fixed it by adding callback for element_depth_order_dirty which sets the depth_order_dirty of the parent.
Important Notes
Checklist
Please include the following checklist in your PR:
Scala,
Java,
and
Rust
style guides.
[ ] Unit tests have been written where possible.- The code is a bit untestable due to high coupling../run.sh ide dist
and./run.sh ide watch
.