Skip to content

Commit

Permalink
80% of calls to skia::GetTopDevice() were retrieving a read-only refe…
Browse files Browse the repository at this point in the history
…rence to the raw pixel data; this CL simplifies those call sites and migrates to a more GPU-friendly and less internals-exposing implementation.

BUG=543755

Review URL: https://codereview.chromium.org/1399223004

Cr-Commit-Position: refs/heads/master@{#355104}
  • Loading branch information
Naburimannu authored and Commit bot committed Oct 20, 2015
1 parent 7fa3690 commit 3c954e6
Show file tree
Hide file tree
Showing 11 changed files with 36 additions and 44 deletions.
18 changes: 6 additions & 12 deletions chrome/browser/thumbnails/content_analysis_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -84,8 +84,7 @@ TEST_F(ThumbnailContentAnalysisTest, ApplyGradientMagnitudeOnImpulse) {
canvas.FillRect(gfx::Rect(0, 0, 800, 600), SkColorSetRGB(10, 10, 10));
canvas.FillRect(gfx::Rect(400, 300, 1, 1), SkColorSetRGB(255, 255, 255));

SkBitmap source =
skia::GetTopDevice(*canvas.sk_canvas())->accessBitmap(false);
SkBitmap source = skia::ReadPixels(canvas.sk_canvas());

SkBitmap reduced_color;
reduced_color.allocPixels(SkImageInfo::MakeA8(source.width(),
Expand Down Expand Up @@ -128,8 +127,7 @@ TEST_F(ThumbnailContentAnalysisTest, ApplyGradientMagnitudeOnFrame) {
canvas.FillRect(gfx::Rect(0, 0, 800, 600), SkColorSetRGB(0, 0, 0));
canvas.DrawRect(draw_rect, SkColorSetRGB(255, 255, 255));

SkBitmap source =
skia::GetTopDevice(*canvas.sk_canvas())->accessBitmap(false);
SkBitmap source = skia::ReadPixels(canvas.sk_canvas());

SkBitmap reduced_color;
reduced_color.allocPixels(SkImageInfo::MakeA8(source.width(),
Expand Down Expand Up @@ -167,8 +165,7 @@ TEST_F(ThumbnailContentAnalysisTest, ExtractImageProfileInformation) {
canvas.FillRect(image_rect, SkColorSetRGB(0, 0, 0));
canvas.DrawRect(draw_rect, SkColorSetRGB(255, 255, 255));

SkBitmap source =
skia::GetTopDevice(*canvas.sk_canvas())->accessBitmap(false);
SkBitmap source = skia::ReadPixels(canvas.sk_canvas());
SkBitmap reduced_color;
reduced_color.allocPixels(SkImageInfo::MakeA8(source.width(),
source.height()));
Expand Down Expand Up @@ -231,8 +228,7 @@ TEST_F(ThumbnailContentAnalysisTest,
canvas.DrawRect(gfx::Rect(300, 250, 99, 100), SkColorSetRGB(255, 255, 255));
canvas.DrawRect(gfx::Rect(401, 250, 99, 100), SkColorSetRGB(255, 255, 255));

SkBitmap source =
skia::GetTopDevice(*canvas.sk_canvas())->accessBitmap(false);
SkBitmap source = skia::ReadPixels(canvas.sk_canvas());
SkBitmap reduced_color;
reduced_color.allocPixels(SkImageInfo::MakeA8(source.width(),
source.height()));
Expand Down Expand Up @@ -573,8 +569,7 @@ TEST_F(ThumbnailContentAnalysisTest, ComputeDecimatedImage) {
std::fill_n(columns.begin() + 300, 100, true);
std::fill_n(columns.begin() + 500, 100, true);

SkBitmap source =
skia::GetTopDevice(*canvas.sk_canvas())->accessBitmap(false);
SkBitmap source = skia::ReadPixels(canvas.sk_canvas());
SkBitmap result = ComputeDecimatedImage(source, rows, columns);
EXPECT_FALSE(result.empty());
EXPECT_EQ(300, result.width());
Expand Down Expand Up @@ -686,8 +681,7 @@ TEST_F(ThumbnailContentAnalysisTest, CreateRetargetedThumbnailImage) {
canvas.DrawRect(pict_rect, SkColorSetRGB(0, 0, 0));
}

SkBitmap source =
skia::GetTopDevice(*canvas.sk_canvas())->accessBitmap(false);
SkBitmap source = skia::ReadPixels(canvas.sk_canvas());

SkBitmap result = CreateRetargetedThumbnailImage(
source, gfx::Size(424, 264), 2.5);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -137,8 +137,7 @@ TEST_F(ContentBasedThumbnailingAlgorithmTest, CreateRetargetedThumbnail) {

// The image consists of vertical non-overlapping stripes 150 pixels wide.
canvas.FillRect(gfx::Rect(200, 200, 800, 400), SkColorSetRGB(255, 255, 255));
SkBitmap source =
skia::GetTopDevice(*canvas.sk_canvas())->accessBitmap(false);
SkBitmap source = skia::ReadPixels(canvas.sk_canvas());

ConsumerCallbackCatcher catcher;
const gfx::Size thumbnail_size(432, 284);
Expand Down
15 changes: 5 additions & 10 deletions chrome/browser/thumbnails/simple_thumbnail_crop_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,7 @@ typedef testing::Test SimpleThumbnailCropTest;
TEST_F(SimpleThumbnailCropTest, GetClippedBitmap_TallerThanWide) {
// The input bitmap is vertically long.
gfx::Canvas canvas(gfx::Size(40, 90), 1.0f, true);
SkBitmap bitmap =
skia::GetTopDevice(*canvas.sk_canvas())->accessBitmap(false);
SkBitmap bitmap = skia::ReadPixels(canvas.sk_canvas());

// The desired size is square.
thumbnails::ClipResult clip_result = thumbnails::CLIP_RESULT_NOT_CLIPPED;
Expand All @@ -43,8 +42,7 @@ TEST_F(SimpleThumbnailCropTest, GetClippedBitmap_TallerThanWide) {
TEST_F(SimpleThumbnailCropTest, GetClippedBitmap_WiderThanTall) {
// The input bitmap is horizontally long.
gfx::Canvas canvas(gfx::Size(70, 40), 1.0f, true);
SkBitmap bitmap =
skia::GetTopDevice(*canvas.sk_canvas())->accessBitmap(false);
SkBitmap bitmap = skia::ReadPixels(canvas.sk_canvas());

// The desired size is square.
thumbnails::ClipResult clip_result = thumbnails::CLIP_RESULT_NOT_CLIPPED;
Expand All @@ -60,8 +58,7 @@ TEST_F(SimpleThumbnailCropTest, GetClippedBitmap_WiderThanTall) {
TEST_F(SimpleThumbnailCropTest, GetClippedBitmap_TooWiderThanTall) {
// The input bitmap is horizontally very long.
gfx::Canvas canvas(gfx::Size(90, 40), 1.0f, true);
SkBitmap bitmap =
skia::GetTopDevice(*canvas.sk_canvas())->accessBitmap(false);
SkBitmap bitmap = skia::ReadPixels(canvas.sk_canvas());

// The desired size is square.
thumbnails::ClipResult clip_result = thumbnails::CLIP_RESULT_NOT_CLIPPED;
Expand All @@ -77,8 +74,7 @@ TEST_F(SimpleThumbnailCropTest, GetClippedBitmap_TooWiderThanTall) {
TEST_F(SimpleThumbnailCropTest, GetClippedBitmap_NotClipped) {
// The input bitmap is square.
gfx::Canvas canvas(gfx::Size(40, 40), 1.0f, true);
SkBitmap bitmap =
skia::GetTopDevice(*canvas.sk_canvas())->accessBitmap(false);
SkBitmap bitmap = skia::ReadPixels(canvas.sk_canvas());

// The desired size is square.
thumbnails::ClipResult clip_result = thumbnails::CLIP_RESULT_NOT_CLIPPED;
Expand All @@ -94,8 +90,7 @@ TEST_F(SimpleThumbnailCropTest, GetClippedBitmap_NotClipped) {
TEST_F(SimpleThumbnailCropTest, GetClippedBitmap_NonSquareOutput) {
// The input bitmap is square.
gfx::Canvas canvas(gfx::Size(40, 40), 1.0f, true);
SkBitmap bitmap =
skia::GetTopDevice(*canvas.sk_canvas())->accessBitmap(false);
SkBitmap bitmap = skia::ReadPixels(canvas.sk_canvas());

// The desired size is horizontally long.
thumbnails::ClipResult clip_result = thumbnails::CLIP_RESULT_NOT_CLIPPED;
Expand Down
3 changes: 1 addition & 2 deletions components/test_runner/web_test_proxy.cc
Original file line number Diff line number Diff line change
Expand Up @@ -600,8 +600,7 @@ void WebTestProxyBase::CapturePixelsForPrinting(
web_frame->printEnd();

DrawSelectionRect(canvas.get());
SkBaseDevice* device = skia::GetTopDevice(*canvas);
const SkBitmap& bitmap = device->accessBitmap(false);
const SkBitmap bitmap = skia::ReadPixels(canvas.get());
callback.Run(bitmap);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,8 +99,7 @@ TEST_F(PluginInstanceThrottlerImplTest, ThrottleByKeyframe) {
gfx::Canvas canvas(gfx::Size(20, 10), 1.0f, true);
canvas.FillRect(gfx::Rect(20, 10), SK_ColorBLACK);
canvas.FillRect(gfx::Rect(10, 10), SK_ColorWHITE);
SkBitmap interesting_bitmap =
skia::GetTopDevice(*canvas.sk_canvas())->accessBitmap(false);
SkBitmap interesting_bitmap = skia::ReadPixels(canvas.sk_canvas());

// Don't throttle for a boring frame.
throttler()->OnImageFlush(&boring_bitmap);
Expand Down
7 changes: 7 additions & 0 deletions skia/ext/platform_canvas.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,13 @@ SkBaseDevice* GetTopDevice(const SkCanvas& canvas) {
return canvas.getTopDevice(true);
}

SkBitmap ReadPixels(SkCanvas* canvas) {
SkBitmap bitmap;
bitmap.setInfo(canvas->imageInfo());
canvas->readPixels(&bitmap, 0, 0);
return bitmap;
}

bool SupportsPlatformPaint(const SkCanvas* canvas) {
PlatformDevice* platform_device = GetPlatformDevice(GetTopDevice(*canvas));
return platform_device && platform_device->SupportsPlatformPaint();
Expand Down
7 changes: 7 additions & 0 deletions skia/ext/platform_canvas.h
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,13 @@ SK_API size_t PlatformCanvasStrideForWidth(unsigned width);
// by the next call to save() or restore().
SK_API SkBaseDevice* GetTopDevice(const SkCanvas& canvas);

// Copies pixels from the SkCanvas into an SkBitmap, fetching pixels from
// GPU memory if necessary.
//
// The bitmap will remain empty if we can't allocate enough memory for a copy
// of the pixels.
SK_API SkBitmap ReadPixels(SkCanvas* canvas);

// Returns true if native platform routines can be used to draw on the
// given canvas. If this function returns false, BeginPlatformPaint will
// return NULL PlatformSurface.
Expand Down
3 changes: 1 addition & 2 deletions skia/ext/platform_canvas_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,7 @@ bool IsOfColor(const SkBitmap& bitmap, int x, int y, uint32_t color) {
bool VerifyRect(const PlatformCanvas& canvas,
uint32_t canvas_color, uint32_t rect_color,
int x, int y, int w, int h) {
SkBaseDevice* device = skia::GetTopDevice(canvas);
const SkBitmap& bitmap = device->accessBitmap(false);
const SkBitmap bitmap = skia::ReadPixels(const_cast<PlatformCanvas*>(&canvas));
SkAutoLockPixels lock(bitmap);

for (int cur_y = 0; cur_y < bitmap.height(); cur_y++) {
Expand Down
3 changes: 1 addition & 2 deletions ui/gfx/blit_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,7 @@ void SetToCanvas(skia::PlatformCanvas* canvas, uint8 values[h][w]) {
// bitmap (see SetToCanvas above).
template<int w, int h>
void VerifyCanvasValues(skia::PlatformCanvas* canvas, uint8 values[h][w]) {
SkBitmap& bitmap = const_cast<SkBitmap&>(
skia::GetTopDevice(*canvas)->accessBitmap(true));
SkBitmap bitmap = skia::ReadPixels(canvas);
SkAutoLockPixels lock(bitmap);
ASSERT_EQ(w, bitmap.width());
ASSERT_EQ(h, bitmap.height());
Expand Down
12 changes: 4 additions & 8 deletions ui/gfx/color_analysis_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -342,8 +342,7 @@ TEST_F(ColorAnalysisTest, ComputeColorCovarianceWithCanvas) {
canvas.FillRect(gfx::Rect(150, 0, 50, 200), SkColorSetRGB(0, 100, 100));
canvas.FillRect(gfx::Rect(200, 0, 50, 200), SkColorSetRGB(0, 0, 100));

SkBitmap bitmap =
skia::GetTopDevice(*canvas.sk_canvas())->accessBitmap(false);
SkBitmap bitmap = skia::ReadPixels(canvas.sk_canvas());
gfx::Matrix3F covariance = ComputeColorCovariance(bitmap);

gfx::Matrix3F expected_covariance = gfx::Matrix3F::Zeros();
Expand Down Expand Up @@ -401,8 +400,7 @@ TEST_F(ColorAnalysisTest, ApplyColorReductionBlackAndWhite) {
// The image consists of vertical non-overlapping stripes 150 pixels wide.
canvas.FillRect(gfx::Rect(0, 0, 150, 200), SkColorSetRGB(0, 0, 0));
canvas.FillRect(gfx::Rect(150, 0, 150, 200), SkColorSetRGB(255, 255, 255));
SkBitmap source =
skia::GetTopDevice(*canvas.sk_canvas())->accessBitmap(false);
SkBitmap source = skia::ReadPixels(canvas.sk_canvas());
SkBitmap result;
result.allocPixels(SkImageInfo::MakeA8(300, 200));

Expand Down Expand Up @@ -439,8 +437,7 @@ TEST_F(ColorAnalysisTest, ApplyColorReductionMultiColor) {
canvas.FillRect(gfx::Rect(0, 0, 100, 200), SkColorSetRGB(100, 0, 0));
canvas.FillRect(gfx::Rect(100, 0, 100, 200), SkColorSetRGB(0, 255, 0));
canvas.FillRect(gfx::Rect(200, 0, 100, 200), SkColorSetRGB(0, 0, 128));
SkBitmap source =
skia::GetTopDevice(*canvas.sk_canvas())->accessBitmap(false);
SkBitmap source = skia::ReadPixels(canvas.sk_canvas());
SkBitmap result;
result.allocPixels(SkImageInfo::MakeA8(300, 200));

Expand Down Expand Up @@ -482,8 +479,7 @@ TEST_F(ColorAnalysisTest, ComputePrincipalComponentImage) {
canvas.FillRect(gfx::Rect(0, 0, 100, 200), SkColorSetRGB(10, 10, 10));
canvas.FillRect(gfx::Rect(100, 0, 100, 200), SkColorSetRGB(100, 100, 100));
canvas.FillRect(gfx::Rect(200, 0, 100, 200), SkColorSetRGB(255, 255, 255));
SkBitmap source =
skia::GetTopDevice(*canvas.sk_canvas())->accessBitmap(false);
SkBitmap source = skia::ReadPixels(canvas.sk_canvas());
SkBitmap result;
result.allocPixels(SkImageInfo::MakeA8(300, 200));

Expand Down
6 changes: 2 additions & 4 deletions ui/gfx/color_utils_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -127,8 +127,7 @@ TEST(ColorUtils, CalculateBoringScore_SingleColor) {
// Fill all pixels in black.
canvas.FillRect(gfx::Rect(kSize), SK_ColorBLACK);

SkBitmap bitmap =
skia::GetTopDevice(*canvas.sk_canvas())->accessBitmap(false);
SkBitmap bitmap = skia::ReadPixels(canvas.sk_canvas());
// The thumbnail should deserve the highest boring score.
EXPECT_DOUBLE_EQ(1.0, CalculateBoringScore(bitmap));
}
Expand All @@ -143,8 +142,7 @@ TEST(ColorUtils, CalculateBoringScore_TwoColors) {
canvas.FillRect(gfx::Rect(0, 0, kSize.width() / 2, kSize.height()),
SK_ColorWHITE);

SkBitmap bitmap =
skia::GetTopDevice(*canvas.sk_canvas())->accessBitmap(false);
SkBitmap bitmap = skia::ReadPixels(canvas.sk_canvas());
ASSERT_EQ(kSize.width(), bitmap.width());
ASSERT_EQ(kSize.height(), bitmap.height());
// The thumbnail should be less boring because two colors are used.
Expand Down

0 comments on commit 3c954e6

Please sign in to comment.