Issue 310743003: use imageInfo instead of (deprecated) getDevice and config()

Issue 310743003: use imageInfo instead of (deprecated) getDevice and config() (Closed)

Created:
6 years, 6 months ago by reed1

Modified:
6 years, 6 months ago

Reviewers:
Ami GONE FROM CHROMIUM

CC:
chromium-reviews, feature-media-reviews_chromium.org

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

Visibility:
Public.

More Reviews

Description

use imageInfo instead of (deprecated) getDevice and config() TBR=scherkus@chromium.org Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=274492

Patch Set 1 #

Total comments: 6

Created: 6 years, 6 months ago

Download [raw] [tar.bz2]

		Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+2 lines, -3 lines)			Patch
	M	media/filters/skcanvas_video_renderer.cc	View		1 chunk	+2 lines, -3 lines	6 comments	Download

Messages

Total messages: 8 (0 generated)

Expand Messages | Collapse Messages

commit-bot: I haz the power

CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/reed@google.com/310743003/1

6 years, 6 months ago (2014-06-03 04:15:50 UTC) #3

Ami GONE FROM CHROMIUM

With no comment about it or linked BUG= unclear why this needed a TBR. https://codereview.chromium.org/310743003/diff/1/media/filters/skcanvas_video_renderer.cc ...

6 years, 6 months ago (2014-06-03 17:01:07 UTC) #5

reed1

https://codereview.chromium.org/310743003/diff/1/media/filters/skcanvas_video_renderer.cc File media/filters/skcanvas_video_renderer.cc (right): https://codereview.chromium.org/310743003/diff/1/media/filters/skcanvas_video_renderer.cc#newcode73 media/filters/skcanvas_video_renderer.cc:73: const SkImageInfo info = canvas->imageInfo(); On 2014/06/03 17:01:07, Ami ...

6 years, 6 months ago (2014-06-03 17:26:08 UTC) #6

Ami GONE FROM CHROMIUM

6 years, 6 months ago (2014-06-03 21:24:47 UTC) #7

reed1

6 years, 6 months ago (2014-06-03 22:18:18 UTC) #8

Message was sent while issue was closed.

On 2014/06/03 21:24:47, Ami leaving Chromium June 9th wrote:
>
https://codereview.chromium.org/310743003/diff/1/media/filters/skcanvas_video...
> File media/filters/skcanvas_video_renderer.cc (right):
> 
>
https://codereview.chromium.org/310743003/diff/1/media/filters/skcanvas_video...
> media/filters/skcanvas_video_renderer.cc:73: const SkImageInfo info =
> canvas->imageInfo();
> On 2014/06/03 17:26:08, reed1 wrote:
> > On 2014/06/03 17:01:07, Ami leaving Chromium June 9th wrote:
> > > const& instead of const ?
> > 
> > The canvas is returning by value, not by ref. Should I still change the
caller
> > to receive a ref?
> 
> Probably not.
> 
>
https://codereview.chromium.org/310743003/diff/1/media/filters/skcanvas_video...
> media/filters/skcanvas_video_renderer.cc:75: if (info.colorType() ==
> kN32_SkColorType && info.isOpaque()) {
> On 2014/06/03 17:26:08, reed1 wrote:
> > On 2014/06/03 17:01:07, Ami leaving Chromium June 9th wrote:
> > > Isn't this a behavior change on Android per l.14?
> > 
> > Not sure the android reference. My change is functionally equivalent to the
> > previous code.
> 
> Huh; I was confused by 
>
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/skia/i...
> #if SK_PMCOLOR_BYTE_ORDER(B,G,R,A)
>     kN32_SkColorType = kBGRA_8888_SkColorType,
> #elif SK_PMCOLOR_BYTE_ORDER(R,G,B,A)
>     kN32_SkColorType = kRGBA_8888_SkColorType,
> 
> But now I see the before code is kARGB_8888_Config, not
kRGBA_8888_SkColorType.

Yea. Part of the reason to move to colortype was to 1) allow for multiple
swizzles, and 2) make it explicitly clear when we're using an alias (e.g. kN32)

Expand Messages | Collapse Messages