Issue 331453004: android: Fix inconsistent sizes in GetScaledContentBitmap

Issue 331453004: android: Fix inconsistent sizes in GetScaledContentBitmap (Closed)

Created:
6 years, 6 months ago by powei

Modified:
6 years, 4 months ago

Reviewers:
aelias_OOO_until_Jul13, no sievers

CC:
chromium-reviews, yusukes+watch_chromium.org, yukishiino+watch_chromium.org, jam, penghuang+watch_chromium.org, nona+watch_chromium.org, darin-cc_chromium.org, James Su, miu+watch_chromium.org, clholgat

Base URL:
https://chromium.googlesource.com/chromium/src.git@master

Visibility:
Public.

More Reviews

Description

android: Fix inconsistent sizes in GetScaledContentBitmap GetScaledContentBitmap assumes that bounds() of RWHVAndroid's layer is in pixel space. This is no longer true since https://codereview.chromium.org/311253004/. This patch makes clear that the |src_subrect| input to GetScaledContentBitmap is in pixel space and that the method internally scales the input size to be in DIP space, which is the expected input space of CopyFromCompositingSurface. BUG=383572

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+19 lines, -25 lines)			Patch
M	content/browser/compositor/delegated_frame_host.cc	View	1 2 3 4 5	2 chunks	+2 lines, -6 lines	0 comments	Download
M	content/browser/renderer_host/render_widget_host_view_android.cc	View	1 2 3 4 5	5 chunks	+16 lines, -18 lines	0 comments	Download
M	ui/snapshot/snapshot_aura.cc	View	1 2 3 4 5	1 chunk	+1 line, -1 line	0 comments	Download

Messages

Total messages: 4 (0 generated)

Expand Messages | Collapse Messages

powei

No bugs were filed, but thumbnails are currently broken on ToT due to this.

6 years, 6 months ago (2014-06-11 20:39:38 UTC) #2

no sievers

>GetScaledContentBitmap assumes that bounds() of RWHVAndroid's layer is in pixel space. This is no longer ...

6 years, 6 months ago (2014-06-11 21:39:11 UTC) #3

powei

6 years, 6 months ago (2014-06-11 21:46:12 UTC) #4

https://codereview.chromium.org/331453004/diff/60001/content/browser/renderer...
File content/browser/renderer_host/render_widget_host_view_android.cc (right):

https://codereview.chromium.org/331453004/diff/60001/content/browser/renderer...
content/browser/renderer_host/render_widget_host_view_android.cc:313:
ConvertRectToDIP(device_scale_factor, src_subrect), scale));
On 2014/06/11 21:39:11, sievers wrote:
> Why does the src_subrect get scaled by |scale|?

I think |src_subrect| assumes its space to be pre-scaled.  It's only used for
the NTP thumbnails.

https://codereview.chromium.org/331453004/diff/60001/content/browser/renderer...
content/browser/renderer_host/render_widget_host_view_android.cc:320:
src_subrect_in_dip = gfx::Rect(dst_size_in_dip);
On 2014/06/11 21:39:11, sievers wrote:
> When does this happen?

It's a somewhat hacky convention that when |src_subrect| is empty, then just
take the entire content.  Added here
(https://codereview.chromium.org/173023005/)  when we were fixing NTP thumbnails
and yet wanted to keep this method for tab switcher thumbnails.  So tab-switcher
thumbnails also pass in empty |src_subrect|s.

Expand Messages | Collapse Messages