SkMipMap::ComputeLevelSize should only cover SkMipMap's levels.
SkMipMap only deals with the levels it generates.
That is to day, it deals with mipmap levels 1-x, not 0-x.
Other functions reflect thing when indexing.
They go from 0 to x-1 (giving the index into SkMipMap's contents).
ComputeLevelSize should also follow that same indexing.
BUG=578304
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2042843005
Review-Url: https://codereview.chromium.org/2042843005
diff --git a/src/core/SkMipMap.cpp b/src/core/SkMipMap.cpp
index e6f4bad..355c1ed 100644
--- a/src/core/SkMipMap.cpp
+++ b/src/core/SkMipMap.cpp
@@ -370,9 +370,9 @@
return nullptr;
}
// whip through our loop to compute the exact size needed
- size_t size = 0;
+ size_t size = 0;
int countLevels = ComputeLevelCount(src.width(), src.height());
- for (int currentMipLevel = countLevels; currentMipLevel > 0; currentMipLevel--) {
+ for (int currentMipLevel = countLevels; currentMipLevel >= 0; currentMipLevel--) {
SkISize mipSize = ComputeLevelSize(src.width(), src.height(), currentMipLevel);
size += SkColorTypeMinRowBytes(ct, mipSize.fWidth) * mipSize.fHeight;
}
@@ -502,19 +502,19 @@
}
int maxLevelCount = ComputeLevelCount(baseWidth, baseHeight);
- if (level > maxLevelCount || level < 0) {
+ if (level >= maxLevelCount || level < 0) {
return SkISize::Make(0, 0);
}
- if (level == 0) {
- return SkISize::Make(baseWidth, baseHeight);
- }
-
// OpenGL's spec requires that each mipmap level have height/width equal to
// max(1, floor(original_height / 2^i)
// (or original_width) where i is the mipmap level.
- int width = SkTMax(1, baseWidth >> level);
- int height = SkTMax(1, baseHeight >> level);
+ // SkMipMap does not include the base mip level.
+ // For example, it contains levels 1-x instead of 0-x.
+ // This is because the image used to create SkMipMap is the base level.
+ // So subtract 1 from the mip level to get the index stored by SkMipMap.
+ int width = SkTMax(1, baseWidth >> (level + 1));
+ int height = SkTMax(1, baseHeight >> (level + 1));
return SkISize::Make(width, height);
}
diff --git a/src/core/SkMipMap.h b/src/core/SkMipMap.h
index a230a23..7c798ef 100644
--- a/src/core/SkMipMap.h
+++ b/src/core/SkMipMap.h
@@ -18,15 +18,26 @@
typedef SkDiscardableMemory* (*SkDiscardableFactoryProc)(size_t bytes);
+/*
+ * SkMipMap will generate mipmap levels when given a base mipmap level image.
+ *
+ * Any function which deals with mipmap levels indices will start with index 0
+ * being the first mipmap level which was generated. Said another way, it does
+ * not include the base level in its range.
+ */
class SkMipMap : public SkCachedData {
public:
static SkMipMap* Build(const SkPixmap& src, SkDiscardableFactoryProc);
static SkMipMap* Build(const SkBitmap& src, SkDiscardableFactoryProc);
// Determines how many levels a SkMipMap will have without creating that mipmap.
+ // This does not include the base mipmap level that the user provided when
+ // creating the SkMipMap.
static int ComputeLevelCount(int baseWidth, int baseHeight);
// Determines the size of a given mipmap level.
+ // |level| is an index into the generated mipmap levels. It does not include
+ // the base level. So index 0 represents mipmap level 1.
static SkISize ComputeLevelSize(int baseWidth, int baseHeight, int level);
struct Level {
@@ -35,7 +46,13 @@
};
bool extractLevel(const SkSize& scale, Level*) const;
+
+ // countLevels returns the number of mipmap levels generated (which does not
+ // include the base mipmap level).
int countLevels() const;
+
+ // |index| is an index into the generated mipmap levels. It does not include
+ // the base level. So index 0 represents mipmap level 1.
bool getLevel(int index, Level*) const;
protected:
diff --git a/tests/MipMapTest.cpp b/tests/MipMapTest.cpp
index e8d02b8..596e995 100644
--- a/tests/MipMapTest.cpp
+++ b/tests/MipMapTest.cpp
@@ -64,11 +64,15 @@
const int mipLevelCount = mm->countLevels();
REPORTER_ASSERT(reporter, mipLevelCount == expectedMipLevelCount);
+ REPORTER_ASSERT(reporter, mipLevelCount == SkMipMap::ComputeLevelCount(width, height));
for (int i = 0; i < mipLevelCount; ++i) {
SkMipMap::Level level;
REPORTER_ASSERT(reporter, mm->getLevel(i, &level));
// Make sure the mipmaps contain valid data and that the sizes are correct
REPORTER_ASSERT(reporter, level.fPixmap.addr());
+ SkISize size = SkMipMap::ComputeLevelSize(width, height, i);
+ REPORTER_ASSERT(reporter, level.fPixmap.width() == size.width());
+ REPORTER_ASSERT(reporter, level.fPixmap.height() == size.height());
// + 1 because SkMipMap does not include the base mipmap level.
int twoToTheMipLevel = 1 << (i + 1);
@@ -161,34 +165,34 @@
DEF_TEST(MipMap_ComputeLevelSize, reporter) {
const LevelSizeScenario tests[] = {
// Test mipmaps with negative sizes
- {-100, 100, 1, SkISize::Make(0, 0)},
- {100, -100, 1, SkISize::Make(0, 0)},
- {-100, -100, 1, SkISize::Make(0, 0)},
+ {-100, 100, 0, SkISize::Make(0, 0)},
+ {100, -100, 0, SkISize::Make(0, 0)},
+ {-100, -100, 0, SkISize::Make(0, 0)},
// Test mipmaps with 0, 1, 2 as dimensions
// (SkMipMap::Build requires a min size of 1)
//
// 0
- {0, 100, 1, SkISize::Make(0, 0)},
- {100, 0, 1, SkISize::Make(0, 0)},
- {0, 0, 1, SkISize::Make(0, 0)},
+ {0, 100, 0, SkISize::Make(0, 0)},
+ {100, 0, 0, SkISize::Make(0, 0)},
+ {0, 0, 0, SkISize::Make(0, 0)},
// 1
- {1, 100, 1, SkISize::Make(1, 50)},
- {100, 1, 1, SkISize::Make(50, 1)},
- {1, 1, 1, SkISize::Make(0, 0)},
+ {1, 100, 0, SkISize::Make(1, 50)},
+ {100, 1, 0, SkISize::Make(50, 1)},
+ {1, 1, 0, SkISize::Make(0, 0)},
// 2
- {2, 100, 1, SkISize::Make(1, 50)},
- {100, 2, 2, SkISize::Make(25, 1)},
- {2, 2, 1, SkISize::Make(1, 1)},
+ {2, 100, 0, SkISize::Make(1, 50)},
+ {100, 2, 1, SkISize::Make(25, 1)},
+ {2, 2, 0, SkISize::Make(1, 1)},
// Test a handful of cases
- {63, 63, 3, SkISize::Make(7, 7)},
- {64, 64, 3, SkISize::Make(8, 8)},
- {127, 127, 3, SkISize::Make(15, 15)},
- {64, 129, 4, SkISize::Make(4, 8)},
- {255, 32, 7, SkISize::Make(1, 1)},
- {500, 1000, 2, SkISize::Make(125, 250)},
+ {63, 63, 2, SkISize::Make(7, 7)},
+ {64, 64, 2, SkISize::Make(8, 8)},
+ {127, 127, 2, SkISize::Make(15, 15)},
+ {64, 129, 3, SkISize::Make(4, 8)},
+ {255, 32, 6, SkISize::Make(1, 1)},
+ {500, 1000, 1, SkISize::Make(125, 250)},
};
for (auto& currentTest : tests) {