From 977a6275d16e2399ae227d9f142d4465f189b493 Mon Sep 17 00:00:00 2001 From: Ting-Yu Lin Date: Thu, 19 Sep 2019 02:37:14 +0000 Subject: [PATCH] Bug 1582224 - Split SIDEWAYS bit in WritingMode. r=jfkthame,emilio Currently, there's no way to tell whether the SIDEWAYS bit is set from `writing-mode:sideways-*` or `writing-mode:vertical-*; text-orientation:sideways;`. To be able to tell them apart, split SIDEWAYS bits into VERTICAL_SIDEWAYS and TEXT_SIDEWAYS. This is needed by my proposed solution in bug 1102175. Also, provide convenience methods related to sideways writing-mode, and replace obscure checks in the codebase. Note that we don't have the use cases to distinguish vertical-rl from sideways-rl in layout, but for the completeness, IsSidewaysLR() is still defined. Differential Revision: https://phabricator.services.mozilla.com/D46321 --HG-- extra : moz-landing-system : lando --- layout/base/nsCaret.cpp | 3 +- layout/generic/WritingModes.h | 41 +++++++++++++++++----- layout/generic/nsFloatManager.cpp | 8 ++--- servo/components/style/logical_geometry.rs | 26 ++++++++------ servo/tests/unit/style/logical_geometry.rs | 16 +++++---- 5 files changed, 62 insertions(+), 32 deletions(-) diff --git a/layout/base/nsCaret.cpp b/layout/base/nsCaret.cpp index 7c21945d3bee..23e919424981 100644 --- a/layout/base/nsCaret.cpp +++ b/layout/base/nsCaret.cpp @@ -878,8 +878,7 @@ void nsCaret::ComputeCaretRects(nsIFrame* aFrame, int32_t aFrameOffset, // right The height of the hook rectangle is the same as the width of the // caret rectangle. if (isVertical) { - bool isSidewaysLR = wm.IsVerticalLR() && !wm.IsLineInverted(); - if (isSidewaysLR) { + if (wm.IsSidewaysLR()) { aHookRect->SetRect(aCaretRect->x + bidiIndicatorSize, aCaretRect->y + (!isCaretRTL ? bidiIndicatorSize * -1 : aCaretRect->height), diff --git a/layout/generic/WritingModes.h b/layout/generic/WritingModes.h index 3f19f9b4d814..1a8d08b191bb 100644 --- a/layout/generic/WritingModes.h +++ b/layout/generic/WritingModes.h @@ -27,7 +27,8 @@ // (In some cases, there are internal (private) methods that don't do this; // such methods should only be used by other methods that have already checked // the writing modes.) -// The check ignores the StyleWritingMode_SIDEWAYS bit of writing mode, because +// The check ignores the StyleWritingMode_VERTICAL_SIDEWAYS and +// StyleWritingMode_TEXT_SIDEWAYS bit of writing mode, because // this does not affect the interpretation of logical coordinates. #define CHECK_WRITING_MODE(param) \ @@ -248,21 +249,45 @@ class WritingMode { } /** - * True if the text-orientation will force all text to be rendered sideways - * in vertical lines, in which case we should prefer an alphabetic baseline; - * otherwise, the default is centered. + * True if vertical sideways writing mode, i.e. when + * writing-mode: sideways-lr | sideways-rl. + */ + bool IsVerticalSideways() const { + return !!(mWritingMode & StyleWritingMode_VERTICAL_SIDEWAYS); + } + + /** + * True if this is writing-mode: sideways-rl (convenience method). + */ + bool IsSidewaysRL() const { return IsVerticalRL() && IsVerticalSideways(); } + + /** + * True if this is writing-mode: sideways-lr (convenience method). + */ + bool IsSidewaysLR() const { return IsVerticalLR() && IsVerticalSideways(); } + + /** + * True if either text-orientation or writing-mode will force all text to be + * rendered sideways in vertical lines, in which case we should prefer an + * alphabetic baseline; otherwise, the default is centered. + * * Note that some glyph runs may be rendered sideways even if this is false, * due to text-orientation:mixed resolution, but in that case the dominant * baseline remains centered. */ bool IsSideways() const { - return !!(mWritingMode & StyleWritingMode_SIDEWAYS); + return !!(mWritingMode & (StyleWritingMode_VERTICAL_SIDEWAYS | + StyleWritingMode_TEXT_SIDEWAYS)); } -#ifdef DEBUG // Used by CHECK_WRITING_MODE to compare modes without regard - // for the StyleWritingMode_SIDEWAYS flag. +#ifdef DEBUG + // Used by CHECK_WRITING_MODE to compare modes without regard for the + // StyleWritingMode_VERTICAL_SIDEWAYS or StyleWritingMode_TEXT_SIDEWAYS flags. WritingMode IgnoreSideways() const { - return WritingMode(mWritingMode.bits & ~StyleWritingMode_SIDEWAYS.bits); + return WritingMode( + mWritingMode.bits & + ~(StyleWritingMode_VERTICAL_SIDEWAYS | StyleWritingMode_TEXT_SIDEWAYS) + .bits); } #endif diff --git a/layout/generic/nsFloatManager.cpp b/layout/generic/nsFloatManager.cpp index 28f2ef9fd9a2..f0ff42df236a 100644 --- a/layout/generic/nsFloatManager.cpp +++ b/layout/generic/nsFloatManager.cpp @@ -2202,12 +2202,8 @@ void nsFloatManager::ImageShapeInfo::CreateInterval( // constructed. We add 1 to aB to capture the end of the block axis pixel. origin.MoveBy(aIMin * aAppUnitsPerDevPixel, (aB + 1) * -aAppUnitsPerDevPixel); - } else if (aWM.IsVerticalLR() && !aWM.IsLineInverted()) { - // sideways-lr. - // Checking IsLineInverted is the only reliable way to distinguish - // vertical-lr from sideways-lr. IsSideways and IsInlineReversed are both - // affected by bidi and text-direction, and so complicate detection. - // These writing modes proceed from the bottom left, and each interval + } else if (aWM.IsSidewaysLR()) { + // This writing mode proceeds from the bottom left, and each interval // moves in a negative inline direction and a positive block direction. // We add 1 to aIMax to capture the end of the inline axis pixel. origin.MoveBy((aIMax + 1) * -aAppUnitsPerDevPixel, diff --git a/servo/components/style/logical_geometry.rs b/servo/components/style/logical_geometry.rs index dd912da4af9d..cad7e1d001bd 100644 --- a/servo/components/style/logical_geometry.rs +++ b/servo/components/style/logical_geometry.rs @@ -50,16 +50,22 @@ bitflags!( const LINE_INVERTED = 1 << 3; /// direction is rtl. const RTL = 1 << 4; - /// Horizontal text within a vertical writing mode is displayed sideways + /// All text within a vertical writing mode is displayed sideways /// and runs top-to-bottom or bottom-to-top; set in these cases: /// - /// * writing-mode: vertical-rl; text-orientation: sideways; - /// * writing-mode: vertical-lr; text-orientation: sideways; /// * writing-mode: sideways-rl; /// * writing-mode: sideways-lr; /// /// Never set without VERTICAL. - const SIDEWAYS = 1 << 5; + const VERTICAL_SIDEWAYS = 1 << 5; + /// Similar to VERTICAL_SIDEWAYS, but is set via text-orientation; + /// set in these cases: + /// + /// * writing-mode: vertical-rl; text-orientation: sideways; + /// * writing-mode: vertical-lr; text-orientation: sideways; + /// + /// Never set without VERTICAL. + const TEXT_SIDEWAYS = 1 << 6; /// Horizontal text within a vertical writing mode is displayed with each /// glyph upright; set in these cases: /// @@ -67,7 +73,7 @@ bitflags!( /// * writing-mode: vertical-lr: text-orientation: upright; /// /// Never set without VERTICAL. - const UPRIGHT = 1 << 6; + const UPRIGHT = 1 << 7; } ); @@ -112,7 +118,7 @@ impl WritingMode { #[cfg(feature = "gecko")] SpecifiedWritingMode::SidewaysRl => { flags.insert(WritingMode::VERTICAL); - flags.insert(WritingMode::SIDEWAYS); + flags.insert(WritingMode::VERTICAL_SIDEWAYS); if direction == Direction::Rtl { flags.insert(WritingMode::INLINE_REVERSED); } @@ -121,7 +127,7 @@ impl WritingMode { SpecifiedWritingMode::SidewaysLr => { flags.insert(WritingMode::VERTICAL); flags.insert(WritingMode::VERTICAL_LR); - flags.insert(WritingMode::SIDEWAYS); + flags.insert(WritingMode::VERTICAL_SIDEWAYS); if direction == Direction::Ltr { flags.insert(WritingMode::INLINE_REVERSED); } @@ -151,7 +157,7 @@ impl WritingMode { flags.remove(WritingMode::INLINE_REVERSED); }, TextOrientation::Sideways => { - flags.insert(WritingMode::SIDEWAYS); + flags.insert(WritingMode::TEXT_SIDEWAYS); }, } }, @@ -187,7 +193,7 @@ impl WritingMode { #[inline] pub fn is_sideways(&self) -> bool { - self.intersects(WritingMode::SIDEWAYS) + self.intersects(WritingMode::VERTICAL_SIDEWAYS | WritingMode::TEXT_SIDEWAYS) } #[inline] @@ -325,7 +331,7 @@ impl fmt::Display for WritingMode { } else { write!(formatter, " RL")?; } - if self.intersects(WritingMode::SIDEWAYS) { + if self.is_sideways() { write!(formatter, " Sideways")?; } if self.intersects(WritingMode::LINE_INVERTED) { diff --git a/servo/tests/unit/style/logical_geometry.rs b/servo/tests/unit/style/logical_geometry.rs index 8d4e1953284c..8d6c06ec9aec 100644 --- a/servo/tests/unit/style/logical_geometry.rs +++ b/servo/tests/unit/style/logical_geometry.rs @@ -6,20 +6,24 @@ use euclid::{Size2D, Point2D, SideOffsets2D, Rect}; use style::logical_geometry::{WritingMode, LogicalSize, LogicalPoint, LogicalMargin, LogicalRect}; #[cfg(test)] -fn modes() -> [WritingMode; 13] { - [ +fn modes() -> Vec { + vec![ WritingMode::empty(), WritingMode::VERTICAL, WritingMode::VERTICAL | WritingMode::VERTICAL_LR, - WritingMode::VERTICAL | WritingMode::VERTICAL_LR | WritingMode::SIDEWAYS, - WritingMode::VERTICAL | WritingMode::SIDEWAYS, + WritingMode::VERTICAL | WritingMode::VERTICAL_LR | WritingMode::VERTICAL_SIDEWAYS, + WritingMode::VERTICAL | WritingMode::VERTICAL_LR | WritingMode::TEXT_SIDEWAYS, + WritingMode::VERTICAL | WritingMode::VERTICAL_SIDEWAYS, + WritingMode::VERTICAL | WritingMode::TEXT_SIDEWAYS, WritingMode::VERTICAL | WritingMode::UPRIGHT, WritingMode::RTL, WritingMode::VERTICAL | WritingMode::RTL, WritingMode::VERTICAL | WritingMode::VERTICAL_LR | WritingMode::RTL, - WritingMode::VERTICAL | WritingMode::VERTICAL_LR | WritingMode::SIDEWAYS | WritingMode::RTL, + WritingMode::VERTICAL | WritingMode::VERTICAL_LR | WritingMode::VERTICAL_SIDEWAYS | WritingMode::RTL, + WritingMode::VERTICAL | WritingMode::VERTICAL_LR | WritingMode::TEXT_SIDEWAYS | WritingMode::RTL, WritingMode::VERTICAL | WritingMode::VERTICAL_LR | WritingMode::UPRIGHT | WritingMode::RTL, - WritingMode::VERTICAL | WritingMode::SIDEWAYS | WritingMode::RTL, + WritingMode::VERTICAL | WritingMode::VERTICAL_SIDEWAYS | WritingMode::RTL, + WritingMode::VERTICAL | WritingMode::TEXT_SIDEWAYS | WritingMode::RTL, WritingMode::VERTICAL | WritingMode::UPRIGHT | WritingMode::RTL, ] }