From 4fd0f87fc7745fdfd5ddcef269695c327a056d83 Mon Sep 17 00:00:00 2001 From: Shark Date: Fri, 6 Sep 2024 15:33:31 +0200 Subject: [PATCH 1/3] implement css inheritance make clippy happy --- crates/gosub_styling/src/render_tree.rs | 58 +++++++++++++++++++++++- crates/gosub_styling/src/styling.rs | 20 ++------ crates/gosub_taffy/src/compute/inline.rs | 2 - 3 files changed, 61 insertions(+), 19 deletions(-) diff --git a/crates/gosub_styling/src/render_tree.rs b/crates/gosub_styling/src/render_tree.rs index b1323ddb2..94349fd78 100644 --- a/crates/gosub_styling/src/render_tree.rs +++ b/crates/gosub_styling/src/render_tree.rs @@ -1,7 +1,7 @@ use std::collections::HashMap; use std::fmt::{Debug, Formatter}; -use crate::property_definitions::get_css_definitions; +use crate::property_definitions::{get_css_definitions, CSS_DEFINITIONS}; use crate::shorthands::FixList; use crate::styling::{match_selector, CssProperties, CssProperty, DeclarationProperty}; use log::warn; @@ -348,6 +348,8 @@ impl RenderTree { self.remove_unrenderable_nodes(); + self.resolve_inheritance(self.root, &Vec::new()); + if L::COLLAPSE_INLINE { self.collapse_inline(self.root); } @@ -355,6 +357,53 @@ impl RenderTree { // self.print_tree(); } + fn resolve_inheritance(&mut self, node_id: NodeId, inherit_props: &Vec<(String, CssValue)>) { + let Some(current_node) = self.get_node(node_id) else { + return; + }; + + for prop in inherit_props { + current_node + .properties + .properties + .entry(prop.0.clone()) + .or_insert_with(|| { + let mut p = CssProperty::new(prop.0.as_str()); + + p.inherited = prop.1.clone(); + + p + }); + } + + let mut inherit_props = inherit_props.clone(); + + 'props: for (name, prop) in &mut current_node.properties.properties { + prop.compute_value(); + + let value = prop.actual.clone(); + + if prop_is_inherit(name) { + for (k, v) in &mut inherit_props { + if k == name { + *v = value; + continue 'props; + } + } + + inherit_props.push((name.clone(), value)); + } + } + + let Some(children) = self.get_children(node_id) else { + return; + }; + + for child in children.clone() { + self.resolve_inheritance(child, &inherit_props); + } + } + /// Removes all unrenderable nodes from the render tree fn remove_unrenderable_nodes(&mut self) { // There are more elements that are not renderable, but for now we only remove the most common ones @@ -491,6 +540,13 @@ impl RenderTree { } } +fn prop_is_inherit(name: &str) -> bool { + CSS_DEFINITIONS + .find_property(name) + .map(|def| def.inherited) + .unwrap_or(false) +} + // Generates a declaration property and adds it to the css_map_entry pub fn add_property_to_map( css_map_entry: &mut CssProperties, diff --git a/crates/gosub_styling/src/styling.rs b/crates/gosub_styling/src/styling.rs index 10ebae73c..7e4877e9f 100644 --- a/crates/gosub_styling/src/styling.rs +++ b/crates/gosub_styling/src/styling.rs @@ -273,6 +273,7 @@ pub struct CssProperty { pub used: CssValue, // Actual value used in the rendering (after rounding, clipping etc.) pub actual: CssValue, + pub inherited: CssValue, } impl CssProperty { @@ -286,6 +287,7 @@ impl CssProperty { computed: CssValue::None, used: CssValue::None, actual: CssValue::None, + inherited: CssValue::None, } } @@ -342,13 +344,8 @@ impl CssProperty { return self.specified.clone(); } - if self.is_inheritable() { - todo!("inheritable properties") - // while let Some(parent) = self.get_parent() { - // if let Some(parent_value) = parent { - // return parent_value.find_computed_value(); - // } - // } + if self.inherited != CssValue::None { + return self.inherited.clone(); } self.get_initial_value().unwrap_or(CssValue::None) @@ -368,15 +365,6 @@ impl CssProperty { } } - // // Returns true when the property is inheritable, false otherwise - fn is_inheritable(&self) -> bool { - let defs = get_css_definitions(); - match defs.find_property(&self.name) { - Some(def) => def.inherited(), - None => false, - } - } - // /// Returns true if the given property is a shorthand property (ie: border, margin etc) pub fn is_shorthand(&self) -> bool { let defs = get_css_definitions(); diff --git a/crates/gosub_taffy/src/compute/inline.rs b/crates/gosub_taffy/src/compute/inline.rs index 38c264610..91908e859 100644 --- a/crates/gosub_taffy/src/compute/inline.rs +++ b/crates/gosub_taffy/src/compute/inline.rs @@ -305,8 +305,6 @@ pub fn compute_inline_layout>( let run_y = run.baseline(); - println!("run_y: {}", run_y); - if current_node_id.into() == 161 || current_node_id.into() == 163 { println!("current_node_id: {:?}", current_node_id.into()); println!("first glyph: {:?}", glyphs.get(2)); From 49fb0744261568568f7e3e8e43e4a42c20186b8e Mon Sep 17 00:00:00 2001 From: Shark Date: Fri, 6 Sep 2024 15:34:04 +0200 Subject: [PATCH 2/3] fix text color, since inheritance works now --- crates/gosub_renderer/src/draw.rs | 47 +++++++++++-------------------- 1 file changed, 17 insertions(+), 30 deletions(-) diff --git a/crates/gosub_renderer/src/draw.rs b/crates/gosub_renderer/src/draw.rs index cb003e3ab..3049ceec1 100644 --- a/crates/gosub_renderer/src/draw.rs +++ b/crates/gosub_renderer/src/draw.rs @@ -250,33 +250,6 @@ where } fn render_node(&mut self, id: NodeId, pos: &mut Point) -> anyhow::Result<()> { - let parent = self.drawer.tree.parent_id(id).unwrap_or(id); - - let parent_color = self - .drawer - .tree - .get_node_mut(parent) - .ok_or(anyhow!("Node {id} not found"))? - .properties - .get("color") - .and_then(|prop| { - prop.compute_value(); - - match &prop.actual { - CssValue::Color(color) => Some(*color), - CssValue::String(color) => Some(RgbColor::from(color.as_str())), - _ => None, - } - }) - .unwrap_or(RgbColor::default()); //HACK: this needs to be removed - - let parent_color = B::Color::rgba( - parent_color.r as u8, - parent_color.g as u8, - parent_color.b as u8, - parent_color.a as u8, - ); - let node = self .drawer .tree @@ -313,14 +286,13 @@ where } } - render_text::(node, parent_color, self.scene, pos); + render_text::(node, self.scene, pos); Ok(()) } } fn render_text( - node: &RenderTreeNode, - color: B::Color, + node: &mut RenderTreeNode, scene: &mut B::Scene, pos: &Point, ) where @@ -335,6 +307,21 @@ fn render_text( // return; // } + let color = node + .properties + .get("color") + .and_then(|prop| { + prop.compute_value(); + + match &prop.actual { + CssValue::Color(color) => Some(*color), + CssValue::String(color) => Some(RgbColor::from(color.as_str())), + _ => None, + } + }) + .map(|color| Color::rgba(color.r as u8, color.g as u8, color.b as u8, color.a as u8)) + .unwrap_or(Color::BLACK); + if let RenderNodeData::Text(ref text) = node.data { let Some(layout) = text.layout.as_ref() else { warn!("No layout for text node"); From b96d10a9c76c7b83572ecf751a2213f3e64c190d Mon Sep 17 00:00:00 2001 From: Shark Date: Sat, 7 Sep 2024 16:24:37 +0200 Subject: [PATCH 3/3] fix tests --- crates/gosub_styling/src/render_tree.rs | 13 ++++--------- crates/gosub_styling/src/styling.rs | 21 ++++++++++++++------- 2 files changed, 18 insertions(+), 16 deletions(-) diff --git a/crates/gosub_styling/src/render_tree.rs b/crates/gosub_styling/src/render_tree.rs index 94349fd78..6a02d526c 100644 --- a/crates/gosub_styling/src/render_tree.rs +++ b/crates/gosub_styling/src/render_tree.rs @@ -1,9 +1,11 @@ use std::collections::HashMap; use std::fmt::{Debug, Formatter}; -use crate::property_definitions::{get_css_definitions, CSS_DEFINITIONS}; +use crate::property_definitions::get_css_definitions; use crate::shorthands::FixList; -use crate::styling::{match_selector, CssProperties, CssProperty, DeclarationProperty}; +use crate::styling::{ + match_selector, prop_is_inherit, CssProperties, CssProperty, DeclarationProperty, +}; use log::warn; use gosub_css3::stylesheet::{CssDeclaration, CssOrigin, CssSelector, CssStylesheet, CssValue}; @@ -540,13 +542,6 @@ impl RenderTree { } } -fn prop_is_inherit(name: &str) -> bool { - CSS_DEFINITIONS - .find_property(name) - .map(|def| def.inherited) - .unwrap_or(false) -} - // Generates a declaration property and adds it to the css_map_entry pub fn add_property_to_map( css_map_entry: &mut CssProperties, diff --git a/crates/gosub_styling/src/styling.rs b/crates/gosub_styling/src/styling.rs index 7e4877e9f..9daa1e319 100644 --- a/crates/gosub_styling/src/styling.rs +++ b/crates/gosub_styling/src/styling.rs @@ -2,7 +2,7 @@ use core::fmt::Debug; use std::cmp::Ordering; use std::collections::HashMap; -use crate::property_definitions::get_css_definitions; +use crate::property_definitions::{get_css_definitions, CSS_DEFINITIONS}; use gosub_css3::stylesheet::{ CssOrigin, CssSelector, CssSelectorPart, CssSelectorType, CssValue, MatcherType, Specificity, }; @@ -423,6 +423,13 @@ impl CssProperties { } } +pub fn prop_is_inherit(name: &str) -> bool { + CSS_DEFINITIONS + .find_property(name) + .map(|def| def.inherited) + .unwrap_or(false) +} + #[cfg(test)] mod tests { use super::*; @@ -468,7 +475,7 @@ mod tests { assert!(prop.is_shorthand()); assert_eq!(prop.name, "border"); assert_eq!(prop.get_initial_value(), Some(CssValue::None)); - assert!(!prop.is_inheritable()); + assert!(!prop_is_inherit(&prop.name)); } #[test] @@ -487,7 +494,7 @@ mod tests { assert!(!prop.is_shorthand()); assert_eq!(prop.name, "color"); assert_eq!(prop.get_initial_value(), Some(&CssValue::None).cloned()); - assert!(prop.is_inheritable()); + assert!(prop_is_inherit(&prop.name)); } #[test] @@ -559,16 +566,16 @@ mod tests { #[test] fn is_inheritable() { let prop = CssProperty::new("border"); - assert!(!prop.is_inheritable()); + assert!(!prop_is_inherit(&prop.name)); let prop = CssProperty::new("color"); - assert!(prop.is_inheritable()); + assert!(prop_is_inherit(&prop.name)); let prop = CssProperty::new("font"); - assert!(prop.is_inheritable()); + assert!(prop_is_inherit(&prop.name)); let prop = CssProperty::new("border-top-color"); - assert!(!prop.is_inheritable()); + assert!(!prop_is_inherit(&prop.name)); } #[test]