Skip to content

Commit

Permalink
Convert gfx::BaselineStyle enum to enum class
Browse files Browse the repository at this point in the history
Bug: None
Change-Id: Icecd89961e98495b79254000b5b89323bb45ad90
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4751001
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1182563}
  • Loading branch information
Yogesh authored and Chromium LUCI CQ committed Aug 11, 2023
1 parent 4d21b75 commit 19a77d4
Show file tree
Hide file tree
Showing 7 changed files with 41 additions and 31 deletions.
1 change: 1 addition & 0 deletions AUTHORS
Original file line number Diff line number Diff line change
Expand Up @@ -1422,6 +1422,7 @@ Yi Zhang <yi.y.zhang@intel.com>
Yizhou Jiang <yizhou.jiang@intel.com>
Yoav Weiss <yoav@yoav.ws>
Yoav Zilberberg <yoav.zilberberg@gmail.com>
Yogesh <yogesh.dabas@samsung.com>
Yoichiro Hibara <hibarayoichiro871@gmail.com>
Yong Ling <yongling@tencent.com>
Yong Shin <sy3620@gmail.com>
Expand Down
6 changes: 3 additions & 3 deletions chrome/browser/ui/views/omnibox/omnibox_text_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ struct TextStyle {
int touchable_size_delta = 0;

// The baseline shift. Ignored under touch (text is always baseline-aligned).
gfx::BaselineStyle baseline = gfx::NORMAL_BASELINE;
gfx::BaselineStyle baseline = gfx::BaselineStyle::kNormalBaseline;
};

// The new answer layout has separate and different treatment of text styles,
Expand All @@ -77,8 +77,8 @@ void ApplyTextStyleForType(SuggestionAnswer::TextStyle text_style,

const gfx::BaselineStyle baseline =
(text_style == SuggestionAnswer::TextStyle::SUPERIOR)
? gfx::SUPERIOR
: gfx::NORMAL_BASELINE;
? gfx::BaselineStyle::kSuperior
: gfx::BaselineStyle::kNormalBaseline;
render_text->ApplyBaselineStyle(baseline, range);

const bool selected =
Expand Down
2 changes: 1 addition & 1 deletion ui/gfx/render_text.h
Original file line number Diff line number Diff line change
Expand Up @@ -949,7 +949,7 @@ class GFX_EXPORT RenderText {
// BreakList positions are stored with text indices, not display indices.
// TODO(msw): Expand to support cursor, selection, background, etc. colors.
BreakList<SkColor> colors_{kPlaceholderColor};
BreakList<BaselineStyle> baselines_{NORMAL_BASELINE};
BreakList<BaselineStyle> baselines_{BaselineStyle::kNormalBaseline};
BreakList<int> font_size_overrides_{0};
BreakList<Font::Weight> weights_{Font::Weight::NORMAL};
internal::StyleArray styles_;
Expand Down
10 changes: 5 additions & 5 deletions ui/gfx/render_text_harfbuzz.cc
Original file line number Diff line number Diff line change
Expand Up @@ -899,23 +899,23 @@ void TextRunHarfBuzz::FontParams::
if (font_size == 0)
font_size = font.GetFontSize();
baseline_offset = 0;
if (baseline_type != NORMAL_BASELINE) {
if (baseline_type != BaselineStyle::kNormalBaseline) {
// Calculate a slightly smaller font. The ratio here is somewhat arbitrary.
// Proportions from 5/9 to 5/7 all look pretty good.
const float ratio = 5.0f / 9.0f;
font_size = base::ClampRound(font.GetFontSize() * ratio);
switch (baseline_type) {
case SUPERSCRIPT:
case BaselineStyle::kSuperscript:
baseline_offset = font.GetCapHeight() - font.GetHeight();
break;
case SUPERIOR:
case BaselineStyle::kSuperior:
baseline_offset =
base::ClampRound(font.GetCapHeight() * ratio) - font.GetCapHeight();
break;
case SUBSCRIPT:
case BaselineStyle::kSubscript:
baseline_offset = font.GetHeight() - font.GetBaseline();
break;
case INFERIOR: // Fall through.
case BaselineStyle::kInferior: // Fall through.
default:
break;
}
Expand Down
2 changes: 1 addition & 1 deletion ui/gfx/render_text_harfbuzz.h
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ struct GFX_EXPORT TextRunHarfBuzz {
Font::Weight weight = Font::Weight::NORMAL;
int font_size = 0;
int baseline_offset = 0;
int baseline_type = 0;
BaselineStyle baseline_type = BaselineStyle::kNormalBaseline;
bool italic = false;
bool strike = false;
bool underline = false;
Expand Down
39 changes: 24 additions & 15 deletions ui/gfx/render_text_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -678,7 +678,8 @@ TEST_F(RenderTextTest, DefaultStyles) {
const char16_t* const cases[] = {kWeak, kLtr, u"Hello", kRtl, u"", u""};
for (size_t i = 0; i < std::size(cases); ++i) {
EXPECT_TRUE(test_api()->colors().EqualsValueForTesting(kPlaceholderColor));
EXPECT_TRUE(test_api()->baselines().EqualsValueForTesting(NORMAL_BASELINE));
EXPECT_TRUE(test_api()->baselines().EqualsValueForTesting(
BaselineStyle::kNormalBaseline));
EXPECT_TRUE(test_api()->font_size_overrides().EqualsValueForTesting(0));
for (size_t style = 0; style < static_cast<int>(TEXT_STYLE_COUNT); ++style)
EXPECT_TRUE(test_api()->styles()[style].EqualsValueForTesting(false));
Expand All @@ -691,13 +692,14 @@ TEST_F(RenderTextTest, SetStyles) {
RenderText* render_text = GetRenderText();
const SkColor color = SK_ColorGREEN;
render_text->SetColor(color);
render_text->SetBaselineStyle(SUPERSCRIPT);
render_text->SetBaselineStyle(BaselineStyle::kSuperscript);
render_text->SetWeight(Font::Weight::BOLD);
render_text->SetStyle(TEXT_STYLE_UNDERLINE, false);
const char16_t* const cases[] = {kWeak, kLtr, u"Hello", kRtl, u"", u""};
for (size_t i = 0; i < std::size(cases); ++i) {
EXPECT_TRUE(test_api()->colors().EqualsValueForTesting(color));
EXPECT_TRUE(test_api()->baselines().EqualsValueForTesting(SUPERSCRIPT));
EXPECT_TRUE(test_api()->baselines().EqualsValueForTesting(
BaselineStyle::kSuperscript));
EXPECT_TRUE(
test_api()->weights().EqualsValueForTesting(Font::Weight::BOLD));
EXPECT_TRUE(
Expand All @@ -722,15 +724,17 @@ TEST_F(RenderTextTest, ApplyStyles) {

// Apply a ranged color and style and check the resulting breaks.
render_text->ApplyColor(SK_ColorGREEN, Range(1, 4));
render_text->ApplyBaselineStyle(SUPERIOR, Range(2, 4));
render_text->ApplyBaselineStyle(BaselineStyle::kSuperior, Range(2, 4));
render_text->ApplyWeight(Font::Weight::BOLD, Range(2, 5));
render_text->ApplyFontSizeOverride(kTestFontSizeOverride, Range(5, 7));

EXPECT_TRUE(test_api()->colors().EqualsForTesting(
{{0, kPlaceholderColor}, {1, SK_ColorGREEN}, {4, kPlaceholderColor}}));

EXPECT_TRUE(test_api()->baselines().EqualsForTesting(
{{0, NORMAL_BASELINE}, {2, SUPERIOR}, {4, NORMAL_BASELINE}}));
{{0, BaselineStyle::kNormalBaseline},
{2, BaselineStyle::kSuperior},
{4, BaselineStyle::kNormalBaseline}}));

EXPECT_TRUE(test_api()->font_size_overrides().EqualsForTesting(
{{0, 0}, {5, kTestFontSizeOverride}, {7, 0}}));
Expand All @@ -743,8 +747,9 @@ TEST_F(RenderTextTest, ApplyStyles) {
// Ensure that setting a value overrides the ranged values.
render_text->SetColor(SK_ColorBLUE);
EXPECT_TRUE(test_api()->colors().EqualsValueForTesting(SK_ColorBLUE));
render_text->SetBaselineStyle(SUBSCRIPT);
EXPECT_TRUE(test_api()->baselines().EqualsValueForTesting(SUBSCRIPT));
render_text->SetBaselineStyle(BaselineStyle::kSubscript);
EXPECT_TRUE(
test_api()->baselines().EqualsValueForTesting(BaselineStyle::kSubscript));
render_text->SetWeight(Font::Weight::NORMAL);
EXPECT_TRUE(
test_api()->weights().EqualsValueForTesting(Font::Weight::NORMAL));
Expand All @@ -753,11 +758,13 @@ TEST_F(RenderTextTest, ApplyStyles) {
// should be used instead of the text length for the range end)
const size_t text_length = render_text->text().length();
render_text->ApplyColor(SK_ColorGREEN, Range(0, text_length));
render_text->ApplyBaselineStyle(SUPERIOR, Range(0, text_length));
render_text->ApplyBaselineStyle(BaselineStyle::kSuperior,
Range(0, text_length));
render_text->ApplyWeight(Font::Weight::BOLD, Range(2, text_length));

EXPECT_TRUE(test_api()->colors().EqualsForTesting({{0, SK_ColorGREEN}}));
EXPECT_TRUE(test_api()->baselines().EqualsForTesting({{0, SUPERIOR}}));
EXPECT_TRUE(test_api()->baselines().EqualsForTesting(
{{0, BaselineStyle::kSuperior}}));
EXPECT_TRUE(test_api()->weights().EqualsForTesting(
{{0, Font::Weight::NORMAL}, {2, Font::Weight::BOLD}}));

Expand Down Expand Up @@ -988,15 +995,17 @@ TEST_F(RenderTextTest, AppendTextKeepsStyles) {
// Setup basic functionality.
render_text->SetText(u"abcd");
render_text->ApplyColor(SK_ColorGREEN, Range(0, 1));
render_text->ApplyBaselineStyle(SUPERSCRIPT, Range(1, 2));
render_text->ApplyBaselineStyle(BaselineStyle::kSuperscript, Range(1, 2));
render_text->ApplyStyle(TEXT_STYLE_UNDERLINE, true, Range(2, 3));
render_text->ApplyFontSizeOverride(20, Range(3, 4));
// Verify basic functionality.
const std::vector<std::pair<size_t, SkColor>> expected_color = {
{0, SK_ColorGREEN}, {1, kPlaceholderColor}};
EXPECT_TRUE(test_api()->colors().EqualsForTesting(expected_color));
const std::vector<std::pair<size_t, BaselineStyle>> expected_baseline = {
{0, NORMAL_BASELINE}, {1, SUPERSCRIPT}, {2, NORMAL_BASELINE}};
{0, BaselineStyle::kNormalBaseline},
{1, BaselineStyle::kSuperscript},
{2, BaselineStyle::kNormalBaseline}};
EXPECT_TRUE(test_api()->baselines().EqualsForTesting(expected_baseline));
const std::vector<std::pair<size_t, bool>> expected_style = {
{0, false}, {2, true}, {3, false}};
Expand Down Expand Up @@ -7434,10 +7443,10 @@ TEST_F(RenderTextTest, DISABLED_TextDoesntClip) {
for (auto* string : kTestStrings) {
paint_canvas.clear(SkColors::kWhite);
render_text->SetText(base::UTF8ToUTF16(string));
render_text->ApplyBaselineStyle(SUPERSCRIPT, Range(1, 2));
render_text->ApplyBaselineStyle(SUPERIOR, Range(3, 4));
render_text->ApplyBaselineStyle(INFERIOR, Range(5, 6));
render_text->ApplyBaselineStyle(SUBSCRIPT, Range(7, 8));
render_text->ApplyBaselineStyle(BaselineStyle::kSuperscript, Range(1, 2));
render_text->ApplyBaselineStyle(BaselineStyle::kSuperior, Range(3, 4));
render_text->ApplyBaselineStyle(BaselineStyle::kInferior, Range(5, 6));
render_text->ApplyBaselineStyle(BaselineStyle::kSubscript, Range(7, 8));
const Size string_size = render_text->GetStringSize();
render_text->SetWeight(Font::Weight::BOLD);
render_text->SetDisplayRect(
Expand Down
12 changes: 6 additions & 6 deletions ui/gfx/text_constants.h
Original file line number Diff line number Diff line change
Expand Up @@ -105,12 +105,12 @@ enum TextStyle {
// | |--------+------------+-----------| |
// | | descent | SUBSCRIPT |
// +--------+---------------------------------+-------------+
enum BaselineStyle {
NORMAL_BASELINE = 0,
SUPERSCRIPT, // e.g. a mathematical exponent would be superscript.
SUPERIOR, // e.g. 8th, the "th" would be superior script.
INFERIOR, // e.g. 1/2, the "2" would be inferior ("1" is superior).
SUBSCRIPT, // e.g. H2O, the "2" would be subscript.
enum class BaselineStyle {
kNormalBaseline = 0,
kSuperscript, // e.g. a mathematical exponent would be superscript.
kSuperior, // e.g. 8th, the "th" would be superior script.
kInferior, // e.g. 1/2, the "2" would be inferior ("1" is superior).
kSubscript, // e.g. H2O, the "2" would be subscript.
};

// Elision behaviors of text that exceeds constrained dimensions.
Expand Down

0 comments on commit 19a77d4

Please sign in to comment.