Commit 7be6ed95 authored by ericrk's avatar ericrk Committed by Commit bot

Remove error handling from MipmapUtil and add DCHECKs

MipmapUtil had error handling where it would return "-1" values to
indicate errors. None of the call sites should trigger this behavior,
so the error checks were converted to DCHECKs.

R=vmpstr
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Review-Url: https://codereview.chromium.org/2105903003
Cr-Commit-Position: refs/heads/master@{#403285}
parent 19a49ad5
...@@ -22,10 +22,10 @@ int MipMapUtil::GetLevelForSize(const gfx::Size& src_size, ...@@ -22,10 +22,10 @@ int MipMapUtil::GetLevelForSize(const gfx::Size& src_size,
int src_width = src_size.width(); int src_width = src_size.width();
int target_height = target_size.height(); int target_height = target_size.height();
int target_width = target_size.width(); int target_width = target_size.width();
bool target_invalid = target_height == 0 || target_width == 0; DCHECK_GT(target_height, 0);
bool src_invalid = src_height == 0 || src_width == 0; DCHECK_GT(target_width, 0);
if (target_invalid || src_invalid) DCHECK_GT(src_width, 0);
return -1; DCHECK_GT(src_height, 0);
int next_mip_height = src_height; int next_mip_height = src_height;
int next_mip_width = src_width; int next_mip_width = src_width;
...@@ -52,8 +52,9 @@ int MipMapUtil::GetLevelForSize(const gfx::Size& src_size, ...@@ -52,8 +52,9 @@ int MipMapUtil::GetLevelForSize(const gfx::Size& src_size,
SkSize MipMapUtil::GetScaleAdjustmentForLevel(const gfx::Size& src_size, SkSize MipMapUtil::GetScaleAdjustmentForLevel(const gfx::Size& src_size,
int mip_level) { int mip_level) {
if (src_size.width() == 0 || src_size.height() == 0 || mip_level == -1) DCHECK_GT(src_size.width(), 0);
return SkSize::Make(-1, -1); DCHECK_GT(src_size.height(), 0);
DCHECK_GE(mip_level, 0);
gfx::Size target_size = GetSizeForLevel(src_size, mip_level); gfx::Size target_size = GetSizeForLevel(src_size, mip_level);
...@@ -64,8 +65,9 @@ SkSize MipMapUtil::GetScaleAdjustmentForLevel(const gfx::Size& src_size, ...@@ -64,8 +65,9 @@ SkSize MipMapUtil::GetScaleAdjustmentForLevel(const gfx::Size& src_size,
gfx::Size MipMapUtil::GetSizeForLevel(const gfx::Size& src_size, gfx::Size MipMapUtil::GetSizeForLevel(const gfx::Size& src_size,
int mip_level) { int mip_level) {
if (src_size.width() == 0 || src_size.height() == 0 || mip_level == -1) DCHECK_GT(src_size.width(), 0);
return gfx::Size(-1, -1); DCHECK_GT(src_size.height(), 0);
DCHECK_GE(mip_level, 0);
return gfx::Size(ScaleAxisToMipLevel(src_size.width(), mip_level), return gfx::Size(ScaleAxisToMipLevel(src_size.width(), mip_level),
ScaleAxisToMipLevel(src_size.height(), mip_level)); ScaleAxisToMipLevel(src_size.height(), mip_level));
......
...@@ -16,25 +16,25 @@ class CC_EXPORT MipMapUtil { ...@@ -16,25 +16,25 @@ class CC_EXPORT MipMapUtil {
// Determine the smallest mip level that is larger than |target_size|. Each // Determine the smallest mip level that is larger than |target_size|. Each
// mip level corresponds to a power of two scale of the image - for instance, // mip level corresponds to a power of two scale of the image - for instance,
// level 0 is original size, level 1 is 2x smaller, level 2 is 4x smaller, // level 0 is original size, level 1 is 2x smaller, level 2 is 4x smaller,
// etc... // etc... This function does not do error checking and must be called with a
// Returns -1 if |src_size| or |target_size| is invalid (any dimension is 0). // valid src_size (width/height > 0) and mip_level (>= 0).
static int GetLevelForSize(const gfx::Size& src_size, static int GetLevelForSize(const gfx::Size& src_size,
const gfx::Size& target_size); const gfx::Size& target_size);
// Determines the scale factor for the given |mip_level|. Returns (-1, -1) if // Determines the scale factor for the given |mip_level|. This function does
// |src_size| is invalid (width/height <= 0), or if mip_level is invalid (== // not do error checking and must be called with a valid src_size
// -1). // (width/height > 0) and mip_level (>= 0).
static SkSize GetScaleAdjustmentForLevel(const gfx::Size& src_size, static SkSize GetScaleAdjustmentForLevel(const gfx::Size& src_size,
int mip_level); int mip_level);
// Determine the size of the given |mip_level|. Returns (-1, -1) if // Determine the size of the given |mip_level|. This function does not do
// |src_size| is invalid (width/height <= 0) or if mip-level is invalid (== // error checking and must be called with a valid src_size (width/height > 0)
// -1). // and mip_level (>= 0).
static gfx::Size GetSizeForLevel(const gfx::Size& src_size, int mip_level); static gfx::Size GetSizeForLevel(const gfx::Size& src_size, int mip_level);
// Determines the scale factor for the smallest mip level that is larger than // Determines the scale factor for the smallest mip level that is larger than
// |target_size|. Returns (-1, -1) if |src_size| or |target_size| is invalid // |target_size|. This function does not do error checking and must be called
// (width/height <= 0). // with a valid src_size (width/height > 0) and mip_level (>= 0).
static SkSize GetScaleAdjustmentForSize(const gfx::Size& src_size, static SkSize GetScaleAdjustmentForSize(const gfx::Size& src_size,
const gfx::Size& target_size); const gfx::Size& target_size);
}; };
......
...@@ -116,34 +116,5 @@ TEST(MipMapUtilTest, Rounding) { ...@@ -116,34 +116,5 @@ TEST(MipMapUtilTest, Rounding) {
MipMapUtil::GetScaleAdjustmentForSize(src_size, target_size_smaller)); MipMapUtil::GetScaleAdjustmentForSize(src_size, target_size_smaller));
} }
// Ensures that we return invalid values correctly.
TEST(MipMapUtilTest, Invalid) {
const gfx::Size valid_size(1024, 1024);
const gfx::Size invalid_size(0, 1024);
const gfx::Size invalid_result_size(-1, -1);
const SkSize invalid_float_result_size = SkSize::Make(-1, -1);
const int invalid_result_level = -1;
EXPECT_EQ(invalid_result_level,
MipMapUtil::GetLevelForSize(valid_size, invalid_size));
EXPECT_EQ(invalid_result_level,
MipMapUtil::GetLevelForSize(invalid_size, valid_size));
EXPECT_FLOAT_SIZE_EQ(
invalid_float_result_size,
MipMapUtil::GetScaleAdjustmentForSize(valid_size, invalid_size));
EXPECT_FLOAT_SIZE_EQ(
invalid_float_result_size,
MipMapUtil::GetScaleAdjustmentForSize(invalid_size, valid_size));
EXPECT_SIZE_EQ(invalid_result_size,
MipMapUtil::GetSizeForLevel(valid_size, invalid_result_level));
EXPECT_SIZE_EQ(invalid_result_size,
MipMapUtil::GetSizeForLevel(invalid_size, 0));
EXPECT_FLOAT_SIZE_EQ(invalid_float_result_size,
MipMapUtil::GetScaleAdjustmentForLevel(invalid_size, 0));
EXPECT_FLOAT_SIZE_EQ(
invalid_float_result_size,
MipMapUtil::GetScaleAdjustmentForLevel(valid_size, invalid_result_level));
}
} // namespace } // namespace
} // namespace cc } // namespace cc
...@@ -818,7 +818,6 @@ ImageDecodeControllerKey ImageDecodeControllerKey::FromDrawImage( ...@@ -818,7 +818,6 @@ ImageDecodeControllerKey ImageDecodeControllerKey::FromDrawImage(
if (quality == kMedium_SkFilterQuality && !target_size.IsEmpty()) { if (quality == kMedium_SkFilterQuality && !target_size.IsEmpty()) {
SkSize mip_target_size = SkSize mip_target_size =
MipMapUtil::GetScaleAdjustmentForSize(src_rect.size(), target_size); MipMapUtil::GetScaleAdjustmentForSize(src_rect.size(), target_size);
DCHECK(mip_target_size.width() != -1.f && mip_target_size.height() != -1.f);
target_size.set_width(src_rect.width() * mip_target_size.width()); target_size.set_width(src_rect.width() * mip_target_size.width());
target_size.set_height(src_rect.height() * mip_target_size.height()); target_size.set_height(src_rect.height() * mip_target_size.height());
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment