Skip to content

Commit

Permalink
Fix depth-ordering update (#3486)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
farmaazon authored and jdunkerley committed May 31, 2022
1 parent d627378 commit b059b3f
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 6 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@
- [You can change font and set letters bold in the <code>text::Area</code>
component][3385]. Use the <code>set_font</code> and
<code>set_bold_bytes</code> respectively.
- [Fixed a text rendering issue in nested sublayer][3486].

#### Enso Standard Library

Expand Down Expand Up @@ -201,6 +202,7 @@
[3462]: https://github.com/enso-org/enso/pull/3462
[3463]: https://github.com/enso-org/enso/pull/3463
[3472]: https://github.com/enso-org/enso/pull/3472
[3486]: https://github.com/enso-org/enso/pull/3486
[3478]: https://github.com/enso-org/enso/pull/3478

#### Enso Compiler
Expand Down
27 changes: 21 additions & 6 deletions lib/rust/ensogl/core/src/display/scene/layer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ use smallvec::alloc::collections::BTreeSet;
use std::any::TypeId;



// =============
// === Layer ===
// =============
Expand Down Expand Up @@ -365,7 +364,7 @@ impl LayerModel {
let on_mut = on_depth_order_dirty(&parents);
let depth_order_dirty = dirty::SharedBool::new(logger_dirty, on_mut);
let global_element_depth_order = default();
let sublayers = Sublayers::new(Logger::new_sub(&logger, "registry"));
let sublayers = Sublayers::new(Logger::new_sub(&logger, "registry"), &parents);
let mask = default();
let scissor_box = default();
let mem_mark = default();
Expand Down Expand Up @@ -800,12 +799,14 @@ impl LayerModel {
}
}

/// Unboxed callback.
/// The callback setting `element_depth_order_dirty` in parents.
pub type OnDepthOrderDirty = impl Fn();
fn on_depth_order_dirty(parents: &Rc<RefCell<Vec<Sublayers>>>) -> OnDepthOrderDirty {
let parents = parents.clone();
move || {
for parent in &*parents.borrow() {
// It's safe to do it having parents borrowed, because the only possible callback called
// [`OnElementDepthOrderDirty`], which don't borrow_mut at any point.
parent.element_depth_order_dirty.set()
}
}
Expand Down Expand Up @@ -851,11 +852,24 @@ impl LayerDynamicShapeInstance {
// === Sublayers ===
// =================

/// The callback propagating `element_depth_order_dirty` flag to parents.
pub type OnElementDepthOrderDirty = impl Fn();
fn on_element_depth_order_dirty(parents: &Rc<RefCell<Vec<Sublayers>>>) -> OnElementDepthOrderDirty {
let parents = parents.clone_ref();
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.
sublayers.element_depth_order_dirty.set()
}
}
}

/// Abstraction for layer sublayers.
#[derive(Clone, CloneRef, Debug)]
pub struct Sublayers {
model: Rc<RefCell<SublayersModel>>,
element_depth_order_dirty: dirty::SharedBool,
element_depth_order_dirty: dirty::SharedBool<OnElementDepthOrderDirty>,
}

impl Deref for Sublayers {
Expand All @@ -874,10 +888,11 @@ impl PartialEq for Sublayers {

impl Sublayers {
/// Constructor.
pub fn new(logger: impl AnyLogger) -> Self {
pub fn new(logger: impl AnyLogger, parents: &Rc<RefCell<Vec<Sublayers>>>) -> Self {
let element_dirty_logger = Logger::new_sub(&logger, "dirty");
let model = default();
let element_depth_order_dirty = dirty::SharedBool::new(element_dirty_logger, ());
let dirty_on_mut = on_element_depth_order_dirty(parents);
let element_depth_order_dirty = dirty::SharedBool::new(element_dirty_logger, dirty_on_mut);
Self { model, element_depth_order_dirty }
}
}
Expand Down

0 comments on commit b059b3f

Please sign in to comment.