Chromium Code Reviews
Help | Chromium Project | Sign in
(355)

Issue 11362070: Remove unused custom vertical text alignment in CanvasSkia. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
1 year, 5 months ago by msw
Modified:
1 year, 5 months ago
Reviewers:
Alexei Svitkine, sky
CC:
chromium-reviews_chromium.org, tfarina, sadrul, ben+watch_chromium.org
Visibility:
Public.

Description

Remove unused custom vertical text alignment in CanvasSkia.

This feature is unused in production code.
Nix gfx::Canvas::TEXT_VALIGN_[TOP|MIDDLE|BOTTOM] flags.
Simplify CanvasSkia vertical aligment; inline VAlignText.
Simplify pango_util; inline AdjustTextRectBasedOnLayout.
Remove TextExample vertical alignment combobox, etc.

BUG=none
TEST=No behavior changes.
R=sky@chromium.org,asvitkine@chromium.org

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=166126

Patch Set 1 : Deprecate unused custom vertical text alignment in CanvasSkia. #

Patch Set 2 : Adjust Canvas flag values; inline TextRectFromLayoutAndBounds. #

Patch Set 3 : Remove now unused kVerticalAlignments array for TextExample. #

Total comments: 4

Patch Set 4 : Sync and rebase. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+43 lines, -122 lines) Lint Patch
M ash/shell/app_list.cc View 1 chunk +0 lines, -1 line 0 comments 0 errors Download
M ui/gfx/canvas.h View 1 2 chunks +9 lines, -12 lines 0 comments ? errors Download
M ui/gfx/canvas_skia.cc View 5 chunks +12 lines, -32 lines 0 comments 0 errors Download
M ui/gfx/pango_util.h View 1 chunk +0 lines, -7 lines 0 comments ? errors Download
M ui/gfx/pango_util.cc View 1 9 chunks +19 lines, -36 lines 0 comments ? errors Download
M ui/views/controls/menu/menu_item_view.cc View 1 2 3 1 chunk +1 line, -1 line 0 comments 0 errors Download
M ui/views/controls/menu/menu_item_view_views.cc View 1 2 3 1 chunk +1 line, -1 line 0 comments ? errors Download
M ui/views/controls/menu/menu_item_view_win.cc View 1 2 3 1 chunk +1 line, -1 line 0 comments ? errors Download
M ui/views/examples/text_example.h View 1 chunk +0 lines, -3 lines 0 comments ? errors Download
M ui/views/examples/text_example.cc View 1 2 3 chunks +0 lines, -28 lines 0 comments ? errors Download
Commit:

Messages

Total messages: 9
msw
Hey Scott and Alexei, please take a look; thanks!
1 year, 5 months ago #1
sky
LGTM
1 year, 5 months ago #2
Alexei Svitkine
I'm a bit mixed on this. On one hand, it's great to delete dead code ...
1 year, 5 months ago #3
sky
I see your point, but if we're only using vertical I don't see a compelling ...
1 year, 5 months ago #4
msw
I hope RenderText will ultimately handle vertical alignment (with top/center/bottom settings as necessary) along with ...
1 year, 5 months ago #5
I haz the power (commit-bot)
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/msw@chromium.org/11362070/7001
1 year, 5 months ago #6
I haz the power (commit-bot)
Failed to apply patch for ui/views/controls/menu/menu_item_view.cc: While running patch -p1 --forward --force --no-backup-if-mismatch; patching file ...
1 year, 5 months ago #7
I haz the power (commit-bot)
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/msw@chromium.org/11362070/11002
1 year, 5 months ago #8
I haz the power (commit-bot)
1 year, 5 months ago #9
Change committed as 166126
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 1280:2d3e6564b7b6