streamline skvm errors
store() returns a success bool only because it can, because it wasn't
going to return any sort of skvm::Value anyway. But it should never
fail given a well-formed skvm::PixelFormat, e.g. one from
SkColorType_to_PixelFormat. So move the "error handling" inside, really
just asserting/assuming it doesn't fail.
And similarly, skvm::SkColorType_to_PixelFormat() can no longer fail, so
have it return the skvm::PixelFormat directly instead of the bool I used
to stage things back when building this out.
Change-Id: I6dc3b6da32cdaaef377fe59b8c94846e902841ee
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/367796
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
diff --git a/src/core/SkVM.cpp b/src/core/SkVM.cpp
index 692dd7f..9385506 100644
--- a/src/core/SkVM.cpp
+++ b/src/core/SkVM.cpp
@@ -1023,41 +1023,42 @@
return round(mul(x, limit));
}
- bool SkColorType_to_PixelFormat(SkColorType ct, PixelFormat* f) {
+ PixelFormat SkColorType_to_PixelFormat(SkColorType ct) {
auto UNORM = PixelFormat::UNORM,
FLOAT = PixelFormat::FLOAT;
switch (ct) {
- case kUnknown_SkColorType: SkASSERT(false); return false;
+ case kUnknown_SkColorType: break;
- case kRGBA_F32_SkColorType: *f = {FLOAT,32,32,32,32, 0,32,64,96}; return true;
+ case kRGBA_F32_SkColorType: return {FLOAT,32,32,32,32, 0,32,64,96};
- case kRGBA_F16Norm_SkColorType: *f = {FLOAT,16,16,16,16, 0,16,32,48}; return true;
- case kRGBA_F16_SkColorType: *f = {FLOAT,16,16,16,16, 0,16,32,48}; return true;
- case kR16G16B16A16_unorm_SkColorType: *f = {UNORM,16,16,16,16, 0,16,32,48}; return true;
+ case kRGBA_F16Norm_SkColorType: return {FLOAT,16,16,16,16, 0,16,32,48};
+ case kRGBA_F16_SkColorType: return {FLOAT,16,16,16,16, 0,16,32,48};
+ case kR16G16B16A16_unorm_SkColorType: return {UNORM,16,16,16,16, 0,16,32,48};
- case kA16_float_SkColorType: *f = {FLOAT, 0, 0,0,16, 0, 0,0,0}; return true;
- case kR16G16_float_SkColorType: *f = {FLOAT, 16,16,0, 0, 0,16,0,0}; return true;
+ case kA16_float_SkColorType: return {FLOAT, 0, 0,0,16, 0, 0,0,0};
+ case kR16G16_float_SkColorType: return {FLOAT, 16,16,0, 0, 0,16,0,0};
- case kAlpha_8_SkColorType: *f = {UNORM, 0,0,0,8, 0,0,0,0}; return true;
- case kGray_8_SkColorType: *f = {UNORM, 8,8,8,0, 0,0,0,0}; return true; // Subtle.
+ case kAlpha_8_SkColorType: return {UNORM, 0,0,0,8, 0,0,0,0};
+ case kGray_8_SkColorType: return {UNORM, 8,8,8,0, 0,0,0,0}; // Subtle.
- case kRGB_565_SkColorType: *f = {UNORM, 5,6,5,0, 11,5,0,0}; return true; // (BGR)
- case kARGB_4444_SkColorType: *f = {UNORM, 4,4,4,4, 12,8,4,0}; return true; // (ABGR)
+ case kRGB_565_SkColorType: return {UNORM, 5,6,5,0, 11,5,0,0}; // (BGR)
+ case kARGB_4444_SkColorType: return {UNORM, 4,4,4,4, 12,8,4,0}; // (ABGR)
- case kRGBA_8888_SkColorType: *f = {UNORM, 8,8,8,8, 0,8,16,24}; return true;
- case kRGB_888x_SkColorType: *f = {UNORM, 8,8,8,0, 0,8,16,32}; return true; // 32-bit
- case kBGRA_8888_SkColorType: *f = {UNORM, 8,8,8,8, 16,8, 0,24}; return true;
+ case kRGBA_8888_SkColorType: return {UNORM, 8,8,8,8, 0,8,16,24};
+ case kRGB_888x_SkColorType: return {UNORM, 8,8,8,0, 0,8,16,32}; // 32-bit
+ case kBGRA_8888_SkColorType: return {UNORM, 8,8,8,8, 16,8, 0,24};
- case kRGBA_1010102_SkColorType: *f = {UNORM, 10,10,10,2, 0,10,20,30}; return true;
- case kBGRA_1010102_SkColorType: *f = {UNORM, 10,10,10,2, 20,10, 0,30}; return true;
- case kRGB_101010x_SkColorType: *f = {UNORM, 10,10,10,0, 0,10,20, 0}; return true;
- case kBGR_101010x_SkColorType: *f = {UNORM, 10,10,10,0, 20,10, 0, 0}; return true;
+ case kRGBA_1010102_SkColorType: return {UNORM, 10,10,10,2, 0,10,20,30};
+ case kBGRA_1010102_SkColorType: return {UNORM, 10,10,10,2, 20,10, 0,30};
+ case kRGB_101010x_SkColorType: return {UNORM, 10,10,10,0, 0,10,20, 0};
+ case kBGR_101010x_SkColorType: return {UNORM, 10,10,10,0, 20,10, 0, 0};
- case kR8G8_unorm_SkColorType: *f = {UNORM, 8, 8,0, 0, 0, 8,0,0}; return true;
- case kR16G16_unorm_SkColorType: *f = {UNORM, 16,16,0, 0, 0,16,0,0}; return true;
- case kA16_unorm_SkColorType: *f = {UNORM, 0, 0,0,16, 0, 0,0,0}; return true;
+ case kR8G8_unorm_SkColorType: return {UNORM, 8, 8,0, 0, 0, 8,0,0};
+ case kR16G16_unorm_SkColorType: return {UNORM, 16,16,0, 0, 0,16,0,0};
+ case kA16_unorm_SkColorType: return {UNORM, 0, 0,0,16, 0, 0,0,0};
}
- return false;
+ SkASSERT(false);
+ return {UNORM, 0,0,0,0, 0,0,0,0};
}
static int byte_size(PixelFormat f) {
@@ -1112,8 +1113,7 @@
static void assert_16byte_is_rgba_f32(PixelFormat f) {
#if defined(SK_DEBUG)
SkASSERT(byte_size(f) == 16);
- PixelFormat rgba_f32;
- SkAssertResult(SkColorType_to_PixelFormat(kRGBA_F32_SkColorType, &rgba_f32));
+ PixelFormat rgba_f32 = SkColorType_to_PixelFormat(kRGBA_F32_SkColorType);
SkASSERT(f.encoding == rgba_f32.encoding);
@@ -1209,7 +1209,7 @@
return packed;
}
- bool Builder::store(PixelFormat f, Ptr ptr, Color c) {
+ void Builder::store(PixelFormat f, Ptr ptr, Color c) {
// Detect a grayscale PixelFormat: r,g,b bit counts and shifts all equal.
if (f.r_bits == f.g_bits && f.g_bits == f.b_bits &&
f.r_shift == f.g_shift && f.g_shift == f.b_shift) {
@@ -1222,24 +1222,23 @@
}
switch (byte_size(f)) {
- case 1: store8 (ptr, pack32(f,c)); return true;
- case 2: store16(ptr, pack32(f,c)); return true;
- case 4: store32(ptr, pack32(f,c)); return true;
+ case 1: store8 (ptr, pack32(f,c)); break;
+ case 2: store16(ptr, pack32(f,c)); break;
+ case 4: store32(ptr, pack32(f,c)); break;
case 8: {
PixelFormat lo,hi;
split_disjoint_8byte_format(f, &lo,&hi);
store64(ptr, pack32(lo,c)
, pack32(hi,c));
- return true;
+ break;
}
case 16: {
assert_16byte_is_rgba_f32(f);
store128(ptr, pun_to_I32(c.r), pun_to_I32(c.g), pun_to_I32(c.b), pun_to_I32(c.a));
- return true;
+ break;
}
default: SkUNREACHABLE;
}
- return false;
}
void Builder::unpremul(F32* r, F32* g, F32* b, F32 a) {
diff --git a/src/core/SkVM.h b/src/core/SkVM.h
index e0445ca..a3d16de 100644
--- a/src/core/SkVM.h
+++ b/src/core/SkVM.h
@@ -548,7 +548,7 @@
int r_bits, g_bits, b_bits, a_bits,
r_shift, g_shift, b_shift, a_shift;
};
- bool SkColorType_to_PixelFormat(SkColorType, PixelFormat*);
+ PixelFormat SkColorType_to_PixelFormat(SkColorType);
SK_BEGIN_REQUIRE_DENSE
struct Instruction {
@@ -876,7 +876,7 @@
I32 to_unorm(int bits, F32); // E.g. to_unorm(8, x) -> round(x * 255)
Color load(PixelFormat, Ptr ptr);
- bool store(PixelFormat, Ptr ptr, Color);
+ void store(PixelFormat, Ptr ptr, Color);
Color gather(PixelFormat, Ptr ptr, int offset, I32 index);
Color gather(PixelFormat f, Uniform u, I32 index) {
return gather(f, u.ptr, u.offset, index);
@@ -1240,7 +1240,7 @@
SI F32 from_unorm(int bits, I32 x) { return x->from_unorm(bits,x); }
SI I32 to_unorm(int bits, F32 x) { return x-> to_unorm(bits,x); }
- SI bool store(PixelFormat f, Ptr p, Color c) { return c->store(f,p,c); }
+ SI void store(PixelFormat f, Ptr p, Color c) { return c->store(f,p,c); }
SI Color gather(PixelFormat f, Ptr p, int off, I32 ix) { return ix->gather(f,p,off,ix); }
SI Color gather(PixelFormat f, Uniform u , I32 ix) { return ix->gather(f,u,ix); }
diff --git a/src/core/SkVMBlitter.cpp b/src/core/SkVMBlitter.cpp
index 8057cde..2d527c4 100644
--- a/src/core/SkVMBlitter.cpp
+++ b/src/core/SkVMBlitter.cpp
@@ -170,12 +170,6 @@
}
}
- skvm::PixelFormat unused;
- if (!SkColorType_to_PixelFormat(params.dst.colorType(), &unused)) {
- // All existing SkColorTypes pass this check. We'd only get here adding new ones.
- *ok = false;
- }
-
return {
shaderHash,
clipHash,
@@ -240,8 +234,7 @@
}
// Load the destination color.
- skvm::PixelFormat dstFormat;
- SkAssertResult(SkColorType_to_PixelFormat(params.dst.colorType(), &dstFormat));
+ skvm::PixelFormat dstFormat = skvm::SkColorType_to_PixelFormat(params.dst.colorType());
skvm::Color dst = p->load(dstFormat, dst_ptr);
if (params.dst.isOpaque()) {
// When a destination is known opaque, we may assume it both starts and stays fully
@@ -269,8 +262,7 @@
break;
case Coverage::MaskLCD16: {
- skvm::PixelFormat fmt;
- SkAssertResult(SkColorType_to_PixelFormat(kRGB_565_SkColorType, &fmt));
+ skvm::PixelFormat fmt = skvm::SkColorType_to_PixelFormat(kRGB_565_SkColorType);
cov = p->load(fmt, p->varying<uint16_t>());
cov.a = select(src.a < dst.a, min(cov.r, min(cov.g, cov.b))
, max(cov.r, max(cov.g, cov.b)));
@@ -330,7 +322,7 @@
}
// Write it out!
- SkAssertResult(store(dstFormat, dst_ptr, src));
+ store(dstFormat, dst_ptr, src);
}
@@ -365,8 +357,7 @@
skvm::Uniforms* uniforms, SkArenaAlloc*) const override {
const SkColorType ct = fSprite.colorType();
- skvm::PixelFormat fmt;
- SkAssertResult(SkColorType_to_PixelFormat(ct, &fmt));
+ skvm::PixelFormat fmt = skvm::SkColorType_to_PixelFormat(ct);
skvm::Color c = p->load(fmt, p->arg(SkColorTypeBytesPerPixel(ct)));
@@ -777,10 +768,6 @@
// TODO: SkVM support for mask filters? definitely possible!
return nullptr;
}
- if (skvm::PixelFormat unused; !SkColorType_to_PixelFormat(sprite.colorType(), &unused)) {
- // All existing SkColorTypes pass this check. We'd only get here adding new ones.
- return nullptr;
- }
bool ok = true;
auto blitter = alloc->make<Blitter>(device, paint, &sprite, SkIPoint{left,top},
SkSimpleMatrixProvider{SkMatrix{}}, std::move(clip), &ok);
diff --git a/src/shaders/SkImageShader.cpp b/src/shaders/SkImageShader.cpp
index 6a0c7cf..88f5aa2 100755
--- a/src/shaders/SkImageShader.cpp
+++ b/src/shaders/SkImageShader.cpp
@@ -879,15 +879,6 @@
skvm::Coord upperLocal = SkShaderBase::ApplyMatrix(p, upperInv, origLocal, uniforms);
- // All existing SkColorTypes pass these checks. We'd only fail here adding new ones.
- skvm::PixelFormat unused;
- if (true && !SkColorType_to_PixelFormat(upper.colorType(), &unused)) {
- return {};
- }
- if (lower && !SkColorType_to_PixelFormat(lower->colorType(), &unused)) {
- return {};
- }
-
// We can exploit image opacity to skip work unpacking alpha channels.
const bool input_is_opaque = SkAlphaTypeIsOpaque(upper.alphaType())
|| SkColorTypeIsAlwaysOpaque(upper.colorType());
@@ -924,8 +915,7 @@
};
auto setup_uniforms = [&](const SkPixmap& pm) -> Uniforms {
- skvm::PixelFormat pixelFormat;
- SkAssertResult(SkColorType_to_PixelFormat(pm.colorType(), &pixelFormat));
+ skvm::PixelFormat pixelFormat = skvm::SkColorType_to_PixelFormat(pm.colorType());
return {
p->uniformF(uniforms->pushF( pm.width())),
p->uniformF(uniforms->pushF(1.0f/pm.width())), // iff tileX == kRepeat
diff --git a/src/sksl/SkSLVMGenerator.cpp b/src/sksl/SkSLVMGenerator.cpp
index 337e9d5..1b0ba75 100644
--- a/src/sksl/SkSLVMGenerator.cpp
+++ b/src/sksl/SkSLVMGenerator.cpp
@@ -1803,8 +1803,7 @@
}
auto sampleChild = [&](int i, skvm::Coord coord) {
- skvm::PixelFormat pixelFormat;
- SkColorType_to_PixelFormat(kRGBA_F32_SkColorType, &pixelFormat);
+ skvm::PixelFormat pixelFormat = skvm::SkColorType_to_PixelFormat(kRGBA_F32_SkColorType);
skvm::I32 index = trunc(coord.x);
index += trunc(coord.y) * children[i].rowBytesAsPixels;
return gather(pixelFormat, children[i].addr, index);