servo: Merge #12233 - Make text decorations have the same color as the text if no shadows are present (from KiChjang:underline-color); r=emilio
authorKeith Yeung <kungfukeith11@gmail.com>
Wed, 06 Jul 2016 01:05:13 -0700
changeset 339219 40cd84ff76b228f45bbe92c27552d34a697b3d54
parent 339218 5d03948225c38a2301d625599380c82b4aa94595
child 339220 3183b3611761536bb0d0c4daefa0d79aa7bd55e6
push id31307
push usergszorc@mozilla.com
push dateSat, 04 Feb 2017 00:59:06 +0000
treeherdermozilla-central@94079d43835f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersemilio
servo: Merge #12233 - Make text decorations have the same color as the text if no shadows are present (from KiChjang:underline-color); r=emilio <!-- Please describe your changes on the following line: --> --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [x] These changes fix #11870 (github issue number if applicable). <!-- Either: --> - [ ] There are tests for these changes OR - [x] These changes do not require tests because according to @SimonSapin, making reftests against underlines are impossible <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> Source-Repo: https://github.com/servo/servo Source-Revision: b577d66ec374811a6493e95b37829269bbc97dfa
servo/components/layout/display_list_builder.rs
--- a/servo/components/layout/display_list_builder.rs
+++ b/servo/components/layout/display_list_builder.rs
@@ -38,16 +38,17 @@ use model::{self, MaybeAuto, ToGfxMatrix
 use net_traits::image::base::PixelFormat;
 use net_traits::image_cache_thread::UsePlaceholder;
 use range::Range;
 use script_layout_interface::restyle_damage::REPAINT;
 use std::default::Default;
 use std::sync::Arc;
 use std::{cmp, f32};
 use style::computed_values::filter::Filter;
+use style::computed_values::text_shadow::TextShadow;
 use style::computed_values::{_servo_overflow_clip_box as overflow_clip_box};
 use style::computed_values::{background_attachment, background_clip, background_origin};
 use style::computed_values::{background_repeat, background_size, border_style};
 use style::computed_values::{cursor, image_rendering, overflow_x, pointer_events, position};
 use style::computed_values::{transform, transform_style, visibility};
 use style::logical_geometry::{LogicalPoint, LogicalRect, LogicalSize, WritingMode};
 use style::properties::style_structs::ServoBorder;
 use style::properties::{self, ComputedValues, ServoComputedValues};
@@ -253,24 +254,22 @@ pub trait FragmentDisplayListBuilding {
                                                       state: &mut DisplayListBuildState,
                                                       stacking_relative_border_box: &Rect<Au>,
                                                       display_list_section: DisplayListSection,
                                                       clip: &ClippingRegion);
 
     /// Creates the text display item for one text fragment. This can be called multiple times for
     /// one fragment if there are text shadows.
     ///
-    /// `shadow_blur_radius` will be `Some` if this is a shadow, even if the blur radius is zero.
+    /// `text_shadow` will be `Some` if this is rendering a shadow.
     fn build_display_list_for_text_fragment(&self,
                                             state: &mut DisplayListBuildState,
                                             text_fragment: &ScannedTextFragmentInfo,
-                                            text_color: RGBA,
                                             stacking_relative_content_box: &Rect<Au>,
-                                            shadow_blur_radius: Option<Au>,
-                                            offset: &Point2D<Au>,
+                                            text_shadow: Option<&TextShadow>,
                                             clip: &ClippingRegion);
 
     /// Creates the display item for a text decoration: underline, overline, or line-through.
     fn build_display_list_for_text_decoration(&self,
                                               state: &mut DisplayListBuildState,
                                               color: &RGBA,
                                               stacking_relative_box: &LogicalRect<Au>,
                                               clip: &ClippingRegion,
@@ -1145,42 +1144,29 @@ impl FragmentDisplayListBuilding for Fra
 
         match self.specific {
             SpecificFragmentInfo::ScannedText(ref text_fragment) => {
                 // Create items for shadows.
                 //
                 // NB: According to CSS-BACKGROUNDS, text shadows render in *reverse* order (front
                 // to back).
 
-                // TODO(emilio): Allow changing more properties by ::selection
-                let text_color = if text_fragment.selected() {
-                    self.selected_style().get_color().color
-                } else {
-                    self.style().get_color().color
-                };
-
                 for text_shadow in self.style.get_inheritedtext().text_shadow.0.iter().rev() {
-                    let offset = &Point2D::new(text_shadow.offset_x, text_shadow.offset_y);
-                    let color = self.style().resolve_color(text_shadow.color);
                     self.build_display_list_for_text_fragment(state,
                                                               &**text_fragment,
-                                                              color,
                                                               &stacking_relative_content_box,
-                                                              Some(text_shadow.blur_radius),
-                                                              offset,
+                                                              Some(text_shadow),
                                                               clip);
                 }
 
                 // Create the main text display item.
                 self.build_display_list_for_text_fragment(state,
                                                           &**text_fragment,
-                                                          text_color,
                                                           &stacking_relative_content_box,
                                                           None,
-                                                          &Point2D::new(Au(0), Au(0)),
                                                           clip);
 
                 if opts::get().show_debug_fragment_borders {
                     self.build_debug_borders_around_text_fragments(state,
                                                                    self.style(),
                                                                    stacking_relative_border_box,
                                                                    &stacking_relative_content_box,
                                                                    &**text_fragment,
@@ -1534,38 +1520,49 @@ impl FragmentDisplayListBuilding for Fra
         if !border_radii.is_square() {
             current_clip.intersect_with_rounded_rect(stacking_relative_border_box, &border_radii)
         }
     }
 
     fn build_display_list_for_text_fragment(&self,
                                             state: &mut DisplayListBuildState,
                                             text_fragment: &ScannedTextFragmentInfo,
-                                            text_color: RGBA,
                                             stacking_relative_content_box: &Rect<Au>,
-                                            shadow_blur_radius: Option<Au>,
-                                            offset: &Point2D<Au>,
+                                            text_shadow: Option<&TextShadow>,
                                             clip: &ClippingRegion) {
+        // TODO(emilio): Allow changing more properties by ::selection
+        let text_color = if let Some(shadow) = text_shadow {
+            // If we're painting a shadow, paint the text the same color as the shadow.
+            self.style().resolve_color(shadow.color)
+        } else if text_fragment.selected() {
+            // Otherwise, paint the text with the color as described in its styling.
+            self.selected_style().get_color().color
+        } else {
+            self.style().get_color().color
+        };
+        let offset = text_shadow.map(|s| Point2D::new(s.offset_x, s.offset_y)).unwrap_or_else(Point2D::zero);
+        let shadow_blur_radius = text_shadow.map(|s| s.blur_radius).unwrap_or(Au(0));
+
         // Determine the orientation and cursor to use.
         let (orientation, cursor) = if self.style.writing_mode.is_vertical() {
             if self.style.writing_mode.is_sideways_left() {
                 (TextOrientation::SidewaysLeft, Cursor::VerticalText)
             } else {
                 (TextOrientation::SidewaysRight, Cursor::VerticalText)
             }
         } else {
             (TextOrientation::Upright, Cursor::Text)
         };
 
         // Compute location of the baseline.
         //
         // FIXME(pcwalton): Get the real container size.
         let container_size = Size2D::zero();
         let metrics = &text_fragment.run.font_metrics;
-        let stacking_relative_content_box = stacking_relative_content_box.translate(offset);
+        let stacking_relative_content_box = stacking_relative_content_box.translate(&offset);
         let baseline_origin = stacking_relative_content_box.origin +
             LogicalPoint::new(self.style.writing_mode,
                               Au(0),
                               metrics.ascent).to_physical(self.style.writing_mode,
                                                           container_size);
 
         // Create the text display item.
         let base = state.create_base_display_item(&stacking_relative_content_box,
@@ -1575,66 +1572,64 @@ impl FragmentDisplayListBuilding for Fra
                                                   DisplayListSection::Content);
         state.add_display_item(DisplayItem::TextClass(box TextDisplayItem {
             base: base,
             text_run: text_fragment.run.clone(),
             range: text_fragment.range,
             text_color: text_color.to_gfx_color(),
             orientation: orientation,
             baseline_origin: baseline_origin,
-            blur_radius: shadow_blur_radius.unwrap_or(Au(0)),
+            blur_radius: shadow_blur_radius,
         }));
 
         // Create display items for text decorations.
         let mut text_decorations = self.style()
                                        .get_inheritedtext()
                                        ._servo_text_decorations_in_effect;
-        if shadow_blur_radius.is_some() {
-            // If we're painting a shadow, paint the decorations the same color as the shadow.
-            text_decorations.underline = text_decorations.underline.map(|_| text_color);
-            text_decorations.overline = text_decorations.overline.map(|_| text_color);
-            text_decorations.line_through = text_decorations.line_through.map(|_| text_color);
-        }
+        // Note that the text decoration colors are always the same as the text color.
+        text_decorations.underline = text_decorations.underline.map(|_| text_color);
+        text_decorations.overline = text_decorations.overline.map(|_| text_color);
+        text_decorations.line_through = text_decorations.line_through.map(|_| text_color);
 
         let stacking_relative_content_box =
             LogicalRect::from_physical(self.style.writing_mode,
                                        stacking_relative_content_box,
                                        container_size);
         if let Some(ref underline_color) = text_decorations.underline {
             let mut stacking_relative_box = stacking_relative_content_box;
             stacking_relative_box.start.b = stacking_relative_content_box.start.b +
                 metrics.ascent - metrics.underline_offset;
             stacking_relative_box.size.block = metrics.underline_size;
             self.build_display_list_for_text_decoration(state,
                                                         underline_color,
                                                         &stacking_relative_box,
                                                         clip,
-                                                        shadow_blur_radius.unwrap_or(Au(0)));
+                                                        shadow_blur_radius);
         }
 
         if let Some(ref overline_color) = text_decorations.overline {
             let mut stacking_relative_box = stacking_relative_content_box;
             stacking_relative_box.size.block = metrics.underline_size;
             self.build_display_list_for_text_decoration(state,
                                                         overline_color,
                                                         &stacking_relative_box,
                                                         clip,
-                                                        shadow_blur_radius.unwrap_or(Au(0)));
+                                                        shadow_blur_radius);
         }
 
         if let Some(ref line_through_color) = text_decorations.line_through {
             let mut stacking_relative_box = stacking_relative_content_box;
             stacking_relative_box.start.b = stacking_relative_box.start.b + metrics.ascent -
                 metrics.strikeout_offset;
             stacking_relative_box.size.block = metrics.strikeout_size;
             self.build_display_list_for_text_decoration(state,
                                                         line_through_color,
                                                         &stacking_relative_box,
                                                         clip,
-                                                        shadow_blur_radius.unwrap_or(Au(0)));
+                                                        shadow_blur_radius);
         }
     }
 
     fn build_display_list_for_text_decoration(&self,
                                               state: &mut DisplayListBuildState,
                                               color: &RGBA,
                                               stacking_relative_box: &LogicalRect<Au>,
                                               clip: &ClippingRegion,