From afea12180da2da526a3f2279d55a01d34c99569a Mon Sep 17 00:00:00 2001 From: Shark Date: Wed, 18 Sep 2024 12:58:35 +0200 Subject: [PATCH 1/2] fix declarations using only the first value --- .gitignore | 5 +++ .../gosub_css3/src/convert/ast_converter.rs | 19 ++++++--- crates/gosub_css3/src/stylesheet.rs | 4 +- crates/gosub_html5/Cargo.toml | 2 +- .../gosub_styling/src/property_definitions.rs | 23 ++++++++++- crates/gosub_styling/src/render_tree.rs | 39 +++++++++++++------ crates/gosub_taffy/src/compute/inline.rs | 4 +- 7 files changed, 73 insertions(+), 23 deletions(-) diff --git a/.gitignore b/.gitignore index c7a968a96..4800108d3 100644 --- a/.gitignore +++ b/.gitignore @@ -15,3 +15,8 @@ settings.db */gosub_bindings/tests/* !*/gosub_bindings/tests/*.c *.dSYM + +.idea +.gitignore +callgrind.* +old_callgrind.* \ No newline at end of file diff --git a/crates/gosub_css3/src/convert/ast_converter.rs b/crates/gosub_css3/src/convert/ast_converter.rs index ba05aa2ca..3b3834c96 100644 --- a/crates/gosub_css3/src/convert/ast_converter.rs +++ b/crates/gosub_css3/src/convert/ast_converter.rs @@ -1,11 +1,13 @@ +use anyhow::anyhow; +use log::warn; + +use gosub_shared::types::Result; + use crate::node::{Node as CssNode, NodeType}; use crate::stylesheet::{ AttributeSelector, Combinator, CssDeclaration, CssOrigin, CssRule, CssSelector, CssSelectorPart, CssStylesheet, CssValue, MatcherType, }; -use anyhow::anyhow; -use gosub_shared::types::Result; -use log::warn; /* @@ -208,9 +210,15 @@ pub fn convert_ast_to_stylesheet( continue; } + let value = if css_values.len() == 1 { + css_values.pop().expect("unreachable") + } else { + CssValue::List(css_values) + }; + rule.declarations.push(CssDeclaration { property: property.clone(), - value: css_values.to_vec(), + value, important: *important, }); } @@ -223,10 +231,11 @@ pub fn convert_ast_to_stylesheet( #[cfg(test)] mod tests { - use super::*; use crate::parser_config::ParserConfig; use crate::Css3; + use super::*; + #[test] fn convert_font_family() { let ast = Css3::parse( diff --git a/crates/gosub_css3/src/stylesheet.rs b/crates/gosub_css3/src/stylesheet.rs index 1a4a40a8b..7dfa5d494 100644 --- a/crates/gosub_css3/src/stylesheet.rs +++ b/crates/gosub_css3/src/stylesheet.rs @@ -56,7 +56,7 @@ pub struct CssDeclaration { pub property: String, // Raw values of the declaration. It is not calculated or converted in any way (ie: "red", "50px" etc) // There can be multiple values (ie: "1px solid black" are split into 3 values) - pub value: Vec, + pub value: CssValue, // ie: !important pub important: bool, } @@ -472,7 +472,7 @@ mod test { }], declarations: vec![CssDeclaration { property: "color".to_string(), - value: vec![CssValue::String("red".to_string())], + value: CssValue::String("red".to_string()), important: false, }], }; diff --git a/crates/gosub_html5/Cargo.toml b/crates/gosub_html5/Cargo.toml index d398a05ce..731f86819 100644 --- a/crates/gosub_html5/Cargo.toml +++ b/crates/gosub_html5/Cargo.toml @@ -8,7 +8,7 @@ license = "MIT" [dependencies] gosub_shared = { path = "../gosub_shared", features = [] } gosub_css3 = { path = "../gosub_css3", features = [] } -derive_more = { version = "1", features = ["from"] } +derive_more = { version = "1", features = ["from", "display"] } phf = { version = "0.11.2", features = ["macros"] } lazy_static = "1.5" thiserror = "1.0.61" diff --git a/crates/gosub_styling/src/property_definitions.rs b/crates/gosub_styling/src/property_definitions.rs index 2ed361366..7a8f59d50 100644 --- a/crates/gosub_styling/src/property_definitions.rs +++ b/crates/gosub_styling/src/property_definitions.rs @@ -1,12 +1,11 @@ use std::collections::HashMap; +use std::sync::LazyLock; use log::warn; use gosub_css3::stylesheet::CssValue; use crate::shorthands::{FixList, Shorthands}; -use std::sync::LazyLock; - use crate::syntax::GroupCombinators::Juxtaposition; use crate::syntax::{CssSyntax, SyntaxComponent}; use crate::syntax_matcher::CssSyntaxTree; @@ -892,4 +891,24 @@ mod tests { unit!(4.0, "px"), ])); } + + #[test] + fn test_font_var() { + let definitions = get_css_definitions(); + let def = definitions + .find_property("font-variation-settings") + .unwrap(); + + assert_true!(def.matches(&[str!("normal")])); + + assert_true!(def.matches(&[str!("wgth"), CssValue::Number(100.0)])); + + assert_true!(def.matches(&[ + str!("wgth"), + CssValue::Number(100.0), + CssValue::Comma, + str!("ital"), + CssValue::Number(100.0) + ])); + } } diff --git a/crates/gosub_styling/src/render_tree.rs b/crates/gosub_styling/src/render_tree.rs index 4529e3f31..d54d1a22b 100644 --- a/crates/gosub_styling/src/render_tree.rs +++ b/crates/gosub_styling/src/render_tree.rs @@ -1,5 +1,6 @@ use std::collections::HashMap; use std::fmt::{Debug, Formatter}; +use std::slice; use log::warn; @@ -314,15 +315,26 @@ impl RenderTree { let value = resolve_functions(&declaration.value, node, &doc); + let match_value = if let CssValue::List(value) = &value { + &**value + } else { + slice::from_ref(&value) + }; + + // create property for the given values + let property_name = declaration.property.clone(); + + if &property_name == "font-variation-settings" { + println!("Font variation settings: {:?}", &value); + } + // Check if the declaration matches the definition and return the "expanded" order - let res = definition.matches_and_shorthands(&value, &mut fix_list); + let res = definition.matches_and_shorthands(match_value, &mut fix_list); if !res { warn!("Declaration does not match definition: {:?}", declaration); continue; } - // create property for the given values - let property_name = declaration.property.clone(); let decl = CssDeclaration { property: property_name.to_string(), value, @@ -585,7 +597,7 @@ pub fn add_property_to_map( // let declaration = DeclarationProperty { // @todo: this seems wrong. We only get the first values from the declared values - value: declaration.value.first().unwrap().clone(), + value: declaration.value.clone(), origin: sheet.origin.clone(), important: declaration.important, location: sheet.location.clone(), @@ -964,13 +976,11 @@ pub fn node_is_undernderable(node: &gosub_html5::node::Node) -> bool { } pub fn resolve_functions( - value: &[CssValue], + value: &CssValue, node: &gosub_html5::node::Node, doc: &Document, -) -> Vec { - let mut result = Vec::with_capacity(value.len()); //TODO: we could give it a &mut Vec and reuse the allocation - - for val in value { +) -> CssValue { + fn resolve(val: &CssValue, node: &gosub_html5::node::Node, doc: &Document) -> CssValue { match val { CssValue::Function(func, values) => { let resolved = match func.as_str() { @@ -980,13 +990,18 @@ pub fn resolve_functions( _ => vec![val.clone()], }; - result.extend(resolved); + CssValue::List(resolved) } - _ => result.push(val.clone()), + _ => val.clone(), } } - result + if let CssValue::List(list) = value { + let resolved = list.iter().map(|val| resolve(val, node, doc)).collect(); + CssValue::List(resolved) + } else { + resolve(value, node, doc) + } } // pub fn walk_render_tree(tree: &RenderTree, visitor: &mut Box>) { diff --git a/crates/gosub_taffy/src/compute/inline.rs b/crates/gosub_taffy/src/compute/inline.rs index f2d6f27ec..2856ff699 100644 --- a/crates/gosub_taffy/src/compute/inline.rs +++ b/crates/gosub_taffy/src/compute/inline.rs @@ -633,7 +633,9 @@ fn parse_font_style(node: &mut impl Node) -> FontStyle { } fn parse_font_axes(p: &mut impl Node) -> Vec { - _ = p; + dbg!(p.get_property("font-variation-settings")); + + // todo!(); //TODO From acbc9add0f1100e8c8a5f85fcf69116eb181b374 Mon Sep 17 00:00:00 2001 From: Shark Date: Wed, 18 Sep 2024 22:22:50 +0200 Subject: [PATCH 2/2] fix wong parsing of commas --- crates/gosub_css3/src/parser/value.rs | 2 +- crates/gosub_css3/src/stylesheet.rs | 3 +++ crates/gosub_render_backend/src/layout.rs | 4 ++++ crates/gosub_styling/src/render_tree.rs | 12 ++++++++---- 4 files changed, 16 insertions(+), 5 deletions(-) diff --git a/crates/gosub_css3/src/parser/value.rs b/crates/gosub_css3/src/parser/value.rs index d7697b3c0..7bf4a000c 100644 --- a/crates/gosub_css3/src/parser/value.rs +++ b/crates/gosub_css3/src/parser/value.rs @@ -48,7 +48,7 @@ impl Css3<'_> { Ok(Some(node)) } TokenType::Comma => { - let node = Node::new(NodeType::Operator(",".into()), t.location); + let node = Node::new(NodeType::Comma, t.location); Ok(Some(node)) } TokenType::LBracket => Err(Error::new( diff --git a/crates/gosub_css3/src/stylesheet.rs b/crates/gosub_css3/src/stylesheet.rs index 7dfa5d494..dcdfdfca9 100644 --- a/crates/gosub_css3/src/stylesheet.rs +++ b/crates/gosub_css3/src/stylesheet.rs @@ -390,6 +390,9 @@ impl CssValue { } Ok(CssValue::Function(name, list)) } + + crate::node::NodeType::Comma => Ok(CssValue::Comma), + _ => Err(anyhow!(format!( "Cannot convert node to CssValue: {:?}", node diff --git a/crates/gosub_render_backend/src/layout.rs b/crates/gosub_render_backend/src/layout.rs index 974de083f..6f5b27316 100644 --- a/crates/gosub_render_backend/src/layout.rs +++ b/crates/gosub_render_backend/src/layout.rs @@ -157,6 +157,8 @@ pub trait CssProperty: Debug + Sized { fn as_list(&self) -> Option>; fn is_none(&self) -> bool; + + fn is_comma(&self) -> bool; } pub trait CssValue: Sized { @@ -170,6 +172,8 @@ pub trait CssValue: Sized { fn as_list(&self) -> Option>; fn is_none(&self) -> bool; + + fn is_comma(&self) -> bool; } pub trait TextLayout { diff --git a/crates/gosub_styling/src/render_tree.rs b/crates/gosub_styling/src/render_tree.rs index d54d1a22b..257ec3a86 100644 --- a/crates/gosub_styling/src/render_tree.rs +++ b/crates/gosub_styling/src/render_tree.rs @@ -324,10 +324,6 @@ impl RenderTree { // create property for the given values let property_name = declaration.property.clone(); - if &property_name == "font-variation-settings" { - println!("Font variation settings: {:?}", &value); - } - // Check if the declaration matches the definition and return the "expanded" order let res = definition.matches_and_shorthands(match_value, &mut fix_list); if !res { @@ -882,6 +878,10 @@ impl gosub_render_backend::layout::CssProperty for CssProperty { fn is_none(&self) -> bool { matches!(self.actual, CssValue::None) } + + fn is_comma(&self) -> bool { + matches!(self.actual, CssValue::Comma) + } } impl gosub_render_backend::layout::CssValue for Value { @@ -940,6 +940,10 @@ impl gosub_render_backend::layout::CssValue for Value { fn is_none(&self) -> bool { matches!(self.0, CssValue::None) } + + fn is_comma(&self) -> bool { + matches!(self.0, CssValue::Comma) + } } impl From for Value {