From 87e3dadf22d8477d3c5584caa142cab303b031fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Wed, 26 Jun 2019 21:21:38 +0000 Subject: [PATCH] style: Use the cbindgen representation for grid line properties. We clamp earlier (parse time rather than computed value time), but that's the only behavior change, which I think doesn't really matter. Differential Revision: https://phabricator.services.mozilla.com/D35198 --- components/style/properties/gecko.mako.rs | 54 ++--------------------- components/style/values/computed/mod.rs | 2 +- components/style/values/generics/grid.rs | 25 ++++++++--- 3 files changed, 25 insertions(+), 56 deletions(-) diff --git a/components/style/properties/gecko.mako.rs b/components/style/properties/gecko.mako.rs index ebc91a97296..fce88b6f5aa 100644 --- a/components/style/properties/gecko.mako.rs +++ b/components/style/properties/gecko.mako.rs @@ -880,15 +880,8 @@ class Side(object): self.ident = name.lower() self.index = index -class GridLine(object): - def __init__(self, name): - self.ident = "grid-" + name.lower() - self.name = self.ident.replace('-', '_') - self.gecko = "m" + to_camel_case(self.ident) - SIDES = [Side("Top", 0), Side("Right", 1), Side("Bottom", 2), Side("Left", 3)] CORNERS = ["top_left", "top_right", "bottom_right", "bottom_left"] -GRID_LINES = map(GridLine, ["row-start", "row-end", "column-start", "column-end"]) %> #[allow(dead_code)] @@ -1077,7 +1070,7 @@ fn static_assert() { % endfor -<% skip_position_longhands = " ".join(x.ident for x in SIDES + GRID_LINES) %> +<% skip_position_longhands = " ".join(x.ident for x in SIDES) %> <%self:impl_trait style_struct_name="Position" skip_longhands="${skip_position_longhands} order align-content justify-content align-self @@ -1132,45 +1125,6 @@ fn static_assert() { ${impl_simple_copy('order', 'mOrder')} - % for value in GRID_LINES: - pub fn set_${value.name}(&mut self, v: longhands::${value.name}::computed_value::T) { - use crate::gecko_bindings::structs::{nsStyleGridLine_kMinLine, nsStyleGridLine_kMaxLine}; - - let line = &mut self.gecko.${value.gecko}; - line.mLineName.set_move(unsafe { - RefPtr::from_addrefed(v.ident.into_addrefed()) - }); - line.mHasSpan = v.is_span; - // clamping the integer between a range - line.mInteger = cmp::max( - nsStyleGridLine_kMinLine, - cmp::min(v.line_num, nsStyleGridLine_kMaxLine), - ); - } - - pub fn copy_${value.name}_from(&mut self, other: &Self) { - self.gecko.${value.gecko}.mHasSpan = other.gecko.${value.gecko}.mHasSpan; - self.gecko.${value.gecko}.mInteger = other.gecko.${value.gecko}.mInteger; - unsafe { - self.gecko.${value.gecko}.mLineName.set(&other.gecko.${value.gecko}.mLineName); - } - } - - pub fn reset_${value.name}(&mut self, other: &Self) { - self.copy_${value.name}_from(other) - } - - pub fn clone_${value.name}(&self) -> longhands::${value.name}::computed_value::T { - longhands::${value.name}::computed_value::T { - is_span: self.gecko.${value.gecko}.mHasSpan, - ident: unsafe { - Atom::from_raw(self.gecko.${value.gecko}.mLineName.mRawPtr) - }, - line_num: self.gecko.${value.gecko}.mInteger, - } - } - % endfor - % for kind in ["rows", "columns"]: pub fn set_grid_auto_${kind}(&mut self, v: longhands::grid_auto_${kind}::computed_value::T) { let gecko = &mut *self.gecko; @@ -1194,11 +1148,11 @@ fn static_assert() { pub fn set_grid_template_${kind}(&mut self, v: longhands::grid_template_${kind}::computed_value::T) { <% self_grid = "self.gecko.mGridTemplate%s" % kind.title() %> - use crate::gecko_bindings::structs::{nsTArray, nsStyleGridLine_kMaxLine}; + use crate::gecko_bindings::structs::nsTArray; use std::usize; use crate::values::CustomIdent; use crate::values::generics::grid::TrackListType::Auto; - use crate::values::generics::grid::{GridTemplateComponent, RepeatCount}; + use crate::values::generics::grid::{GridTemplateComponent, RepeatCount, MAX_GRID_LINE}; #[inline] fn set_line_names(servo_names: &[CustomIdent], gecko_names: &mut nsTArray>) { @@ -1213,7 +1167,7 @@ fn static_assert() { } } - let max_lines = nsStyleGridLine_kMaxLine as usize - 1; // for accounting the final + let max_lines = MAX_GRID_LINE as usize - 1; // for accounting the final let result = match v { GridTemplateComponent::None => ptr::null_mut(), diff --git a/components/style/values/computed/mod.rs b/components/style/values/computed/mod.rs index 999596c25f7..6e38eeeda75 100644 --- a/components/style/values/computed/mod.rs +++ b/components/style/values/computed/mod.rs @@ -7,7 +7,7 @@ use self::transform::DirectionVector; use super::animated::ToAnimatedValue; use super::generics::grid::GridTemplateComponent as GenericGridTemplateComponent; -use super::generics::grid::{GridLine as GenericGridLine, TrackBreadth as GenericTrackBreadth}; +use super::generics::grid::{GenericGridLine, TrackBreadth as GenericTrackBreadth}; use super::generics::grid::{TrackList as GenericTrackList, TrackSize as GenericTrackSize}; use super::generics::transform::IsParallelTo; use super::generics::{self, GreaterThanOrEqualToOne, NonNegative, ZeroToOne}; diff --git a/components/style/values/generics/grid.rs b/components/style/values/generics/grid.rs index 2f876e784dd..3504278cfb8 100644 --- a/components/style/values/generics/grid.rs +++ b/components/style/values/generics/grid.rs @@ -13,9 +13,17 @@ use crate::values::specified::grid::parse_line_names; use crate::values::{CSSFloat, CustomIdent}; use cssparser::Parser; use std::fmt::{self, Write}; -use std::{mem, usize}; +use std::{cmp, mem, usize}; use style_traits::{CssWriter, ParseError, StyleParseErrorKind, ToCss}; + +/// These are the limits that we choose to clamp grid line numbers to. +/// http://drafts.csswg.org/css-grid/#overlarge-grids +/// line_num is clamped to this range at parse time. +pub const MIN_GRID_LINE: i32 = -10000; +/// See above. +pub const MAX_GRID_LINE: i32 = 10000; + /// A `` type. /// /// @@ -32,14 +40,20 @@ use style_traits::{CssWriter, ParseError, StyleParseErrorKind, ToCss}; )] #[repr(C)] pub struct GenericGridLine { - /// Flag to check whether it's a `span` keyword. - pub is_span: bool, /// A custom identifier for named lines, or the empty atom otherwise. /// /// pub ident: Atom, /// Denotes the nth grid line from grid item's placement. + /// + /// This is clamped by MIN_GRID_LINE and MAX_GRID_LINE. + /// + /// NOTE(emilio): If we ever allow animating these we need to either do + /// something more complicated for the clamping, or do this clamping at + /// used-value time. pub line_num: Integer, + /// Flag to check whether it's a `span` keyword. + pub is_span: bool, } pub use self::GenericGridLine as GridLine; @@ -128,11 +142,12 @@ impl Parse for GridLine { grid_line.is_span = true; } else if let Ok(i) = input.try(|i| specified::Integer::parse(context, i)) { // FIXME(emilio): Probably shouldn't reject if it's calc()... - if i.value() == 0 || val_before_span || grid_line.line_num.value() != 0 { + let value = i.value(); + if value == 0 || val_before_span || !grid_line.line_num.is_zero() { return Err(location.new_custom_error(StyleParseErrorKind::UnspecifiedError)); } - grid_line.line_num = i; + grid_line.line_num = specified::Integer::new(cmp::max(MIN_GRID_LINE, cmp::min(value, MAX_GRID_LINE))); } else if let Ok(name) = input.try(|i| i.expect_ident_cloned()) { if val_before_span || grid_line.ident != atom!("") { return Err(location.new_custom_error(StyleParseErrorKind::UnspecifiedError));