Skip to content

Comments

Add Selector layer#1591

Open
PoignardAzur wants to merge 5 commits intolinebender:mainfrom
PoignardAzur:add_selector_menu
Open

Add Selector layer#1591
PoignardAzur wants to merge 5 commits intolinebender:mainfrom
PoignardAzur:add_selector_menu

Conversation

@PoignardAzur
Copy link
Contributor

No description provided.

@PoignardAzur PoignardAzur marked this pull request as ready for review January 21, 2026 14:38
Copy link
Member

@xStrom xStrom left a comment

Choose a reason for hiding this comment

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

Overall this looks like a great step towards having a decent selector menu.

There are a bunch of potential race conditions. I don't think they need to be necessarily solved in this PR, but they should at least be documented.

@xStrom xStrom added the waiting on author Needs author input before proceeding. label Jan 28, 2026
@PoignardAzur
Copy link
Contributor Author

I've added more comments and TODOs.

Overall, I think all three potential races conditions can be fixed by using a future change which I'm calling "associated layers", which will basically means that widgets store a list of layers they created, and Masonry keeps that list up to date.

Copy link
Member

@xStrom xStrom left a comment

Choose a reason for hiding this comment

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

Some churn to deal with but we're getting close to landing this!

Comment on lines +6 to +20
use accesskit::{Node, Role};
use masonry_core::core::NoAction;
use tracing::{Span, trace_span};
use vello::Scene;

use crate::core::MeasureCtx;
use crate::core::{
AccessCtx, AccessEvent, ChildrenIds, EventCtx, LayoutCtx, PaintCtx, PointerEvent,
PropertiesMut, PropertiesRef, RegisterCtx, TextEvent, Update, UpdateCtx, Widget, WidgetId,
WidgetMut, WidgetPod,
};
use crate::kurbo::{Axis, Size};
use crate::layout::{LayoutSize, LenReq, SizeDef};
use crate::properties::{BorderWidth, BoxShadow, Padding};
use crate::widgets::Label;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
use accesskit::{Node, Role};
use masonry_core::core::NoAction;
use tracing::{Span, trace_span};
use vello::Scene;
use crate::core::MeasureCtx;
use crate::core::{
AccessCtx, AccessEvent, ChildrenIds, EventCtx, LayoutCtx, PaintCtx, PointerEvent,
PropertiesMut, PropertiesRef, RegisterCtx, TextEvent, Update, UpdateCtx, Widget, WidgetId,
WidgetMut, WidgetPod,
};
use crate::kurbo::{Axis, Size};
use crate::layout::{LayoutSize, LenReq, SizeDef};
use crate::properties::{BorderWidth, BoxShadow, Padding};
use crate::widgets::Label;
use accesskit::{Node, Role};
use tracing::{Span, trace_span};
use vello::Scene;
use crate::core::MeasureCtx;
use crate::core::{
AccessCtx, AccessEvent, ChildrenIds, EventCtx, LayoutCtx, NoAction, PaintCtx, PointerEvent,
PropertiesMut, PropertiesRef, RegisterCtx, TextEvent, Update, UpdateCtx, Widget, WidgetId,
WidgetMut, WidgetPod,
};
use crate::kurbo::{Axis, Size};
use crate::layout::{LayoutSize, LenReq, SizeDef};
use crate::widgets::Label;

Comment on lines +91 to +128
fn measure(
&mut self,
ctx: &mut MeasureCtx<'_>,
props: &PropertiesRef<'_>,
axis: Axis,
len_req: LenReq,
cross_length: Option<f64>,
) -> f64 {
// TODO: Remove HACK: Until scale factor rework happens, just pretend it's always 1.0.
// https://github.com/linebender/xilem/issues/1264
let scale = 1.0;

let border = props.get::<BorderWidth>();
let padding = props.get::<Padding>();

let border_length = border.length(axis).dp(scale);
let padding_length = padding.length(axis).dp(scale);

let cross = axis.cross();
let cross_space = cross_length.map(|cross_length| {
let cross_border_length = border.length(cross).dp(scale);
let cross_padding_length = padding.length(cross).dp(scale);
(cross_length - cross_border_length - cross_padding_length).max(0.)
});

let auto_length = len_req.reduce(border_length + padding_length).into();
let context_size = LayoutSize::maybe(cross, cross_space);

let child_length = ctx.compute_length(
&mut self.child,
auto_length,
context_size,
axis,
cross_space,
);

child_length + border_length + padding_length
}
Copy link
Member

Choose a reason for hiding this comment

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

measure can now be a lot simpler.

Suggested change
fn measure(
&mut self,
ctx: &mut MeasureCtx<'_>,
props: &PropertiesRef<'_>,
axis: Axis,
len_req: LenReq,
cross_length: Option<f64>,
) -> f64 {
// TODO: Remove HACK: Until scale factor rework happens, just pretend it's always 1.0.
// https://github.com/linebender/xilem/issues/1264
let scale = 1.0;
let border = props.get::<BorderWidth>();
let padding = props.get::<Padding>();
let border_length = border.length(axis).dp(scale);
let padding_length = padding.length(axis).dp(scale);
let cross = axis.cross();
let cross_space = cross_length.map(|cross_length| {
let cross_border_length = border.length(cross).dp(scale);
let cross_padding_length = padding.length(cross).dp(scale);
(cross_length - cross_border_length - cross_padding_length).max(0.)
});
let auto_length = len_req.reduce(border_length + padding_length).into();
let context_size = LayoutSize::maybe(cross, cross_space);
let child_length = ctx.compute_length(
&mut self.child,
auto_length,
context_size,
axis,
cross_space,
);
child_length + border_length + padding_length
}
fn measure(
&mut self,
ctx: &mut MeasureCtx<'_>,
_props: &PropertiesRef<'_>,
axis: Axis,
len_req: LenReq,
cross_length: Option<f64>,
) -> f64 {
let auto_length = len_req.into();
let context_size = LayoutSize::maybe(axis.cross(), cross_length);
ctx.compute_length(
&mut self.child,
auto_length,
context_size,
axis,
cross_length,
)
}

Comment on lines +130 to +156
fn layout(&mut self, ctx: &mut LayoutCtx<'_>, props: &PropertiesRef<'_>, size: Size) {
// TODO: Remove HACK: Until scale factor rework happens, just pretend it's always 1.0.
// https://github.com/linebender/xilem/issues/1264
let scale = 1.0;

let border = props.get::<BorderWidth>();
let padding = props.get::<Padding>();
let shadow = props.get::<BoxShadow>();

let space = border.size_down(size, scale);
let space = padding.size_down(space, scale);

let child_size = ctx.compute_size(&mut self.child, SizeDef::fit(space), space.into());
ctx.run_layout(&mut self.child, child_size);

let child_origin = ((size - child_size).to_vec2() * 0.5).to_point();
ctx.place_child(&mut self.child, child_origin);

let baseline = ctx.child_baseline_offset(&self.child);
let baseline = border.baseline_up(baseline, scale);
let baseline = padding.baseline_up(baseline, scale);
ctx.set_baseline_offset(baseline);

if shadow.is_visible() {
ctx.set_paint_insets(shadow.get_insets());
}
}
Copy link
Member

Choose a reason for hiding this comment

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

layout can be simplified as well.

Suggested change
fn layout(&mut self, ctx: &mut LayoutCtx<'_>, props: &PropertiesRef<'_>, size: Size) {
// TODO: Remove HACK: Until scale factor rework happens, just pretend it's always 1.0.
// https://github.com/linebender/xilem/issues/1264
let scale = 1.0;
let border = props.get::<BorderWidth>();
let padding = props.get::<Padding>();
let shadow = props.get::<BoxShadow>();
let space = border.size_down(size, scale);
let space = padding.size_down(space, scale);
let child_size = ctx.compute_size(&mut self.child, SizeDef::fit(space), space.into());
ctx.run_layout(&mut self.child, child_size);
let child_origin = ((size - child_size).to_vec2() * 0.5).to_point();
ctx.place_child(&mut self.child, child_origin);
let baseline = ctx.child_baseline_offset(&self.child);
let baseline = border.baseline_up(baseline, scale);
let baseline = padding.baseline_up(baseline, scale);
ctx.set_baseline_offset(baseline);
if shadow.is_visible() {
ctx.set_paint_insets(shadow.get_insets());
}
}
fn layout(&mut self, ctx: &mut LayoutCtx<'_>, _props: &PropertiesRef<'_>, size: Size) {
let child_size = ctx.compute_size(&mut self.child, SizeDef::fit(size), size.into());
ctx.run_layout(&mut self.child, child_size);
let child_origin = ((size - child_size).to_vec2() * 0.5).to_point();
ctx.place_child(&mut self.child, child_origin);
let child_baseline = ctx.child_baseline_offset(&self.child);
let child_bottom = child_origin.y + child_size.height;
let bottom_gap = size.height - child_bottom;
ctx.set_baseline_offset(child_baseline + bottom_gap);
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

waiting on author Needs author input before proceeding.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants