Skip to content

Commit

Permalink
Review suggestions
Browse files Browse the repository at this point in the history
  • Loading branch information
noituri committed Jan 30, 2025
1 parent 67d3378 commit 3d67e1f
Show file tree
Hide file tree
Showing 16 changed files with 188 additions and 223 deletions.
9 changes: 9 additions & 0 deletions compositor_api/src/types/component.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,15 @@ pub struct View {
/// List of box shadows.
pub box_shadow: Option<Vec<BoxShadow>>,

/// (**default=`0.0`**) Padding for all sides of the component.
pub padding: Option<f32>,

/// (**default=`0.0`**) Padding for the top and bottom of the component.
pub padding_vertical: Option<f32>,

/// (**default=`0.0`**) Padding for the left and right of the component.
pub padding_horizontal: Option<f32>,

/// (**default=`0.0`**) Padding on top side in pixels.
pub padding_top: Option<f32>,

Expand Down
24 changes: 20 additions & 4 deletions compositor_api/src/types/from_component.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,10 +87,26 @@ impl TryFrom<View> for scene::ViewComponent {
None => scene::Overflow::Hidden,
};
let padding = scene::Padding {
top: view.padding_top.unwrap_or(0.0),
right: view.padding_right.unwrap_or(0.0),
bottom: view.padding_bottom.unwrap_or(0.0),
left: view.padding_left.unwrap_or(0.0),
top: view
.padding_top
.or(view.padding_vertical)
.or(view.padding)
.unwrap_or(0.0),
bottom: view
.padding_bottom
.or(view.padding_vertical)
.or(view.padding)
.unwrap_or(0.0),
left: view
.padding_left
.or(view.padding_horizontal)
.or(view.padding)
.unwrap_or(0.0),
right: view
.padding_right
.or(view.padding_horizontal)
.or(view.padding)
.unwrap_or(0.0),
};

if padding.top < 0.0 || padding.right < 0.0 || padding.bottom < 0.0 || padding.left < 0.0 {
Expand Down
24 changes: 7 additions & 17 deletions compositor_render/src/scene/layout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use crate::{
use super::{
rescaler_component::StatefulRescalerComponent, tiles_component::StatefulTilesComponent,
view_component::StatefulViewComponent, AbsolutePosition, BorderRadius, ComponentId,
HorizontalPosition, Padding, Position, RGBAColor, Size, StatefulComponent, VerticalPosition,
HorizontalPosition, Position, RGBAColor, Size, StatefulComponent, VerticalPosition,
};

#[derive(Debug, Clone)]
Expand Down Expand Up @@ -161,28 +161,18 @@ impl StatefulLayoutComponent {
child: &mut StatefulComponent,
position: AbsolutePosition,
parent_size: Size,
child_padding: Padding,
parent_padding: Padding,
pts: Duration,
) -> NestedLayout {
let width = position
.width
.unwrap_or(parent_size.width + child_padding.horizontal());
let height = position
.height
.unwrap_or(parent_size.height + child_padding.vertical());
let width = position.width.unwrap_or(parent_size.width);
let height = position.height.unwrap_or(parent_size.height);

let top = match position.position_vertical {
VerticalPosition::TopOffset(top) => top + parent_padding.top,
VerticalPosition::BottomOffset(bottom) => {
parent_size.height - bottom - height - parent_padding.bottom + parent_padding.top
}
VerticalPosition::TopOffset(top) => top,
VerticalPosition::BottomOffset(bottom) => parent_size.height - bottom - height,
};
let left = match position.position_horizontal {
HorizontalPosition::LeftOffset(left) => left + parent_padding.left,
HorizontalPosition::RightOffset(right) => {
parent_size.width - right - width - parent_padding.right + parent_padding.left
}
HorizontalPosition::LeftOffset(left) => left,
HorizontalPosition::RightOffset(right) => parent_size.width - right - width,
};

let rotation_degrees = position.rotation_degrees;
Expand Down
4 changes: 0 additions & 4 deletions compositor_render/src/scene/view_component.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,10 +66,6 @@ impl StatefulViewComponent {
.with_padding(view.padding)
}

pub(super) fn padding(&self, pts: Duration) -> Padding {
self.view(pts).padding
}

pub(super) fn component_id(&self) -> Option<&ComponentId> {
self.end.id.as_ref()
}
Expand Down
54 changes: 19 additions & 35 deletions compositor_render/src/scene/view_component/layout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ use std::time::Duration;

use crate::{
scene::{
layout::StatefulLayoutComponent, BorderRadius, Overflow, Padding, Position, RGBAColor,
Size, StatefulComponent, ViewChildrenDirection,
layout::StatefulLayoutComponent, BorderRadius, Overflow, Position, RGBAColor, Size,
StatefulComponent, ViewChildrenDirection,
},
transformations::layout::{LayoutContent, Mask, NestedLayout},
};
Expand Down Expand Up @@ -72,18 +72,12 @@ impl ViewComponentParam {
let children: Vec<_> = children
.iter_mut()
.map(|child| {
let (position, padding) = match child {
StatefulComponent::Layout(StatefulLayoutComponent::View(view)) => {
(view.position(pts), view.padding(pts))
}
StatefulComponent::Layout(layout) => (layout.position(pts), Padding::default()),
ref non_layout_component => (
Position::Static {
width: non_layout_component.width(pts),
height: non_layout_component.height(pts),
},
Padding::default(),
),
let position = match child {
StatefulComponent::Layout(layout) => layout.position(pts),
ref non_layout_component => Position::Static {
width: non_layout_component.width(pts),
height: non_layout_component.height(pts),
},
};
match position {
Position::Static { width, height } => {
Expand All @@ -105,12 +99,7 @@ impl ViewComponentParam {
}
Position::Absolute(position) => {
StatefulLayoutComponent::layout_absolute_position_child(
child,
position,
size,
padding,
self.padding,
pts,
child, position, size, pts,
)
}
}
Expand Down Expand Up @@ -143,27 +132,22 @@ impl ViewComponentParam {
pts: Duration,
) -> (NestedLayout, f32) {
let mut static_offset = opts.static_offset;

let (static_width, static_height) = match self.direction {
ViewChildrenDirection::Row => (opts.static_child_size, opts.parent_size.height),
ViewChildrenDirection::Column => (opts.parent_size.width, opts.static_child_size),
};

// Parent padding can shrink the child if it doesn't have width/height provided
let static_width = static_width - self.padding.horizontal();
let static_height = static_height - self.padding.vertical();

let width = opts.width.unwrap_or(static_width);
let height = opts.height.unwrap_or(static_height);

let (top, left, width, height) = match self.direction {
ViewChildrenDirection::Row => {
let width = opts.width.unwrap_or(opts.static_child_size);
let height = opts
.height
.unwrap_or(opts.parent_size.height - self.padding.vertical());
let top = opts.parent_border_width + self.padding.top;
let left = static_offset + self.padding.left;
static_offset += width;
(top, left, width, height)
}
ViewChildrenDirection::Column => {
let height = opts.height.unwrap_or(opts.static_child_size);
let width = opts
.width
.unwrap_or(opts.parent_size.width - self.padding.horizontal());
let top = static_offset + self.padding.top;
let left = opts.parent_border_width + self.padding.left;
static_offset += height;
Expand Down Expand Up @@ -222,8 +206,8 @@ impl ViewComponentParam {
/// size represents dimensions of content (without a border).
fn static_child_size(&self, size: Size, children: &[StatefulComponent], pts: Duration) -> f32 {
let max_size = match self.direction {
super::ViewChildrenDirection::Row => size.width,
super::ViewChildrenDirection::Column => size.height,
super::ViewChildrenDirection::Row => size.width - self.padding.horizontal(),
super::ViewChildrenDirection::Column => size.height - self.padding.vertical(),
};

let children_with_unknown_size_count = Self::static_children_iter(children, pts)
Expand Down
24 changes: 24 additions & 0 deletions schemas/scene.schema.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

49 changes: 0 additions & 49 deletions snapshot_tests/view/column_view_padding_static_children.scene.json

This file was deleted.

41 changes: 18 additions & 23 deletions snapshot_tests/view/nested_padding_static_children.scene.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,45 +6,40 @@
"children": [
{
"type": "view",
"background_color": "#FF0000FF",
"background_color": "red",
"direction": "row",
"width": 300,
"height": 300,
"children": [
{
"type": "view",
"padding_top": 10,
"padding_left": 5,
"border_width": 4,
"border_color": "#FF00FFFF",
"background_color": "#0000FFFF",
"border_width": 10,
"border_color": "blue",
"children": []
},
{
"type": "view",
"padding_top": 10,
"padding_left": 15,
"border_width": 4,
"border_color": "#00FF00FF",
"padding_top": 20,
"padding_left": 20,
"border_width": 10,
"border_color": "blue",
"children": [
{
"type": "view",
"padding_top": 10,
"padding_top": 20,
"padding_bottom": 20,
"padding_right": 10,
"border_width": 4,
"border_color": "#0000FFFF",
"padding_left": 20,
"background_color": "#00FF00FF",
"padding_right": 40,
"border_width": 10,
"border_color": "green",
"background_color": "blue",
"children": [
{
"type": "view",
"width": 250,
"height": 250,
"padding_top": 40,
"border_width": 4,
"border_color": "#FF0000FF",
"background_color": "#00FFFFFF",
"width": 150,
"height": 150,
"padding_left": 80,
"border_width": 10,
"border_color": "magenta",
"background_color": "yellow",
"children": []
}
]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,31 +6,28 @@
"children": [
{
"type": "view",
"background_color": "#FF0000FF",
"background_color": "red",
"direction": "row",
"width": 300,
"height": 300,
"children": [
{
"type": "view",
"padding_top": 10,
"padding_left": 5,
"background_color": "#0000FFFF",
"background_color": "blue",
"children": []
},
{
"type": "view",
"padding_top": 10,
"padding_left": 15,
"padding_top": 20,
"padding_left": 20,
"children": [
{
"type": "view",
"background_color": "#00FF00FF",
"top": 10,
"right": 30,
"background_color": "yellow",
"top": 40,
"left": 40,
"padding_top": 20,
"padding_left": 20,
"padding_right": 550,
"children": [
{
"type": "input_stream",
Expand Down
Loading

0 comments on commit 3d67e1f

Please sign in to comment.