Skip to content

Commit

Permalink
SkBitmap and SkPixelRef no longer need lock/unlock
Browse files Browse the repository at this point in the history
Refactoring CL -- no intended behavior change. Just removing calls to empty apis.

BUG=skia:6481
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2;master.tryserver.chromium.linux:linux_site_isolation

refactoring CL
TBR=jochen, hubbe, finnur, thestig, wez

Review-Url: https://codereview.chromium.org/2823003002
Cr-Commit-Position: refs/heads/master@{#465770}
  • Loading branch information
reed-at-google authored and Commit bot committed Apr 19, 2017
1 parent d920c93 commit 1139536
Show file tree
Hide file tree
Showing 109 changed files with 96 additions and 510 deletions.
4 changes: 1 addition & 3 deletions cc/layers/scrollbar_layer_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1211,10 +1211,8 @@ class ScaledScrollbarLayerTestScaledRasterization : public ScrollbarLayerTest {

DCHECK(bitmap);

AutoLockUIResourceBitmap locked_bitmap(*bitmap);

const SkColor* pixels =
reinterpret_cast<const SkColor*>(locked_bitmap.GetPixels());
reinterpret_cast<const SkColor*>(bitmap->GetPixels());
SkColor color = argb_to_skia(
scrollbar_layer->fake_scrollbar()->paint_fill_color());
int width = bitmap->GetSize().width();
Expand Down
1 change: 0 additions & 1 deletion cc/output/gl_renderer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2830,7 +2830,6 @@ void GLRenderer::FinishedReadback(unsigned source_buffer,
if (src_pixels) {
bitmap.reset(new SkBitmap);
bitmap->allocN32Pixels(size.width(), size.height());
std::unique_ptr<SkAutoLockPixels> lock(new SkAutoLockPixels(*bitmap));
uint8_t* dest_pixels = static_cast<uint8_t*>(bitmap->getPixels());

size_t row_bytes = size.width() * 4;
Expand Down
78 changes: 28 additions & 50 deletions cc/output/renderer_pixeltest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1986,12 +1986,10 @@ TYPED_TEST(RendererPixelTest, RenderPassAndMaskWithPartialQuad) {
ResourceId mask_resource_id = this->resource_provider_->CreateResource(
mask_rect.size(), ResourceProvider::TEXTURE_HINT_IMMUTABLE, RGBA_8888,
gfx::ColorSpace());
{
SkAutoLockPixels lock(bitmap);
this->resource_provider_->CopyToResource(
mask_resource_id, reinterpret_cast<uint8_t*>(bitmap.getPixels()),
mask_rect.size());
}

this->resource_provider_->CopyToResource(
mask_resource_id, reinterpret_cast<uint8_t*>(bitmap.getPixels()),
mask_rect.size());

// This RenderPassDrawQuad does not include the full |viewport_rect| which is
// the size of the child render pass.
Expand Down Expand Up @@ -2081,12 +2079,10 @@ TYPED_TEST(RendererPixelTest, RenderPassAndMaskWithPartialQuad2) {
ResourceId mask_resource_id = this->resource_provider_->CreateResource(
mask_rect.size(), ResourceProvider::TEXTURE_HINT_IMMUTABLE, RGBA_8888,
gfx::ColorSpace());
{
SkAutoLockPixels lock(bitmap);
this->resource_provider_->CopyToResource(
mask_resource_id, reinterpret_cast<uint8_t*>(bitmap.getPixels()),
mask_rect.size());
}

this->resource_provider_->CopyToResource(
mask_resource_id, reinterpret_cast<uint8_t*>(bitmap.getPixels()),
mask_rect.size());

// This RenderPassDrawQuad does not include the full |viewport_rect| which is
// the size of the child render pass.
Expand Down Expand Up @@ -2809,25 +2805,19 @@ TYPED_TEST(RendererPixelTest, TileDrawQuadNearestNeighbor) {

SkBitmap bitmap;
bitmap.allocN32Pixels(2, 2);
{
SkAutoLockPixels lock(bitmap);
SkCanvas canvas(bitmap);
draw_point_color(&canvas, 0, 0, SK_ColorGREEN);
draw_point_color(&canvas, 0, 1, SK_ColorBLUE);
draw_point_color(&canvas, 1, 0, SK_ColorBLUE);
draw_point_color(&canvas, 1, 1, SK_ColorGREEN);
}
SkCanvas canvas(bitmap);
draw_point_color(&canvas, 0, 0, SK_ColorGREEN);
draw_point_color(&canvas, 0, 1, SK_ColorBLUE);
draw_point_color(&canvas, 1, 0, SK_ColorBLUE);
draw_point_color(&canvas, 1, 1, SK_ColorGREEN);

gfx::Size tile_size(2, 2);
ResourceId resource = this->resource_provider_->CreateResource(
tile_size, ResourceProvider::TEXTURE_HINT_IMMUTABLE, RGBA_8888,
gfx::ColorSpace());

{
SkAutoLockPixels lock(bitmap);
this->resource_provider_->CopyToResource(
resource, static_cast<uint8_t*>(bitmap.getPixels()), tile_size);
}
this->resource_provider_->CopyToResource(
resource, static_cast<uint8_t*>(bitmap.getPixels()), tile_size);

int id = 1;
gfx::Transform transform_to_root;
Expand Down Expand Up @@ -2860,25 +2850,19 @@ TYPED_TEST(SoftwareRendererPixelTest, TextureDrawQuadNearestNeighbor) {

SkBitmap bitmap;
bitmap.allocN32Pixels(2, 2);
{
SkAutoLockPixels lock(bitmap);
SkCanvas canvas(bitmap);
draw_point_color(&canvas, 0, 0, SK_ColorGREEN);
draw_point_color(&canvas, 0, 1, SK_ColorBLUE);
draw_point_color(&canvas, 1, 0, SK_ColorBLUE);
draw_point_color(&canvas, 1, 1, SK_ColorGREEN);
}
SkCanvas canvas(bitmap);
draw_point_color(&canvas, 0, 0, SK_ColorGREEN);
draw_point_color(&canvas, 0, 1, SK_ColorBLUE);
draw_point_color(&canvas, 1, 0, SK_ColorBLUE);
draw_point_color(&canvas, 1, 1, SK_ColorGREEN);

gfx::Size tile_size(2, 2);
ResourceId resource = this->resource_provider_->CreateResource(
tile_size, ResourceProvider::TEXTURE_HINT_IMMUTABLE, RGBA_8888,
gfx::ColorSpace());

{
SkAutoLockPixels lock(bitmap);
this->resource_provider_->CopyToResource(
resource, static_cast<uint8_t*>(bitmap.getPixels()), tile_size);
}
this->resource_provider_->CopyToResource(
resource, static_cast<uint8_t*>(bitmap.getPixels()), tile_size);

int id = 1;
gfx::Transform transform_to_root;
Expand Down Expand Up @@ -2913,7 +2897,6 @@ TYPED_TEST(SoftwareRendererPixelTest, TextureDrawQuadLinear) {
SkBitmap bitmap;
bitmap.allocN32Pixels(2, 2);
{
SkAutoLockPixels lock(bitmap);
SkCanvas canvas(bitmap);
draw_point_color(&canvas, 0, 0, SK_ColorGREEN);
draw_point_color(&canvas, 0, 1, SK_ColorBLUE);
Expand All @@ -2926,11 +2909,8 @@ TYPED_TEST(SoftwareRendererPixelTest, TextureDrawQuadLinear) {
tile_size, ResourceProvider::TEXTURE_HINT_IMMUTABLE, RGBA_8888,
gfx::ColorSpace());

{
SkAutoLockPixels lock(bitmap);
this->resource_provider_->CopyToResource(
resource, static_cast<uint8_t*>(bitmap.getPixels()), tile_size);
}
this->resource_provider_->CopyToResource(
resource, static_cast<uint8_t*>(bitmap.getPixels()), tile_size);

int id = 1;
gfx::Transform transform_to_root;
Expand Down Expand Up @@ -3289,12 +3269,10 @@ TEST_F(GLRendererPixelTest, TextureQuadBatching) {
ResourceId resource = this->resource_provider_->CreateResource(
mask_rect.size(), ResourceProvider::TEXTURE_HINT_IMMUTABLE, RGBA_8888,
gfx::ColorSpace());
{
SkAutoLockPixels lock(bitmap);
this->resource_provider_->CopyToResource(
resource, reinterpret_cast<uint8_t*>(bitmap.getPixels()),
mask_rect.size());
}

this->resource_provider_->CopyToResource(
resource, reinterpret_cast<uint8_t*>(bitmap.getPixels()),
mask_rect.size());

// Arbitrary dividing lengths to divide up the resource into 16 quads.
int widths[] = {
Expand Down
14 changes: 0 additions & 14 deletions cc/resources/ui_resource_bitmap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -89,18 +89,4 @@ UIResourceBitmap::UIResourceBitmap(sk_sp<SkPixelRef> pixel_ref,
UIResourceBitmap::UIResourceBitmap(const UIResourceBitmap& other) = default;

UIResourceBitmap::~UIResourceBitmap() {}

AutoLockUIResourceBitmap::AutoLockUIResourceBitmap(
const UIResourceBitmap& bitmap) : bitmap_(bitmap) {
bitmap_.pixel_ref_->lockPixels();
}

AutoLockUIResourceBitmap::~AutoLockUIResourceBitmap() {
bitmap_.pixel_ref_->unlockPixels();
}

const uint8_t* AutoLockUIResourceBitmap::GetPixels() const {
return static_cast<const uint8_t*>(bitmap_.pixel_ref_->pixels());
}

} // namespace cc
14 changes: 4 additions & 10 deletions cc/resources/ui_resource_bitmap.h
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,10 @@ class CC_EXPORT UIResourceBitmap {
return pixel_ref_ ? pixel_ref_->rowBytes() * size_.height() : 0;
}

const uint8_t* GetPixels() const {
return static_cast<const uint8_t*>(pixel_ref_->pixels());
}

private:
friend class AutoLockUIResourceBitmap;

Expand All @@ -67,16 +71,6 @@ class CC_EXPORT UIResourceBitmap {
bool opaque_;
};

class CC_EXPORT AutoLockUIResourceBitmap {
public:
explicit AutoLockUIResourceBitmap(const UIResourceBitmap& bitmap);
~AutoLockUIResourceBitmap();
const uint8_t* GetPixels() const;

private:
const UIResourceBitmap& bitmap_;
};

} // namespace cc

#endif // CC_RESOURCES_UI_RESOURCE_BITMAP_H_
6 changes: 0 additions & 6 deletions cc/test/pixel_comparator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,6 @@ bool ExactPixelComparator::Compare(const SkBitmap& actual_bmp,
DCHECK(actual_bmp.width() == expected_bmp.width() &&
actual_bmp.height() == expected_bmp.height());

SkAutoLockPixels lock_actual_bmp(actual_bmp);
SkAutoLockPixels lock_expected_bmp(expected_bmp);

for (int x = 0; x < actual_bmp.width(); ++x) {
for (int y = 0; y < actual_bmp.height(); ++y) {
SkColor actual_color = actual_bmp.getColor(x, y);
Expand Down Expand Up @@ -96,9 +93,6 @@ bool FuzzyPixelComparator::Compare(const SkBitmap& actual_bmp,
// Check that bitmaps are not empty.
DCHECK(actual_bmp.width() > 0 && actual_bmp.height() > 0);

SkAutoLockPixels lock_actual_bmp(actual_bmp);
SkAutoLockPixels lock_expected_bmp(expected_bmp);

for (int x = 0; x < actual_bmp.width(); ++x) {
for (int y = 0; y < actual_bmp.height(); ++y) {
SkColor actual_color = actual_bmp.getColor(x, y);
Expand Down
8 changes: 2 additions & 6 deletions cc/trees/layer_tree_host_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3876,9 +3876,7 @@ void LayerTreeHostImpl::CreateUIResource(UIResourceId uid,
gfx::ColorSpace::CreateSRGB());

if (!scaled) {
AutoLockUIResourceBitmap bitmap_lock(bitmap);
auto* pixels = bitmap_lock.GetPixels();
resource_provider_->CopyToResource(id, pixels, source_size);
resource_provider_->CopyToResource(id, bitmap.GetPixels(), source_size);
} else {
// Only support auto-resizing for N32 textures (since this is primarily for
// scrollbars). Users of other types need to ensure they are not too big.
Expand All @@ -3895,10 +3893,9 @@ void LayerTreeHostImpl::CreateUIResource(UIResourceId uid,
source_size.width(), source_size.height(), kPremul_SkAlphaType);
int row_bytes = source_size.width() * 4;

AutoLockUIResourceBitmap bitmap_lock(bitmap);
SkBitmap source_bitmap;
source_bitmap.setInfo(info, row_bytes);
source_bitmap.setPixels(const_cast<uint8_t*>(bitmap_lock.GetPixels()));
source_bitmap.setPixels(const_cast<uint8_t*>(bitmap.GetPixels()));

// This applies the scale to draw the |bitmap| into |scaled_bitmap|.
SkBitmap scaled_bitmap;
Expand All @@ -3912,7 +3909,6 @@ void LayerTreeHostImpl::CreateUIResource(UIResourceId uid,
scaled_canvas.clear(SK_ColorTRANSPARENT);
scaled_canvas.drawBitmap(source_bitmap, 0, 0);

SkAutoLockPixels scaled_bitmap_lock(scaled_bitmap);
auto* pixels = static_cast<uint8_t*>(scaled_bitmap.getPixels());
resource_provider_->CopyToResource(id, pixels, upload_size);
}
Expand Down
14 changes: 0 additions & 14 deletions chrome/browser/android/thumbnail/thumbnail_cache.cc
Original file line number Diff line number Diff line change
Expand Up @@ -502,8 +502,6 @@ bool WriteToFile(base::File& file,
return false;

// Write ETC1 header.
compressed_data->lockPixels();

unsigned char etc1_buffer[ETC_PKM_HEADER_SIZE];
etc1_pkm_format_header(etc1_buffer,
compressed_data->info().width(),
Expand All @@ -523,8 +521,6 @@ bool WriteToFile(base::File& file,
if (pixel_bytes_written != data_size)
return false;

compressed_data->unlockPixels();

if (!WriteBigEndianToFile(file, kCurrentExtraVersion))
return false;

Expand Down Expand Up @@ -575,7 +571,6 @@ void ThumbnailCache::CompressionTask(
gfx::Size content_size;

if (!raw_data.empty()) {
SkAutoLockPixels raw_data_lock(raw_data);
gfx::Size raw_data_size(raw_data.width(), raw_data.height());
size_t pixel_size = 4; // Pixel size is 4 bytes for kARGB_8888_Config.
size_t stride = pixel_size * raw_data_size.width();
Expand All @@ -590,7 +585,6 @@ void ThumbnailCache::CompressionTask(
sk_sp<SkPixelRef> etc1_pixel_ref(SkMallocPixelRef::MakeWithData(
info, 0, NULL, std::move(etc1_pixel_data)));

etc1_pixel_ref->lockPixels();
bool success = etc1_encode_image(
reinterpret_cast<unsigned char*>(raw_data.getPixels()),
raw_data_size.width(),
Expand All @@ -601,7 +595,6 @@ void ThumbnailCache::CompressionTask(
encoded_size.width(),
encoded_size.height());
etc1_pixel_ref->setImmutable();
etc1_pixel_ref->unlockPixels();

if (success) {
compressed_data = std::move(etc1_pixel_ref);
Expand Down Expand Up @@ -860,16 +853,13 @@ void ThumbnailCache::DecompressionTask(
buffer_size.height(),
kRGBA_8888_SkColorType,
kOpaque_SkAlphaType));
SkAutoLockPixels raw_data_lock(raw_data);
compressed_data->lockPixels();
success = etc1_decode_image(
reinterpret_cast<unsigned char*>(compressed_data->pixels()),
reinterpret_cast<unsigned char*>(raw_data.getPixels()),
buffer_size.width(),
buffer_size.height(),
raw_data.bytesPerPixel(),
raw_data.rowBytes());
compressed_data->unlockPixels();
raw_data.setImmutable();

if (!success) {
Expand All @@ -884,7 +874,6 @@ void ThumbnailCache::DecompressionTask(
content_size.height(),
kRGBA_8888_SkColorType,
kOpaque_SkAlphaType));
SkAutoLockPixels raw_data_small_lock(raw_data_small);
SkCanvas small_canvas(raw_data_small);
small_canvas.drawBitmap(raw_data, 0, 0);
raw_data_small.setImmutable();
Expand All @@ -911,7 +900,6 @@ std::pair<SkBitmap, float> ThumbnailCache::CreateApproximation(
float scale) {
DCHECK(!bitmap.empty());
DCHECK_GT(scale, 0);
SkAutoLockPixels bitmap_lock(bitmap);
float new_scale = 1.f / kApproximationScaleFactor;

gfx::Size dst_size = gfx::ScaleToFlooredSize(
Expand All @@ -922,8 +910,6 @@ std::pair<SkBitmap, float> ThumbnailCache::CreateApproximation(
bitmap.info().colorType(),
bitmap.info().alphaType()));
dst_bitmap.eraseColor(0);
SkAutoLockPixels dst_bitmap_lock(dst_bitmap);

SkCanvas canvas(dst_bitmap);
canvas.scale(new_scale, new_scale);
canvas.drawBitmap(bitmap, 0, 0, NULL);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,8 +92,6 @@ class AccessibilityHighlightManagerTest : public InProcessBrowserTest {
void ComputeImageStats() {
diff_count_ = 0;
double accum[4] = {0, 0, 0, 0};
SkAutoLockPixels lock_before(before_bmp_);
SkAutoLockPixels lock_after(after_bmp_);
for (int x = 0; x < before_bmp_.width(); ++x) {
for (int y = 0; y < before_bmp_.height(); ++y) {
SkColor before_color = before_bmp_.getColor(x, y);
Expand Down Expand Up @@ -139,7 +137,6 @@ class AccessibilityHighlightManagerTest : public InProcessBrowserTest {
run_loop_quitter_ = run_loop.QuitClosure();
run_loop.Run();
SkBitmap bitmap = image_.AsBitmap();
SkAutoLockPixels lock(bitmap);
if (bitmap.width() != bounds.width() ||
bitmap.height() != bounds.height()) {
LOG(INFO) << "Bitmap not correct size, trying to capture again";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,6 @@ bool SkDifferentPixelsMetric::diff(

// Prepare the pixels for comparison
result->poiCount = 0;
baseline->lockPixels();
test->lockPixels();
for (int y = 0; y < height; y++) {
// Grab a row from each image for easy comparison
// TODO(epoger): The code below already assumes 4 bytes per pixel, so I
Expand Down Expand Up @@ -102,23 +100,11 @@ bool SkDifferentPixelsMetric::diff(
}
}
}
test->unlockPixels();
baseline->unlockPixels();

result->maxRedDiff = maxRedDiff;
result->maxGreenDiff = maxGreenDiff;
result->maxBlueDiff = maxBlueDiff;

if (bitmapsToCreate.alphaMask) {
result->poiAlphaMask.unlockPixels();
}
if (bitmapsToCreate.rgbDiff) {
result->rgbDiffBitmap.unlockPixels();
}
if (bitmapsToCreate.whiteDiff) {
result->whiteDiffBitmap.unlockPixels();
}

// Calculates the percentage of identical pixels
result->result = 1.0 - ((double)result->poiCount / (width * height));

Expand Down
Loading

0 comments on commit 1139536

Please sign in to comment.