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

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

Can't Edit
Can't Publish+Mail
Start Review
Created:
2 years, 8 months ago by msw
Modified:
2 years, 8 months ago
CC:
chromium-reviews, 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) Patch
M ash/shell/app_list.cc View 1 chunk +0 lines, -1 line 0 comments Download
M ui/gfx/canvas.h View 1 2 chunks +9 lines, -12 lines 0 comments Download
M ui/gfx/canvas_skia.cc View 5 chunks +12 lines, -32 lines 0 comments Download
M ui/gfx/pango_util.h View 1 chunk +0 lines, -7 lines 0 comments Download
M ui/gfx/pango_util.cc View 1 9 chunks +19 lines, -36 lines 0 comments Download
M ui/views/controls/menu/menu_item_view.cc View 1 2 3 1 chunk +1 line, -1 line 0 comments Download
M ui/views/controls/menu/menu_item_view_views.cc View 1 2 3 1 chunk +1 line, -1 line 0 comments Download
M ui/views/controls/menu/menu_item_view_win.cc View 1 2 3 1 chunk +1 line, -1 line 0 comments Download
M ui/views/examples/text_example.h View 1 chunk +0 lines, -3 lines 0 comments Download
M ui/views/examples/text_example.cc View 1 2 3 chunks +0 lines, -28 lines 0 comments Download
Commit: CQ not working?

Messages

Total messages: 9 (0 generated)
msw
Hey Scott and Alexei, please take a look; thanks!
2 years, 8 months ago (2012-11-03 04:42:43 UTC) #1
sky (OOO until 7-20)
LGTM
2 years, 8 months ago (2012-11-05 15:25:36 UTC) #2
Alexei (OOO until July 6)
I'm a bit mixed on this. On one hand, it's great to delete dead code ...
2 years, 8 months ago (2012-11-05 15:44:42 UTC) #3
sky (OOO until 7-20)
I see your point, but if we're only using vertical I don't see a compelling ...
2 years, 8 months ago (2012-11-05 15:59:49 UTC) #4
msw
I hope RenderText will ultimately handle vertical alignment (with top/center/bottom settings as necessary) along with ...
2 years, 8 months ago (2012-11-05 22:40:50 UTC) #5
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/msw@chromium.org/11362070/7001
2 years, 8 months ago (2012-11-05 22:41:12 UTC) #6
commit-bot: I haz the power
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 ...
2 years, 8 months ago (2012-11-05 23:58:28 UTC) #7
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/msw@chromium.org/11362070/11002
2 years, 8 months ago (2012-11-06 00:09:37 UTC) #8
commit-bot: I haz the power
2 years, 8 months ago (2012-11-06 03:34:14 UTC) #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 1f9106d