dnrops
/
xilem
mirror of https://github.com/linebender/xilem
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Resolve mismatch between label width and alignment (#727) 

If the alignment was set previously, and the `CrossAxisAlignment` was
not `Start`, the width Parley believed we were in wouldn't match the
width used for Masonry layout. This would lead to weird behaviour.

See the diff for the test images in
0a68159869
for more context
This commit is contained in:
Daniel McNab 2024-11-06 11:31:46 +00:00 committed by GitHub
parent 419f8f115e
commit ee3126139f
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
11 changed files with 180 additions and 42 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

37
Cargo.lock generated
View File

@ -122,6 +122,12 @@ version = "1.0.2"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "f26201604c87b1e01bd3d98f8d5d9a8fcbb815e8cedb41ffccbeb4bf593a35fe"
[[package]]
name = "adler2"
version = "2.0.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "512761e0bb2578dd7380c6baaa0f4ce03e84f95e960231d1dec8bf4d7d6e2627"
[[package]]
name = "ahash"
version = "0.8.11"
@ -473,7 +479,7 @@ dependencies = [
"cc",
"cfg-if",
"libc",
"miniz_oxide",
"miniz_oxide 0.7.4",
"object",
"rustc-demangle",
]
@ -556,9 +562,9 @@ checksum = "79296716171880943b8470b5f8d03aa55eb2e645a4874bdbb28adb49162e012c"
[[package]]
name = "bytemuck"
version = "1.16.3"
version = "1.19.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "102087e286b4677862ea56cf8fc58bb2cdfa8725c40ffb80fe3a008eb7f2fc83"
checksum = "8334215b81e418a0a7bdb8ef0849474f40bb10c8b71f1c4ed315cff49f32494d"
dependencies = [
"bytemuck_derive",
]
@ -1104,7 +1110,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "7f211bbe8e69bbd0cfdea405084f128ae8b4aaa6b0b522fc8f2b009084797920"
dependencies = [
"crc32fast",
"miniz_oxide",
"miniz_oxide 0.7.4",
]
[[package]]
@ -2140,6 +2146,15 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "b8a240ddb74feaf34a79a7add65a741f3167852fba007066dcac1ca548d89c08"
dependencies = [
"adler",
]
[[package]]
name = "miniz_oxide"
version = "0.8.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "e2d80299ef12ff69b16a84bb182e3b9df68b5a91574d3d4fa6e41b65deec4df1"
dependencies = [
"adler2",
"simd-adler32",
]
@ -2675,15 +2690,15 @@ checksum = "d231b230927b5e4ad203db57bbcbee2802f6bce620b1e4a9024a07d94e2907ec"
[[package]]
name = "png"
version = "0.17.13"
version = "0.17.14"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "06e4b0d3d1312775e782c86c91a111aa1f910cbb65e1337f9975b5f9a554b5e1"
checksum = "52f9d46a34a05a6a57566bc2bfae066ef07585a6e3fa30fbbdff5936380623f0"
dependencies = [
"bitflags 1.3.2",
"crc32fast",
"fdeflate",
"flate2",
"miniz_oxide",
"miniz_oxide 0.8.0",
]
[[package]]
@ -3532,18 +3547,18 @@ dependencies = [
[[package]]
name = "thiserror"
version = "1.0.63"
version = "1.0.65"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "c0342370b38b6a11b6cc11d6a805569958d54cfa061a29969c3b5ce2ea405724"
checksum = "5d11abd9594d9b38965ef50805c5e469ca9cc6f197f883f717e0269a3057b3d5"
dependencies = [
"thiserror-impl",
]
[[package]]
name = "thiserror-impl"
version = "1.0.63"
version = "1.0.65"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "a4558b58466b9ad7ca0f102865eccc95938dca1a74a856f2b57b6629050da261"
checksum = "ae71770322cbd277e69d762a16c444af02aa0575ac0d174f0b9562d3b37f8602"
dependencies = [
"proc-macro2",
"quote",

32
masonry/src/text/text_layout.rs
View File

@ -255,6 +255,9 @@ impl TextLayout {
}
/// Set the [`Alignment`] for this layout.
///
/// This alignment can only be meaningful when a
/// [maximum width](Self::set_max_advance) is provided.
pub fn set_text_alignment(&mut self, alignment: Alignment) {
if self.alignment != alignment {
self.alignment = alignment;
@ -313,20 +316,43 @@ impl TextLayout {
&self.layout
}
/// The size of the laid-out text, excluding any trailing whitespace.
/// The size of the region the text should be drawn in,
/// excluding any trailing whitespace if present.
///
/// Should be used for the drawing of non-interactive text (as the
/// trailing whitespace is selectable for interactive text).
///
/// This is not meaningful until [`Self::rebuild`] has been called.
pub fn size(&self) -> Size {
self.assert_rebuilt("size");
Size::new(self.layout.width().into(), self.layout.height().into())
Size::new(
self.layout_width(self.layout.width()).into(),
self.layout.height().into(),
)
}
/// The size of the laid-out text, including any trailing whitespace.
///
/// Should be used for the drawing of interactive text.
///
/// This is not meaningful until [`Self::rebuild`] has been called.
pub fn full_size(&self) -> Size {
self.assert_rebuilt("full_size");
Size::new(self.layout.full_width().into(), self.layout.height().into())
Size::new(
self.layout_width(self.layout.full_width()).into(),
self.layout.height().into(),
)
}
/// If performing layout `max_advance` to calculate text alignment, the only
/// reasonable behaviour is to take up the entire available width.
///
/// The coherent way to have multiple items laid out on the same line is for them to
/// be inside the same text layout object "region". This is currently deferred.
/// As an interim solution, we allow multiple items to be on the same line if the `max_advance` wasn't used
/// (and therefore the text alignment argument is effectively ignored).
fn layout_width(&self, width: f32) -> f32 {
self.max_advance.unwrap_or(width)
}
/// Return the text's [`LayoutMetrics`].

40
masonry/src/widget/label.rs
View File

@ -20,8 +20,9 @@ use crate::{
RegisterCtx, TextEvent, Update, UpdateCtx, Widget, WidgetId,
};
// added padding between the edges of the widget and the text.
pub(super) const LABEL_X_PADDING: f64 = 2.0;
/// Added padding between each horizontal edge of the widget
/// and the text in logical pixels.
const LABEL_X_PADDING: f64 = 2.0;
/// Options for handling lines that are too wide for the label.
#[derive(Debug, Clone, Copy, PartialEq)]
@ -202,7 +203,9 @@ impl Widget for Label {
.rebuild(font_ctx, layout_ctx, &self.text, self.text_changed);
self.text_changed = false;
}
// We ignore trailing whitespace for a label
// We would like to ignore trailing whitespace for a label.
// However, Parley doesn't make that an option when using `max_advance`.
// If we aren't wrapping words, we can safely ignore this, however.
let text_size = self.text_layout.size();
let label_size = Size {
height: text_size.height,
@ -265,7 +268,7 @@ mod tests {
use crate::assert_render_snapshot;
use crate::testing::TestHarness;
use crate::theme::{PRIMARY_DARK, PRIMARY_LIGHT};
use crate::widget::{Flex, SizedBox};
use crate::widget::{CrossAxisAlignment, Flex, SizedBox};
#[test]
fn simple_label() {
@ -291,6 +294,35 @@ mod tests {
assert_render_snapshot!(harness, "styled_label");
}
#[test]
/// A wrapping label's alignment should be respected, regardkess of
/// its parent's alignment.
fn label_alignment_flex() {
fn base_label() -> Label {
Label::new("Hello")
.with_text_size(10.0)
.with_line_break_mode(LineBreaking::WordWrap)
}
let label1 = base_label().with_text_alignment(Alignment::Start);
let label2 = base_label().with_text_alignment(Alignment::Middle);
let label3 = base_label().with_text_alignment(Alignment::End);
let label4 = base_label().with_text_alignment(Alignment::Start);
let label5 = base_label().with_text_alignment(Alignment::Middle);
let label6 = base_label().with_text_alignment(Alignment::End);
let flex = Flex::column()
.with_flex_child(label1, CrossAxisAlignment::Start)
.with_flex_child(label2, CrossAxisAlignment::Start)
.with_flex_child(label3, CrossAxisAlignment::Start)
.with_flex_child(label4, CrossAxisAlignment::Center)
.with_flex_child(label5, CrossAxisAlignment::Center)
.with_flex_child(label6, CrossAxisAlignment::Center)
.gap(0.0);
let mut harness = TestHarness::create_with_size(flex, Size::new(80.0, 80.0));
assert_render_snapshot!(harness, "label_alignment_flex");
}
#[test]
fn line_break_modes() {
let widget = Flex::column()

60
masonry/src/widget/prose.rs
View File

@ -11,13 +11,16 @@ use vello::peniko::BlendMode;
use vello::Scene;
use crate::text::{ArcStr, TextBrush, TextWithSelection};
use crate::widget::label::LABEL_X_PADDING;
use crate::widget::{LineBreaking, WidgetMut};
use crate::{
AccessCtx, AccessEvent, BoxConstraints, CursorIcon, EventCtx, LayoutCtx, PaintCtx,
PointerEvent, QueryCtx, RegisterCtx, TextEvent, Update, UpdateCtx, Widget, WidgetId,
};
/// Added padding between each horizontal edge of the widget
/// and the text in logical pixels.
const PROSE_X_PADDING: f64 = 2.0;
/// The prose widget is a widget which displays text which can be
/// selected with keyboard and mouse, and which can be copied from,
/// but cannot be modified by the user.
@ -136,7 +139,7 @@ impl Prose {
impl Widget for Prose {
fn on_pointer_event(&mut self, ctx: &mut EventCtx, event: &PointerEvent) {
let window_origin = ctx.widget_state.window_origin();
let inner_origin = Point::new(window_origin.x + LABEL_X_PADDING, window_origin.y);
let inner_origin = Point::new(window_origin.x + PROSE_X_PADDING, window_origin.y);
match event {
PointerEvent::PointerDown(button, state) => {
if !ctx.is_disabled() {
@ -223,7 +226,7 @@ impl Widget for Prose {
None
} else if bc.max().width.is_finite() {
// TODO: Does Prose have different needs here?
Some(bc.max().width as f32 - 2. * LABEL_X_PADDING as f32)
Some(bc.max().width as f32 - 2. * PROSE_X_PADDING as f32)
} else if bc.min().width.is_sign_negative() {
Some(0.0)
} else {
@ -234,11 +237,11 @@ impl Widget for Prose {
let (font_ctx, layout_ctx) = ctx.text_contexts();
self.text_layout.rebuild(font_ctx, layout_ctx);
}
// We ignore trailing whitespace for a label
let text_size = self.text_layout.size();
// We include trailing whitespace for prose, as it can be selected.
let text_size = self.text_layout.full_size();
let label_size = Size {
height: text_size.height,
width: text_size.width + 2. * LABEL_X_PADDING,
width: text_size.width + 2. * PROSE_X_PADDING,
};
bc.constrain(label_size)
}
@ -255,7 +258,7 @@ impl Widget for Prose {
scene.push_layer(BlendMode::default(), 1., Affine::IDENTITY, &clip_rect);
}
self.text_layout
.draw(scene, Point::new(LABEL_X_PADDING, 0.0));
.draw(scene, Point::new(PROSE_X_PADDING, 0.0));
if self.line_break_mode == LineBreaking::Clip {
scene.pop_layer();
@ -289,4 +292,45 @@ impl Widget for Prose {
}
}
// TODO - Add tests
// TODO - Add more tests
#[cfg(test)]
mod tests {
use parley::layout::Alignment;
use vello::kurbo::Size;
use crate::{
assert_render_snapshot,
testing::TestHarness,
widget::{CrossAxisAlignment, Flex, LineBreaking, Prose},
};
#[test]
/// A wrapping prose's alignment should be respected, regardkess of
/// its parent's alignment.
fn prose_alignment_flex() {
fn base_label() -> Prose {
// Trailing whitespace is displayed when laying out prose.
Prose::new("Hello ")
.with_text_size(10.0)
.with_line_break_mode(LineBreaking::WordWrap)
}
let label1 = base_label().with_text_alignment(Alignment::Start);
let label2 = base_label().with_text_alignment(Alignment::Middle);
let label3 = base_label().with_text_alignment(Alignment::End);
let label4 = base_label().with_text_alignment(Alignment::Start);
let label5 = base_label().with_text_alignment(Alignment::Middle);
let label6 = base_label().with_text_alignment(Alignment::End);
let flex = Flex::column()
.with_flex_child(label1, CrossAxisAlignment::Start)
.with_flex_child(label2, CrossAxisAlignment::Start)
.with_flex_child(label3, CrossAxisAlignment::Start)
.with_flex_child(label4, CrossAxisAlignment::Center)
.with_flex_child(label5, CrossAxisAlignment::Center)
.with_flex_child(label6, CrossAxisAlignment::Center)
.gap(0.0);
let mut harness = TestHarness::create_with_size(flex, Size::new(80.0, 80.0));
assert_render_snapshot!(harness, "prose_alignment_flex");
}
}

3
masonry/src/widget/screenshots/masonry__widget__label__tests__label_alignment_flex.png Normal file
View File

@ -0,0 +1,3 @@
version https://git-lfs.github.com/spec/v1
oid sha256:c4333c177b7c8911446cec6e9c0b3b9ce779fa1b45884845de89ff2cfd0683cd
size 2553

3
masonry/src/widget/screenshots/masonry__widget__prose__tests__prose_alignment_flex.png Normal file
View File

@ -0,0 +1,3 @@
version https://git-lfs.github.com/spec/v1
oid sha256:11a0972091eedf3fcdbf767dd9bb2da2bec93870baae03459c0b7623f12a14f9
size 2534

13
xilem/examples/http_cats.rs
View File

@ -13,8 +13,8 @@ use winit::window::Window;
use xilem::core::fork;
use xilem::core::one_of::OneOf3;
use xilem::view::{
button, flex, image, portal, prose, sized_box, spinner, worker, Axis, CrossAxisAlignment,
FlexExt, FlexSpacer,
button, flex, image, inline_prose, portal, prose, sized_box, spinner, worker, Axis, FlexExt,
FlexSpacer,
};
use xilem::{Color, EventLoop, EventLoopBuilder, TextAlignment, WidgetView, Xilem};
@ -102,12 +102,10 @@ impl HttpCats {
portal(sized_box(info_area).expand_width()).flex(1.),
))
.direction(Axis::Horizontal)
.cross_axis_alignment(CrossAxisAlignment::Fill)
.must_fill_major_axis(true)
.flex(1.),
))
.must_fill_major_axis(true)
.cross_axis_alignment(CrossAxisAlignment::Fill),
.must_fill_major_axis(true),
worker(
worker_value,
|proxy, mut rx| async move {
@ -164,8 +162,8 @@ impl Status {
let code = self.code;
flex((
// TODO: Reduce allocations here?
prose(self.code.to_string()),
prose(self.message),
inline_prose(self.code.to_string()),
inline_prose(self.message),
FlexSpacer::Flex(1.),
// TODO: Spinner if image pending?
// TODO: Tick if image loaded?
@ -199,7 +197,6 @@ impl Status {
prose("Copyright ©️ https://http.cat ").alignment(TextAlignment::End),
))
.main_axis_alignment(xilem::view::MainAxisAlignment::Start)
.cross_axis_alignment(CrossAxisAlignment::Fill)
}
}

4
xilem/examples/mason.rs
View File

@ -61,9 +61,7 @@ fn app_logic(data: &mut AppData) -> impl WidgetView<AppData> {
fork(
flex((
flex((
label("Label")
.brush(Color::REBECCA_PURPLE)
.alignment(TextAlignment::Start),
label("Label").brush(Color::REBECCA_PURPLE),
label("Bold Label").weight(TextWeight::BOLD),
// TODO masonry doesn't allow setting disabled manually anymore?
// label("Disabled label").disabled(),

5
xilem/examples/variable_clock.rs
View File

@ -13,7 +13,8 @@ use time::{OffsetDateTime, UtcOffset};
use winit::error::EventLoopError;
use xilem::core::fork;
use xilem::view::{
button, flex, label, portal, prose, sized_box, task, variable_label, Axis, FlexExt, FlexSpacer,
button, flex, inline_prose, label, portal, prose, sized_box, task, variable_label, Axis,
FlexExt, FlexSpacer,
};
use xilem::{Color, EventLoop, EventLoopBuilder, WidgetView, Xilem};
@ -113,7 +114,7 @@ impl TimeZone {
let date_time_in_self = data.now_utc.to_offset(self.offset);
sized_box(flex((
flex((
prose(self.region),
inline_prose(self.region),
FlexSpacer::Flex(1.),
label(format!("UTC{}", self.offset)).brush(
if data.local_offset.is_ok_and(|it| it == self.offset) {

24
xilem/src/view/prose.rs
View File

@ -2,7 +2,7 @@
// SPDX-License-Identifier: Apache-2.0
use masonry::text::{ArcStr, TextBrush};
use masonry::widget;
use masonry::widget::{self, LineBreaking};
use crate::core::{DynMessage, Mut, ViewMarker};
use crate::{Color, MessageResult, Pod, TextAlignment, View, ViewCtx, ViewId};
@ -13,15 +13,27 @@ pub fn prose(content: impl Into<ArcStr>) -> Prose {
text_brush: Color::WHITE.into(),
alignment: TextAlignment::default(),
text_size: masonry::theme::TEXT_SIZE_NORMAL as f32,
line_break_mode: LineBreaking::WordWrap,
}
}
/// A version of [`prose`] suitable for including in the same line
/// as other content.
///
/// Note that setting [`alignment`](Prose::alignment) on the result
/// will be meaningless.
#[doc(alias = "span")]
pub fn inline_prose(content: impl Into<ArcStr>) -> Prose {
prose(content).line_break_mode(LineBreaking::Overflow)
}
pub struct Prose {
content: ArcStr,
text_brush: TextBrush,
alignment: TextAlignment,
text_size: f32,
line_break_mode: LineBreaking,
// TODO: disabled: bool,
// TODO: add more attributes of `masonry::widget::Prose`
}
@ -43,6 +55,10 @@ impl Prose {
self.text_size = text_size;
self
}
pub fn line_break_mode(mut self, line_break_mode: LineBreaking) -> Self {
self.line_break_mode = line_break_mode;
self
}
}
impl ViewMarker for Prose {}
@ -55,7 +71,8 @@ impl<State, Action> View<State, Action, ViewCtx> for Prose {
widget::Prose::new(self.content.clone())
.with_text_brush(self.text_brush.clone())
.with_text_alignment(self.alignment)
.with_text_size(self.text_size),
.with_text_size(self.text_size)
.with_line_break_mode(self.line_break_mode),
);
(widget_pod, ())
}
@ -79,6 +96,9 @@ impl<State, Action> View<State, Action, ViewCtx> for Prose {
if prev.text_size != self.text_size {
widget::Prose::set_text_size(&mut element, self.text_size);
}
if prev.line_break_mode != self.line_break_mode {
widget::Prose::set_line_break_mode(&mut element, self.line_break_mode);
}
}
fn teardown(&self, (): &mut Self::ViewState, _: &mut ViewCtx, _: Mut<Self::Element>) {}

1
xilem/src/view/variable_label.rs
View File

@ -100,7 +100,6 @@ impl<State, Action> View<State, Action, ViewCtx> for VariableLabel {
let widget_pod = ctx.new_pod(
widget::VariableLabel::new(self.label.clone())
.with_text_brush(self.text_brush.clone())
.with_line_break_mode(widget::LineBreaking::WordWrap)
.with_text_alignment(self.alignment)
.with_font(self.font)
.with_text_size(self.text_size)