[librsvg: 7/17] Store a ParsedProperty in a Declaration, not a String value



commit cbb711fd9e7c5a071893f613c64d6750e3ccaef1
Author: Federico Mena Quintero <federico gnome org>
Date:   Thu May 2 18:02:00 2019 -0500

    Store a ParsedProperty in a Declaration, not a String value
    
    This lets us avoid reparsing the property's value each time
    it gets looked up in the CssRules.
    
    Also, this adds
    SpecifiedValues::set_style_pair_from_parsed_property(), which is a
    duplicate of ::parse_style_pair() except that it takes a
    ParsedProperty instead of a string.  It is needed because of
    the (non-fully-compliant?) way we deal with !important properties;
    hopefully that will be refactored away.

 rsvg_internals/src/css.rs        | 57 ++++++++++++++++++++++++----------------
 rsvg_internals/src/properties.rs | 35 +++++++++++++++++++-----
 2 files changed, 62 insertions(+), 30 deletions(-)
---
diff --git a/rsvg_internals/src/css.rs b/rsvg_internals/src/css.rs
index f7ec7785..eec0d497 100644
--- a/rsvg_internals/src/css.rs
+++ b/rsvg_internals/src/css.rs
@@ -14,11 +14,13 @@ use crate::attributes::Attribute;
 use crate::croco::*;
 use crate::error::LoadingError;
 use crate::io::{self, BinaryData};
-use crate::properties::SpecifiedValues;
+use crate::properties::{
+    parse_attribute_value_into_parsed_property, ParsedProperty, SpecifiedValues,
+};
 use crate::util::utf8_cstr;
 
 struct Declaration {
-    prop_value: String,
+    property: ParsedProperty,
     important: bool,
 }
 
@@ -106,29 +108,33 @@ impl CssRules {
             })
     }
 
-    fn define(&mut self, selector: &str, prop_name: &str, prop_value: &str, important: bool) {
+    fn define(
+        &mut self,
+        selector: &str,
+        attr: Attribute,
+        property: ParsedProperty,
+        important: bool,
+    ) {
         let decl_list = self
             .selectors_to_declarations
             .entry(selector.to_string())
             .or_insert_with(|| DeclarationList::new());
 
-        if let Ok(attr) = Attribute::from_str(prop_name) {
-            match decl_list.entry(attr) {
-                Entry::Occupied(mut e) => {
-                    let decl = e.get_mut();
+        match decl_list.entry(attr) {
+            Entry::Occupied(mut e) => {
+                let decl = e.get_mut();
 
-                    if !decl.important {
-                        decl.prop_value = prop_value.to_string();
-                        decl.important = important;
-                    }
+                if !decl.important {
+                    decl.property = property;
+                    decl.important = important;
                 }
+            }
 
-                Entry::Vacant(v) => {
-                    v.insert(Declaration {
-                        prop_value: prop_value.to_string(),
-                        important,
-                    });
-                }
+            Entry::Vacant(v) => {
+                v.insert(Declaration {
+                    property,
+                    important,
+                });
             }
         }
     }
@@ -143,10 +149,9 @@ impl CssRules {
     ) -> bool {
         if let Some(decl_list) = self.selectors_to_declarations.get(selector) {
             for (attr, declaration) in decl_list.iter() {
-                // FIXME: this is ignoring errors
-                let _ = values.parse_style_pair(
+                values.set_style_pair_from_parsed_property(
                     *attr,
-                    &declaration.prop_value,
+                    &declaration.property,
                     declaration.important,
                     important_styles,
                 );
@@ -255,9 +260,15 @@ unsafe extern "C" fn css_property(
 
                 let important = from_glib(a_is_important);
 
-                handler_data
-                    .css_rules
-                    .define(&selector_name, prop_name, &prop_value, important);
+                if let Ok(attr) = Attribute::from_str(prop_name) {
+                    if let Ok(Some(prop)) =
+                        parse_attribute_value_into_parsed_property(attr, &prop_value, true)
+                    {
+                        handler_data
+                            .css_rules
+                            .define(&selector_name, attr, prop, important);
+                    }
+                }
             }
         }
 
diff --git a/rsvg_internals/src/properties.rs b/rsvg_internals/src/properties.rs
index f974e52d..e033d7dd 100644
--- a/rsvg_internals/src/properties.rs
+++ b/rsvg_internals/src/properties.rs
@@ -227,7 +227,7 @@ pub struct ComputedValues {
 }
 
 #[cfg_attr(rustfmt, rustfmt_skip)]
-fn parse_attribute_value_into_parsed_property(attr: Attribute, value: &str, accept_shorthands: bool) -> 
Result<Option<ParsedProperty>, ValueErrorKind> {
+pub fn parse_attribute_value_into_parsed_property(attr: Attribute, value: &str, accept_shorthands: bool) -> 
Result<Option<ParsedProperty>, ValueErrorKind> {
     // please keep these sorted
     match attr {
         Attribute::BaselineShift =>
@@ -537,7 +537,9 @@ impl SpecifiedValues {
         value: &str,
         accept_shorthands: bool,
     ) -> Result<(), NodeError> {
-        match parse_attribute_value_into_parsed_property(attr, value, accept_shorthands).attribute(attr) {
+        match parse_attribute_value_into_parsed_property(attr, value, accept_shorthands)
+            .attribute(attr)
+        {
             Ok(Some(prop)) => self.set_parsed_property(&prop),
             Ok(None) => (),
             Err(e) => {
@@ -591,7 +593,7 @@ impl SpecifiedValues {
         Ok(())
     }
 
-    pub fn parse_style_pair(
+    fn parse_style_pair(
         &mut self,
         attr: Attribute,
         value: &str,
@@ -609,6 +611,24 @@ impl SpecifiedValues {
         self.parse_attribute_pair(attr, value, true)
     }
 
+    pub fn set_style_pair_from_parsed_property(
+        &mut self,
+        attr: Attribute,
+        prop: &ParsedProperty,
+        important: bool,
+        important_styles: &mut HashSet<Attribute>,
+    ) {
+        if !important && important_styles.contains(&attr) {
+            return;
+        }
+
+        if important {
+            important_styles.insert(attr);
+        }
+
+        self.set_parsed_property(prop);
+    }
+
     pub fn parse_style_declarations(
         &mut self,
         declarations: &str,
@@ -666,13 +686,14 @@ where
     parse_input(&mut parser)
 }
 
-pub fn parse_input<T>(
-    input: &mut Parser,
-) -> Result<SpecifiedValue<T>, <T as Parse>::Err>
+pub fn parse_input<T>(input: &mut Parser) -> Result<SpecifiedValue<T>, <T as Parse>::Err>
 where
     T: Property<ComputedValues> + Clone + Default + Parse,
 {
-    if input.try_parse(|p| p.expect_ident_matching("inherit")).is_ok() {
+    if input
+        .try_parse(|p| p.expect_ident_matching("inherit"))
+        .is_ok()
+    {
         Ok(SpecifiedValue::Inherit)
     } else {
         Parse::parse(input).map(SpecifiedValue::Specified)


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]