Issue 8383028: ui/gfx: Convert Canvas::ClipRectInt() to use gfx::Rect.

Issue 8383028: ui/gfx: Convert Canvas::ClipRectInt() to use gfx::Rect. (Closed)

Created:
9 years, 2 months ago by tfarina

Modified:
9 years, 2 months ago

Reviewers:
Peter Kasting

CC:
chromium-reviews, tfarina, dhollowa, Paweł Hajdan Jr.

Base URL:
svn://svn.chromium.org/chrome/trunk/src

Visibility:
Public.

Description

ui/gfx: Convert Canvas::ClipRectInt() to use gfx::Rect. BUG=100898 R=pkasting@chromium.org Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=107276

Patch Set 2 : address Peter's review #

Patch Set 3 : fix typo #

Created: 9 years, 2 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+111 lines, -102 lines)			Patch
M	chrome/browser/ui/gtk/tabs/tab_renderer_gtk.cc	View		2 chunks	+4 lines, -3 lines	0 comments	Download
M	chrome/browser/ui/views/infobars/infobar_view.cc	View		1 chunk	+1 line, -1 line	0 comments	Download
M	chrome/browser/ui/views/tabs/base_tab.cc	View	1	2 chunks	+2 lines, -2 lines	0 comments	Download
M	ui/gfx/canvas.h	View		1 chunk	+1 line, -1 line	0 comments	Download
M	ui/gfx/canvas_direct2d.h	View		2 chunks	+39 lines, -31 lines	0 comments	Download
M	ui/gfx/canvas_direct2d.cc	View	1	1 chunk	+3 lines, -3 lines	0 comments	Download
M	ui/gfx/canvas_direct2d_unittest.cc	View		4 chunks	+5 lines, -5 lines	0 comments	Download
M	ui/gfx/canvas_skia.h	View		3 chunks	+42 lines, -33 lines	0 comments	Download
M	ui/gfx/canvas_skia.cc	View	1	3 chunks	+4 lines, -6 lines	0 comments	Download
M	ui/gfx/canvas_skia_win.cc	View		1 chunk	+1 line, -2 lines	0 comments	Download
M	ui/gfx/render_text.cc	View		1 chunk	+1 line, -2 lines	0 comments	Download
M	ui/views/view.cc	View	1	1 chunk	+1 line, -1 line	0 comments	Download
M	ui/views/widget/native_widget_win.cc	View	1	1 chunk	+1 line, -2 lines	0 comments	Download
M	views/controls/combobox/native_combobox_views.cc	View	1 2	2 chunks	+2 lines, -3 lines	0 comments	Download
M	views/view.cc	View		1 chunk	+3 lines, -3 lines	0 comments	Download
M	views/widget/native_widget_win.cc	View	1	1 chunk	+1 line, -4 lines	0 comments	Download

Messages

Total messages: 3 (0 generated)

Expand Messages | Collapse Messages

Peter Kasting

LGTM with nits http://codereview.chromium.org/8383028/diff/5001/chrome/browser/ui/views/tabs/base_tab.cc File chrome/browser/ui/views/tabs/base_tab.cc (right): http://codereview.chromium.org/8383028/diff/5001/chrome/browser/ui/views/tabs/base_tab.cc#newcode447 chrome/browser/ui/views/tabs/base_tab.cc:447: canvas->ClipRectInt(gfx::Rect(0, 0, width(), height())); Nit: I ...

9 years, 2 months ago (2011-10-25 20:24:46 UTC) #2

tfarina

9 years, 2 months ago (2011-10-25 23:52:40 UTC) #3

http://codereview.chromium.org/8383028/diff/5001/chrome/browser/ui/views/tabs...
File chrome/browser/ui/views/tabs/base_tab.cc (right):

http://codereview.chromium.org/8383028/diff/5001/chrome/browser/ui/views/tabs...
chrome/browser/ui/views/tabs/base_tab.cc:447: canvas->ClipRectInt(gfx::Rect(0,
0, width(), height()));
On 2011/10/25 20:24:47, Peter Kasting wrote:
> Nit: I think you can pass GetLocalBounds() here.

Done.

http://codereview.chromium.org/8383028/diff/5001/ui/gfx/canvas_direct2d.cc
File ui/gfx/canvas_direct2d.cc (right):

http://codereview.chromium.org/8383028/diff/5001/ui/gfx/canvas_direct2d.cc#ne...
ui/gfx/canvas_direct2d.cc:178: RectToRectF(rect.x(), rect.y(), rect.width(),
rect.height()),
On 2011/10/25 20:24:47, Peter Kasting wrote:
> Nit: Just RectToRectF(rect) should work.

Done.

http://codereview.chromium.org/8383028/diff/5001/ui/gfx/canvas_direct2d.cc#ne...
ui/gfx/canvas_direct2d.cc:183: return rect.width() > 0 && rect.height() > 0;
On 2011/10/25 20:24:47, Peter Kasting wrote:
> Nit: Simpler:
> 
> return !rect.IsEmpty();

Done.

http://codereview.chromium.org/8383028/diff/5001/ui/gfx/canvas_skia.cc
File ui/gfx/canvas_skia.cc (right):

http://codereview.chromium.org/8383028/diff/5001/ui/gfx/canvas_skia.cc#newcod...
ui/gfx/canvas_skia.cc:113: SkRect new_clip;
On 2011/10/25 20:24:47, Peter Kasting wrote:
> Nit: Simpler:
> 
> return canvas_->clipRect(gfx::RectToSkRect(rect));
> 
> You may need to #include "ui/gfx/skia_util.h".

Done.

http://codereview.chromium.org/8383028/diff/5001/ui/views/view.cc
File ui/views/view.cc (right):

http://codereview.chromium.org/8383028/diff/5001/ui/views/view.cc#newcode555
ui/views/view.cc:555: if (canvas->ClipRectInt(gfx::Rect(origin(), size()))) {
On 2011/10/25 20:24:47, Peter Kasting wrote:
> Nit: Isn't this just |bounds_|?

Done.

http://codereview.chromium.org/8383028/diff/5001/ui/views/widget/native_widge...
File ui/views/widget/native_widget_win.cc (right):

http://codereview.chromium.org/8383028/diff/5001/ui/views/widget/native_widge...
ui/views/widget/native_widget_win.cc:454: window_contents_->ClipRectInt(
On 2011/10/25 20:24:47, Peter Kasting wrote:
> Nit: I think just gfx::Rect(r) works

Done.

http://codereview.chromium.org/8383028/diff/5001/views/controls/combobox/nati...
File views/controls/combobox/native_combobox_views.cc (right):

http://codereview.chromium.org/8383028/diff/5001/views/controls/combobox/nati...
views/controls/combobox/native_combobox_views.cc:305:
canvas->ClipRectInt(gfx::Rect(insets.left(),
On 2011/10/25 20:24:47, Peter Kasting wrote:
> Nit: I think you can just pass GetContentsBounds() here

Done.

Expand Messages | Collapse Messages