Title: [233441] trunk/Source/WebCore
- Revision
- 233441
- Author
- [email protected]
- Date
- 2018-07-02 15:27:42 -0700 (Mon, 02 Jul 2018)
Log Message
Refactor InlineTextBox::emphasisMarkExistsAndIsAbove()
<https://webkit.org/b/187204>
Reviewed by Darin Adler.
No new tests since there is no change in behavior.
Refactor emphasisMarkExistsAndIsAbove() to return a
std::optional<bool> instead of returning a bool and taking a
std::optional<bool> argument. The state returned is now:
- std::nullopt => emphasis mark doesn't exist or is suppressed.
- false => emphasis mark exists and is not suppressed, but is not above.
- true => emphasis mark exists and is not suppressed, and is above.
* rendering/InlineFlowBox.cpp:
(WebCore::InlineFlowBox::placeBoxesInBlockDirection):
(WebCore::InlineFlowBox::addTextBoxVisualOverflow):
(WebCore::InlineFlowBox::computeOverAnnotationAdjustment const):
(WebCore::InlineFlowBox::computeUnderAnnotationAdjustment const):
- Update for refactored method. Remove some redundant checks
for TextEmphasisMark::None that already happen in
emphasisMarkExistsAndIsAbove().
* rendering/InlineTextBox.cpp:
(WebCore::emphasisPositionHasNeitherLeftNorRight): Delete.
- Replaced by an OptionSet<TextEmphasisPosition>.
(WebCore::InlineTextBox::emphasisMarkExistsAndIsAbove const):
- Refactor as described above.
(WebCore::InlineTextBox::paintMarkedTextForeground):
- Update for refactored method.
* rendering/InlineTextBox.h:
(WebCore::InlineTextBox::emphasisMarkExistsAndIsAbove const):
- Update for new method signature.
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (233440 => 233441)
--- trunk/Source/WebCore/ChangeLog 2018-07-02 22:20:46 UTC (rev 233440)
+++ trunk/Source/WebCore/ChangeLog 2018-07-02 22:27:42 UTC (rev 233441)
@@ -1,3 +1,38 @@
+2018-07-02 David Kilzer <[email protected]>
+
+ Refactor InlineTextBox::emphasisMarkExistsAndIsAbove()
+ <https://webkit.org/b/187204>
+
+ Reviewed by Darin Adler.
+
+ No new tests since there is no change in behavior.
+
+ Refactor emphasisMarkExistsAndIsAbove() to return a
+ std::optional<bool> instead of returning a bool and taking a
+ std::optional<bool> argument. The state returned is now:
+ - std::nullopt => emphasis mark doesn't exist or is suppressed.
+ - false => emphasis mark exists and is not suppressed, but is not above.
+ - true => emphasis mark exists and is not suppressed, and is above.
+
+ * rendering/InlineFlowBox.cpp:
+ (WebCore::InlineFlowBox::placeBoxesInBlockDirection):
+ (WebCore::InlineFlowBox::addTextBoxVisualOverflow):
+ (WebCore::InlineFlowBox::computeOverAnnotationAdjustment const):
+ (WebCore::InlineFlowBox::computeUnderAnnotationAdjustment const):
+ - Update for refactored method. Remove some redundant checks
+ for TextEmphasisMark::None that already happen in
+ emphasisMarkExistsAndIsAbove().
+ * rendering/InlineTextBox.cpp:
+ (WebCore::emphasisPositionHasNeitherLeftNorRight): Delete.
+ - Replaced by an OptionSet<TextEmphasisPosition>.
+ (WebCore::InlineTextBox::emphasisMarkExistsAndIsAbove const):
+ - Refactor as described above.
+ (WebCore::InlineTextBox::paintMarkedTextForeground):
+ - Update for refactored method.
+ * rendering/InlineTextBox.h:
+ (WebCore::InlineTextBox::emphasisMarkExistsAndIsAbove const):
+ - Update for new method signature.
+
2018-07-02 Megan Gardner <[email protected]>
Enable copy paste on iOS apps for Mac
Modified: trunk/Source/WebCore/rendering/InlineFlowBox.cpp (233440 => 233441)
--- trunk/Source/WebCore/rendering/InlineFlowBox.cpp 2018-07-02 22:20:46 UTC (rev 233440)
+++ trunk/Source/WebCore/rendering/InlineFlowBox.cpp 2018-07-02 22:27:42 UTC (rev 233441)
@@ -698,9 +698,8 @@
}
}
if (is<InlineTextBox>(*child)) {
- std::optional<bool> emphasisMarkIsAbove;
- if (downcast<InlineTextBox>(*child).emphasisMarkExistsAndIsAbove(childLineStyle, emphasisMarkIsAbove)) {
- if (emphasisMarkIsAbove && (*emphasisMarkIsAbove != childLineStyle.isFlippedLinesWritingMode()))
+ if (std::optional<bool> markExistsAndIsAbove = downcast<InlineTextBox>(*child).emphasisMarkExistsAndIsAbove(childLineStyle)) {
+ if (*markExistsAndIsAbove != childLineStyle.isFlippedLinesWritingMode())
hasAnnotationsBefore = true;
else
hasAnnotationsAfter = true;
@@ -894,10 +893,9 @@
int leftGlyphOverflow = -strokeOverflow - leftGlyphEdge;
int rightGlyphOverflow = strokeOverflow + rightGlyphEdge;
- std::optional<bool> emphasisMarkIsAbove;
- if (lineStyle.textEmphasisMark() != TextEmphasisMark::None && textBox.emphasisMarkExistsAndIsAbove(lineStyle, emphasisMarkIsAbove)) {
+ if (std::optional<bool> markExistsAndIsAbove = textBox.emphasisMarkExistsAndIsAbove(lineStyle)) {
int emphasisMarkHeight = lineStyle.fontCascade().emphasisMarkHeight(lineStyle.textEmphasisMarkString());
- if (emphasisMarkIsAbove && (*emphasisMarkIsAbove == !lineStyle.isFlippedLinesWritingMode()))
+ if (*markExistsAndIsAbove == !lineStyle.isFlippedLinesWritingMode())
topGlyphOverflow = std::min(topGlyphOverflow, -emphasisMarkHeight);
else
bottomGlyphOverflow = std::max(bottomGlyphOverflow, emphasisMarkHeight);
@@ -1575,8 +1573,8 @@
if (is<InlineTextBox>(*child)) {
const RenderStyle& childLineStyle = child->lineStyle();
- std::optional<bool> emphasisMarkIsAbove;
- if (childLineStyle.textEmphasisMark() != TextEmphasisMark::None && downcast<InlineTextBox>(*child).emphasisMarkExistsAndIsAbove(childLineStyle, emphasisMarkIsAbove) && emphasisMarkIsAbove && *emphasisMarkIsAbove) {
+ std::optional<bool> markExistsAndIsAbove = downcast<InlineTextBox>(*child).emphasisMarkExistsAndIsAbove(childLineStyle);
+ if (markExistsAndIsAbove && *markExistsAndIsAbove) {
if (!childLineStyle.isFlippedLinesWritingMode()) {
int topOfEmphasisMark = child->logicalTop() - childLineStyle.fontCascade().emphasisMarkHeight(childLineStyle.textEmphasisMarkString());
result = std::max(result, allowedPosition - topOfEmphasisMark);
@@ -1623,9 +1621,8 @@
if (is<InlineTextBox>(*child)) {
const RenderStyle& childLineStyle = child->lineStyle();
- std::optional<bool> emphasisMarkIsAbove;
- downcast<InlineTextBox>(*child).emphasisMarkExistsAndIsAbove(childLineStyle, emphasisMarkIsAbove);
- if (childLineStyle.textEmphasisMark() != TextEmphasisMark::None && emphasisMarkIsAbove && !*emphasisMarkIsAbove) {
+ std::optional<bool> markExistsAndIsAbove = downcast<InlineTextBox>(*child).emphasisMarkExistsAndIsAbove(childLineStyle);
+ if (markExistsAndIsAbove && !*markExistsAndIsAbove) {
if (!childLineStyle.isFlippedLinesWritingMode()) {
LayoutUnit bottomOfEmphasisMark = child->logicalBottom() + childLineStyle.fontCascade().emphasisMarkHeight(childLineStyle.textEmphasisMarkString());
result = std::max(result, bottomOfEmphasisMark - allowedPosition);
Modified: trunk/Source/WebCore/rendering/InlineTextBox.cpp (233440 => 233441)
--- trunk/Source/WebCore/rendering/InlineTextBox.cpp 2018-07-02 22:20:46 UTC (rev 233440)
+++ trunk/Source/WebCore/rendering/InlineTextBox.cpp 2018-07-02 22:27:42 UTC (rev 233441)
@@ -353,43 +353,45 @@
return false;
}
-static inline bool emphasisPositionHasNeitherLeftNorRight(OptionSet<TextEmphasisPosition> emphasisPosition)
+std::optional<bool> InlineTextBox::emphasisMarkExistsAndIsAbove(const RenderStyle& style) const
{
- return !(emphasisPosition & TextEmphasisPosition::Left) && !(emphasisPosition & TextEmphasisPosition::Right);
-}
-
-bool InlineTextBox::emphasisMarkExistsAndIsAbove(const RenderStyle& style, std::optional<bool>& above) const
-{
// This function returns true if there are text emphasis marks and they are suppressed by ruby text.
if (style.textEmphasisMark() == TextEmphasisMark::None)
- return false;
+ return std::nullopt;
+ const OptionSet<TextEmphasisPosition> horizontalMask { TextEmphasisPosition::Left, TextEmphasisPosition::Right };
+
auto emphasisPosition = style.textEmphasisPosition();
+ auto emphasisPositionHorizontalValue = emphasisPosition & horizontalMask;
ASSERT(!((emphasisPosition & TextEmphasisPosition::Over) && (emphasisPosition & TextEmphasisPosition::Under)));
- ASSERT(!((emphasisPosition & TextEmphasisPosition::Left) && (emphasisPosition & TextEmphasisPosition::Right)));
-
- if (emphasisPositionHasNeitherLeftNorRight(emphasisPosition))
- above = emphasisPosition.contains(TextEmphasisPosition::Over);
+ ASSERT(emphasisPositionHorizontalValue != horizontalMask);
+
+ bool isAbove = false;
+ if (!emphasisPositionHorizontalValue)
+ isAbove = emphasisPosition.contains(TextEmphasisPosition::Over);
else if (style.isHorizontalWritingMode())
- above = emphasisPosition.contains(TextEmphasisPosition::Over);
+ isAbove = emphasisPosition.contains(TextEmphasisPosition::Over);
else
- above = emphasisPosition.contains(TextEmphasisPosition::Right);
-
+ isAbove = emphasisPositionHorizontalValue == TextEmphasisPosition::Right;
+
if ((style.isHorizontalWritingMode() && (emphasisPosition & TextEmphasisPosition::Under))
|| (!style.isHorizontalWritingMode() && (emphasisPosition & TextEmphasisPosition::Left)))
- return true; // Ruby text is always over, so it cannot suppress emphasis marks under.
+ return isAbove; // Ruby text is always over, so it cannot suppress emphasis marks under.
RenderBlock* containingBlock = renderer().containingBlock();
if (!containingBlock->isRubyBase())
- return true; // This text is not inside a ruby base, so it does not have ruby text over it.
+ return isAbove; // This text is not inside a ruby base, so it does not have ruby text over it.
if (!is<RenderRubyRun>(*containingBlock->parent()))
- return true; // Cannot get the ruby text.
+ return isAbove; // Cannot get the ruby text.
RenderRubyText* rubyText = downcast<RenderRubyRun>(*containingBlock->parent()).rubyText();
// The emphasis marks over are suppressed only if there is a ruby text box and it not empty.
- return !rubyText || !rubyText->hasLines();
+ if (rubyText && rubyText->hasLines())
+ return std::nullopt;
+
+ return isAbove;
}
struct InlineTextBox::MarkedTextStyle {
@@ -1012,11 +1014,10 @@
const RenderStyle& lineStyle = this->lineStyle();
float emphasisMarkOffset = 0;
- std::optional<bool> emphasisMarkAbove;
- bool hasTextEmphasis = emphasisMarkExistsAndIsAbove(lineStyle, emphasisMarkAbove);
- const AtomicString& emphasisMark = hasTextEmphasis ? lineStyle.textEmphasisMarkString() : nullAtom();
+ std::optional<bool> markExistsAndIsAbove = emphasisMarkExistsAndIsAbove(lineStyle);
+ const AtomicString& emphasisMark = markExistsAndIsAbove ? lineStyle.textEmphasisMarkString() : nullAtom();
if (!emphasisMark.isEmpty())
- emphasisMarkOffset = (emphasisMarkAbove && *emphasisMarkAbove) ? -font.fontMetrics().ascent() - font.emphasisMarkDescent(emphasisMark) : font.fontMetrics().descent() + font.emphasisMarkAscent(emphasisMark);
+ emphasisMarkOffset = *markExistsAndIsAbove ? -font.fontMetrics().ascent() - font.emphasisMarkDescent(emphasisMark) : font.fontMetrics().descent() + font.emphasisMarkAscent(emphasisMark);
TextPainter textPainter { context };
textPainter.setFont(font);
Modified: trunk/Source/WebCore/rendering/InlineTextBox.h (233440 => 233441)
--- trunk/Source/WebCore/rendering/InlineTextBox.h 2018-07-02 22:20:46 UTC (rev 233440)
+++ trunk/Source/WebCore/rendering/InlineTextBox.h 2018-07-02 22:27:42 UTC (rev 233441)
@@ -87,7 +87,7 @@
int baselinePosition(FontBaseline) const final;
LayoutUnit lineHeight() const final;
- bool emphasisMarkExistsAndIsAbove(const RenderStyle&, std::optional<bool>& isAbove) const;
+ std::optional<bool> emphasisMarkExistsAndIsAbove(const RenderStyle&) const;
LayoutRect logicalOverflowRect() const;
void setLogicalOverflowRect(const LayoutRect&);
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
