From ffe9d882e183328d84182f5c91041ead5f8a4836 Mon Sep 17 00:00:00 2001 From: Marco Castelluccio Date: Fri, 4 Oct 2019 12:17:27 +0000 Subject: [PATCH] Bug 1572805 - Use cbindgen for text-emphasis-style. r=boris I sent https://github.com/eqrion/cbindgen/pull/377 since I got sick of copy-pasting the private default constructor stuff :) Differential Revision: https://phabricator.services.mozilla.com/D41419 UltraBlame original commit: f252db10f2e09c75ea4f2db63e101fd53b8e7b4c --- layout/generic/nsLineLayout.cpp | 6 +- layout/generic/nsTextFrame.cpp | 43 ++++++++-- layout/style/ServoBindings.toml | 1 + layout/style/nsStyleConsts.h | 18 ---- layout/style/nsStyleStruct.cpp | 12 ++- layout/style/nsStyleStruct.h | 15 +++- .../components/style/properties/gecko.mako.rs | 84 +------------------ .../components/style/values/computed/text.rs | 3 + .../components/style/values/specified/text.rs | 46 +--------- servo/ports/geckolib/cbindgen.toml | 10 +++ 10 files changed, 72 insertions(+), 166 deletions(-) diff --git a/layout/generic/nsLineLayout.cpp b/layout/generic/nsLineLayout.cpp index e0b4dc37e8791..69231a54f29f4 100644 --- a/layout/generic/nsLineLayout.cpp +++ b/layout/generic/nsLineLayout.cpp @@ -1672,7 +1672,7 @@ void nsLineLayout::AdjustLeadings(nsIFrame* spanFrame, PerSpanData* psd, requiredStartLeading += leadings.mStart; requiredEndLeading += leadings.mEnd; } - if (aStyleText->HasTextEmphasis()) { + if (aStyleText->HasEffectiveTextEmphasis()) { nscoord bsize = GetBSizeOfEmphasisMarks(spanFrame, aInflation); LogicalSide side = aStyleText->TextEmphasisSide(mRootSpan->mWritingMode); if (side == eLogicalSideBStart) { @@ -2279,7 +2279,7 @@ void nsLineLayout::VerticalAlignFrames(PerSpanData* psd) { fm, minimumLineBSize, lineWM.IsLineInverted()); nscoord blockEnd = blockStart + minimumLineBSize; - if (mStyleText->HasTextEmphasis()) { + if (mStyleText->HasEffectiveTextEmphasis()) { nscoord fontMaxHeight = fm->MaxHeight(); nscoord emphasisHeight = GetBSizeOfEmphasisMarks(spanFrame, inflation); @@ -3303,7 +3303,7 @@ void nsLineLayout::RelativePositionFrames(PerSpanData* psd, if (pfd->mRecomputeOverflow || frame->Style()->HasTextDecorationLines() || - frame->StyleText()->HasTextEmphasis() || + frame->StyleText()->HasEffectiveTextEmphasis() || frame->StyleText()->HasWebkitTextStroke()) { nsTextFrame* f = static_cast(frame); r = f->RecomputeOverflow(mBlockReflowInput->mFrame); diff --git a/layout/generic/nsTextFrame.cpp b/layout/generic/nsTextFrame.cpp index 8019843b98f65..043d893a9a1ab 100644 --- a/layout/generic/nsTextFrame.cpp +++ b/layout/generic/nsTextFrame.cpp @@ -2161,7 +2161,7 @@ already_AddRefed BuildTextRunsScanner::BuildTextRunForFrames( lastComputedStyle->IsTextCombined()) { anyTextTransformStyle = true; } - if (textStyle->HasTextEmphasis()) { + if (textStyle->HasEffectiveTextEmphasis()) { anyTextEmphasis = true; } flags |= GetSpacingFlags(f); @@ -5233,10 +5233,38 @@ struct EmphasisMarkInfo { NS_DECLARE_FRAME_PROPERTY_DELETABLE(EmphasisMarkProperty, EmphasisMarkInfo) +static void ComputeTextEmphasisStyleString(const StyleTextEmphasisStyle& aStyle, + nsAString& aOut) { + MOZ_ASSERT(!aStyle.IsNone()); + if (aStyle.IsString()) { + nsDependentCSubstring string = aStyle.AsString().AsString(); + AppendUTF8toUTF16(string, aOut); + return; + } + const auto& keyword = aStyle.AsKeyword(); + const bool fill = keyword.fill == StyleTextEmphasisFillMode::Filled; + switch (keyword.shape) { + case StyleTextEmphasisShapeKeyword::Dot: + return aOut.AppendLiteral(fill ? u"\u2022" : u"\u25e6"); + case StyleTextEmphasisShapeKeyword::Circle: + return aOut.AppendLiteral(fill ? u"\u25cf" : u"\u25cb"); + case StyleTextEmphasisShapeKeyword::DoubleCircle: + return aOut.AppendLiteral(fill ? u"\u25c9" : u"\u25ce"); + case StyleTextEmphasisShapeKeyword::Triangle: + return aOut.AppendLiteral(fill ? u"\u25b2" : u"\u25b3"); + case StyleTextEmphasisShapeKeyword::Sesame: + return aOut.AppendLiteral(fill ? u"\ufe45" : u"\ufe46"); + default: + MOZ_ASSERT_UNREACHABLE("Unknown emphasis style shape"); + } +} + static already_AddRefed GenerateTextRunForEmphasisMarks( nsTextFrame* aFrame, gfxFontGroup* aFontGroup, ComputedStyle* aComputedStyle, const nsStyleText* aStyleText) { - const nsString& emphasisString = aStyleText->mTextEmphasisStyleString; + nsAutoString string; + ComputeTextEmphasisStyleString(aStyleText->mTextEmphasisStyle, string); + RefPtr dt = CreateReferenceDrawTarget(aFrame); auto appUnitsPerDevUnit = aFrame->PresContext()->AppUnitsPerDevPixel(); gfx::ShapedTextFlags flags = @@ -5245,9 +5273,9 @@ static already_AddRefed GenerateTextRunForEmphasisMarks( flags = gfx::ShapedTextFlags::TEXT_ORIENT_VERTICAL_UPRIGHT; } - return aFontGroup->MakeTextRun( - emphasisString.get(), emphasisString.Length(), dt, appUnitsPerDevUnit, - flags, nsTextFrameUtils::Flags(), nullptr); + return aFontGroup->MakeTextRun(string.get(), string.Length(), dt, + appUnitsPerDevUnit, flags, + nsTextFrameUtils::Flags(), nullptr); } static nsRubyFrame* FindFurthestInlineRubyAncestor(nsTextFrame* aFrame) { @@ -5265,7 +5293,7 @@ static nsRubyFrame* FindFurthestInlineRubyAncestor(nsTextFrame* aFrame) { nsRect nsTextFrame::UpdateTextEmphasis(WritingMode aWM, PropertyProvider& aProvider) { const nsStyleText* styleText = StyleText(); - if (!styleText->HasTextEmphasis()) { + if (!styleText->HasEffectiveTextEmphasis()) { DeleteProperty(EmphasisMarkProperty()); return nsRect(); } @@ -7117,7 +7145,8 @@ void nsTextFrame::DrawText(Range aRange, const gfx::Point& aTextBaselinePt, const bool drawDecorations = !aParams.provider->GetFontGroup()->ShouldSkipDrawing() && - (decorations.HasDecorationLines() || StyleText()->HasTextEmphasis()); + (decorations.HasDecorationLines() || + StyleText()->HasEffectiveTextEmphasis()); if (drawDecorations) { DrawTextRunAndDecorations(aRange, aTextBaselinePt, aParams, decorations); } else { diff --git a/layout/style/ServoBindings.toml b/layout/style/ServoBindings.toml index 18ed06b669dee..65f9c90534c00 100644 --- a/layout/style/ServoBindings.toml +++ b/layout/style/ServoBindings.toml @@ -519,6 +519,7 @@ cbindgen-types = [ { gecko = "StyleGenericTrackListValue", servo = "values::generics::grid::TrackListValue" }, { gecko = "StyleGenericTrackList", servo = "values::generics::grid::TrackList" }, { gecko = "StyleGenericGridTemplateComponent", servo = "values::generics::grid::GridTemplateComponent" }, + { gecko = "StyleTextEmphasisStyle", servo = "values::computed::text::TextEmphasisStyle" }, ] mapped-generic-types = [ diff --git a/layout/style/nsStyleConsts.h b/layout/style/nsStyleConsts.h index fed9d66d1866b..58647d064f479 100644 --- a/layout/style/nsStyleConsts.h +++ b/layout/style/nsStyleConsts.h @@ -731,24 +731,6 @@ enum class StyleWhiteSpace : uint8_t { NS_STYLE_TEXT_EMPHASIS_POSITION_RIGHT) - - - - - -#define NS_STYLE_TEXT_EMPHASIS_STYLE_NONE 0 -#define NS_STYLE_TEXT_EMPHASIS_STYLE_FILL_MASK (1 << 3) -#define NS_STYLE_TEXT_EMPHASIS_STYLE_FILLED (0 << 3) -#define NS_STYLE_TEXT_EMPHASIS_STYLE_OPEN (1 << 3) -#define NS_STYLE_TEXT_EMPHASIS_STYLE_SHAPE_MASK 7 -#define NS_STYLE_TEXT_EMPHASIS_STYLE_DOT 1 -#define NS_STYLE_TEXT_EMPHASIS_STYLE_CIRCLE 2 -#define NS_STYLE_TEXT_EMPHASIS_STYLE_DOUBLE_CIRCLE 3 -#define NS_STYLE_TEXT_EMPHASIS_STYLE_TRIANGLE 4 -#define NS_STYLE_TEXT_EMPHASIS_STYLE_SESAME 5 -#define NS_STYLE_TEXT_EMPHASIS_STYLE_STRING 255 - - enum class StyleTextRendering : uint8_t { Auto, Optimizespeed, diff --git a/layout/style/nsStyleStruct.cpp b/layout/style/nsStyleStruct.cpp index 836876c31571f..1026be41e01a0 100644 --- a/layout/style/nsStyleStruct.cpp +++ b/layout/style/nsStyleStruct.cpp @@ -3371,7 +3371,6 @@ nsStyleText::nsStyleText(const Document& aDocument) mTextCombineUpright(NS_STYLE_TEXT_COMBINE_UPRIGHT_NONE), mControlCharacterVisibility( nsLayoutUtils::ControlCharVisibilityDefault()), - mTextEmphasisStyle(NS_STYLE_TEXT_EMPHASIS_STYLE_NONE), mTextRendering(StyleTextRendering::Auto), mTextEmphasisColor(StyleColor::CurrentColor()), mWebkitTextFillColor(StyleColor::CurrentColor()), @@ -3384,7 +3383,8 @@ nsStyleText::nsStyleText(const Document& aDocument) mTextIndent(LengthPercentage::Zero()), mTextUnderlineOffset(LengthOrAuto::Auto()), mTextDecorationSkipInk(StyleTextDecorationSkipInk::Auto), - mWebkitTextStrokeWidth(0) { + mWebkitTextStrokeWidth(0), + mTextEmphasisStyle(StyleTextEmphasisStyle::None()) { MOZ_COUNT_CTOR(nsStyleText); RefPtr language = aDocument.GetContentLanguageAsAtomForStyle(); mTextEmphasisPosition = @@ -3410,7 +3410,6 @@ nsStyleText::nsStyleText(const nsStyleText& aSource) mTextCombineUpright(aSource.mTextCombineUpright), mControlCharacterVisibility(aSource.mControlCharacterVisibility), mTextEmphasisPosition(aSource.mTextEmphasisPosition), - mTextEmphasisStyle(aSource.mTextEmphasisStyle), mTextRendering(aSource.mTextRendering), mTextEmphasisColor(aSource.mTextEmphasisColor), mWebkitTextFillColor(aSource.mWebkitTextFillColor), @@ -3424,7 +3423,7 @@ nsStyleText::nsStyleText(const nsStyleText& aSource) mTextDecorationSkipInk(aSource.mTextDecorationSkipInk), mWebkitTextStrokeWidth(aSource.mWebkitTextStrokeWidth), mTextShadow(aSource.mTextShadow), - mTextEmphasisStyleString(aSource.mTextEmphasisStyleString) { + mTextEmphasisStyle(aSource.mTextEmphasisStyle) { MOZ_COUNT_CTOR(nsStyleText); } @@ -3463,8 +3462,8 @@ nsChangeHint nsStyleText::CalcDifference(const nsStyleText& aNewData) const { return NS_STYLE_HINT_REFLOW; } - if (HasTextEmphasis() != aNewData.HasTextEmphasis() || - (HasTextEmphasis() && + if (HasEffectiveTextEmphasis() != aNewData.HasEffectiveTextEmphasis() || + (HasEffectiveTextEmphasis() && mTextEmphasisPosition != aNewData.mTextEmphasisPosition)) { return nsChangeHint_AllReflowHints | nsChangeHint_RepaintFrame; @@ -3482,7 +3481,6 @@ nsChangeHint nsStyleText::CalcDifference(const nsStyleText& aNewData) const { if (mTextShadow != aNewData.mTextShadow || mTextEmphasisStyle != aNewData.mTextEmphasisStyle || - mTextEmphasisStyleString != aNewData.mTextEmphasisStyleString || mWebkitTextStrokeWidth != aNewData.mWebkitTextStrokeWidth) { hint |= nsChangeHint_UpdateSubtreeOverflow | nsChangeHint_SchedulePaint | nsChangeHint_RepaintFrame; diff --git a/layout/style/nsStyleStruct.h b/layout/style/nsStyleStruct.h index 065099076d12b..b9d24ec2c1d08 100644 --- a/layout/style/nsStyleStruct.h +++ b/layout/style/nsStyleStruct.h @@ -1146,7 +1146,6 @@ struct MOZ_NEEDS_MEMMOVABLE_MEMBERS nsStyleText { uint8_t mControlCharacterVisibility; uint8_t mTextEmphasisPosition; - uint8_t mTextEmphasisStyle; mozilla::StyleTextRendering mTextRendering; mozilla::StyleColor mTextEmphasisColor; mozilla::StyleColor mWebkitTextFillColor; @@ -1164,8 +1163,7 @@ struct MOZ_NEEDS_MEMMOVABLE_MEMBERS nsStyleText { nscoord mWebkitTextStrokeWidth; mozilla::StyleArcSlice mTextShadow; - - nsString mTextEmphasisStyleString; + mozilla::StyleTextEmphasisStyle mTextEmphasisStyle; mozilla::StyleWordBreak EffectiveWordBreak() const { if (mWordBreak == mozilla::StyleWordBreak::BreakWord) { @@ -1225,7 +1223,16 @@ struct MOZ_NEEDS_MEMMOVABLE_MEMBERS nsStyleText { owrap == mozilla::StyleOverflowWrap::Anywhere; } - bool HasTextEmphasis() const { return !mTextEmphasisStyleString.IsEmpty(); } + bool HasEffectiveTextEmphasis() const { + if (mTextEmphasisStyle.IsNone()) { + return false; + } + if (mTextEmphasisStyle.IsString() && + mTextEmphasisStyle.AsString().AsString().IsEmpty()) { + return false; + } + return true; + } bool HasWebkitTextStroke() const { return mWebkitTextStrokeWidth > 0; } diff --git a/servo/components/style/properties/gecko.mako.rs b/servo/components/style/properties/gecko.mako.rs index 122ac4d10a5a0..75d73983cccac 100644 --- a/servo/components/style/properties/gecko.mako.rs +++ b/servo/components/style/properties/gecko.mako.rs @@ -2578,97 +2578,15 @@ fn static_assert() { <%self:impl_trait style_struct_name="InheritedText" - skip_longhands="text-align text-emphasis-style - -webkit-text-stroke-width text-emphasis-position"> + skip_longhands="text-align -webkit-text-stroke-width text-emphasis-position"> <% text_align_keyword = Keyword("text-align", "start end left right center justify -moz-center -moz-left -moz-right char", gecko_strip_moz_prefix=False) %> ${impl_keyword('text_align', 'mTextAlign', text_align_keyword)} - fn clear_text_emphasis_style_if_string(&mut self) { - if self.gecko.mTextEmphasisStyle == structs::NS_STYLE_TEXT_EMPHASIS_STYLE_STRING as u8 { - self.gecko.mTextEmphasisStyleString.truncate(); - self.gecko.mTextEmphasisStyle = structs::NS_STYLE_TEXT_EMPHASIS_STYLE_NONE as u8; - } - } - ${impl_simple_type_with_conversion("text_emphasis_position")} - pub fn set_text_emphasis_style(&mut self, v: values::computed::TextEmphasisStyle) { - use crate::values::computed::TextEmphasisStyle; - use crate::values::specified::text::{TextEmphasisFillMode, TextEmphasisShapeKeyword}; - - self.clear_text_emphasis_style_if_string(); - let (te, s) = match v { - TextEmphasisStyle::None => (structs::NS_STYLE_TEXT_EMPHASIS_STYLE_NONE, ""), - TextEmphasisStyle::Keyword { fill, shape } => { - let gecko_fill = match fill { - TextEmphasisFillMode::Filled => structs::NS_STYLE_TEXT_EMPHASIS_STYLE_FILLED, - TextEmphasisFillMode::Open => structs::NS_STYLE_TEXT_EMPHASIS_STYLE_OPEN, - }; - - let gecko_shape = match shape { - TextEmphasisShapeKeyword::Dot => structs::NS_STYLE_TEXT_EMPHASIS_STYLE_DOT, - TextEmphasisShapeKeyword::Circle => structs::NS_STYLE_TEXT_EMPHASIS_STYLE_CIRCLE, - TextEmphasisShapeKeyword::DoubleCircle => structs::NS_STYLE_TEXT_EMPHASIS_STYLE_DOUBLE_CIRCLE, - TextEmphasisShapeKeyword::Triangle => structs::NS_STYLE_TEXT_EMPHASIS_STYLE_TRIANGLE, - TextEmphasisShapeKeyword::Sesame => structs::NS_STYLE_TEXT_EMPHASIS_STYLE_SESAME, - }; - - (gecko_shape | gecko_fill, shape.char(fill)) - }, - TextEmphasisStyle::String(ref s) => { - (structs::NS_STYLE_TEXT_EMPHASIS_STYLE_STRING, &**s) - }, - }; - self.gecko.mTextEmphasisStyleString.assign_str(s); - self.gecko.mTextEmphasisStyle = te as u8; - } - - pub fn copy_text_emphasis_style_from(&mut self, other: &Self) { - self.clear_text_emphasis_style_if_string(); - if other.gecko.mTextEmphasisStyle == structs::NS_STYLE_TEXT_EMPHASIS_STYLE_STRING as u8 { - self.gecko.mTextEmphasisStyleString - .assign(&*other.gecko.mTextEmphasisStyleString) - } - self.gecko.mTextEmphasisStyle = other.gecko.mTextEmphasisStyle; - } - - pub fn reset_text_emphasis_style(&mut self, other: &Self) { - self.copy_text_emphasis_style_from(other) - } - - pub fn clone_text_emphasis_style(&self) -> values::computed::TextEmphasisStyle { - use crate::values::computed::TextEmphasisStyle; - use crate::values::specified::text::{TextEmphasisFillMode, TextEmphasisShapeKeyword}; - - if self.gecko.mTextEmphasisStyle == structs::NS_STYLE_TEXT_EMPHASIS_STYLE_NONE as u8 { - return TextEmphasisStyle::None; - } - - if self.gecko.mTextEmphasisStyle == structs::NS_STYLE_TEXT_EMPHASIS_STYLE_STRING as u8 { - return TextEmphasisStyle::String(self.gecko.mTextEmphasisStyleString.to_string().into()); - } - - let fill = - self.gecko.mTextEmphasisStyle & structs::NS_STYLE_TEXT_EMPHASIS_STYLE_OPEN as u8 == 0; - - let fill = if fill { TextEmphasisFillMode::Filled } else { TextEmphasisFillMode::Open }; - - let shape = - match self.gecko.mTextEmphasisStyle as u32 & !structs::NS_STYLE_TEXT_EMPHASIS_STYLE_OPEN { - structs::NS_STYLE_TEXT_EMPHASIS_STYLE_DOT => TextEmphasisShapeKeyword::Dot, - structs::NS_STYLE_TEXT_EMPHASIS_STYLE_CIRCLE => TextEmphasisShapeKeyword::Circle, - structs::NS_STYLE_TEXT_EMPHASIS_STYLE_DOUBLE_CIRCLE => TextEmphasisShapeKeyword::DoubleCircle, - structs::NS_STYLE_TEXT_EMPHASIS_STYLE_TRIANGLE => TextEmphasisShapeKeyword::Triangle, - structs::NS_STYLE_TEXT_EMPHASIS_STYLE_SESAME => TextEmphasisShapeKeyword::Sesame, - _ => panic!("Unexpected value in style struct for text-emphasis-style property") - }; - - TextEmphasisStyle::Keyword { fill, shape } - } - ${impl_non_negative_length('_webkit_text_stroke_width', 'mWebkitTextStrokeWidth')} diff --git a/servo/components/style/values/computed/text.rs b/servo/components/style/values/computed/text.rs index 9b767c8da6466..f6133e570769e 100644 --- a/servo/components/style/values/computed/text.rs +++ b/servo/components/style/values/computed/text.rs @@ -195,8 +195,11 @@ impl TextDecorationsInEffect { } + +/// cbindgen:derive-tagged-enum-copy-constructor=true #[derive(Clone, Debug, MallocSizeOf, PartialEq, ToCss, ToResolvedValue)] #[allow(missing_docs)] +#[repr(C, u8)] pub enum TextEmphasisStyle { Keyword { diff --git a/servo/components/style/values/specified/text.rs b/servo/components/style/values/specified/text.rs index e143db0af2767..06baf8c4868b0 100644 --- a/servo/components/style/values/specified/text.rs +++ b/servo/components/style/values/specified/text.rs @@ -671,6 +671,7 @@ pub enum TextEmphasisStyle { #[derive(Clone, Copy, Debug, MallocSizeOf, Parse, PartialEq, SpecifiedValueInfo, ToCss, ToShmem)] +#[repr(u8)] pub enum TextEmphasisFillMode { Filled, @@ -690,6 +691,7 @@ impl TextEmphasisFillMode { #[derive( Clone, Copy, Debug, Eq, MallocSizeOf, Parse, PartialEq, SpecifiedValueInfo, ToCss, ToShmem, )] +#[repr(u8)] pub enum TextEmphasisShapeKeyword { Dot, @@ -703,50 +705,6 @@ pub enum TextEmphasisShapeKeyword { Sesame, } -impl TextEmphasisShapeKeyword { - - pub fn char(&self, fill: TextEmphasisFillMode) -> &'static str { - let fill = fill == TextEmphasisFillMode::Filled; - match *self { - TextEmphasisShapeKeyword::Dot => { - if fill { - "\u{2022}" - } else { - "\u{25e6}" - } - }, - TextEmphasisShapeKeyword::Circle => { - if fill { - "\u{25cf}" - } else { - "\u{25cb}" - } - }, - TextEmphasisShapeKeyword::DoubleCircle => { - if fill { - "\u{25c9}" - } else { - "\u{25ce}" - } - }, - TextEmphasisShapeKeyword::Triangle => { - if fill { - "\u{25b2}" - } else { - "\u{25b3}" - } - }, - TextEmphasisShapeKeyword::Sesame => { - if fill { - "\u{fe45}" - } else { - "\u{fe46}" - } - }, - } - } -} - impl ToComputedValue for TextEmphasisStyle { type ComputedValue = ComputedTextEmphasisStyle; diff --git a/servo/ports/geckolib/cbindgen.toml b/servo/ports/geckolib/cbindgen.toml index a845dd1e6b5ce..b0b507333988d 100644 --- a/servo/ports/geckolib/cbindgen.toml +++ b/servo/ports/geckolib/cbindgen.toml @@ -165,6 +165,7 @@ include = [ "SVGPaint", "SVGPaintKind", "GridTemplateComponent", + "TextEmphasisStyle", ] item_types = ["enums", "structs", "typedefs", "functions", "constants"] renaming_overrides_prefixing = true @@ -677,3 +678,12 @@ renaming_overrides_prefixing = true inline Span> LineNameLists(bool aIsSubgrid) const; inline Span> TrackListValues() const; """ + +"TextEmphasisStyle" = """ + private: + // Private default constructor without initialization so that the helper + // constructor functions still work as expected. They take care of + // initializing the fields properly. + StyleTextEmphasisStyle() {} + public: +"""