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>
This commit is contained in:
Mike Klein 2021-02-08 09:46:59 -06:00 committed by Skia Commit-Bot
parent 4c0a35f9d6
commit 447f33105a
6 changed files with 42 additions and 69 deletions

View File

@ -1023,41 +1023,42 @@ namespace skvm {
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 @@ namespace skvm {
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 @@ namespace skvm {
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 @@ namespace skvm {
}
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) {

View File

@ -548,7 +548,7 @@ namespace skvm {
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 @@ namespace skvm {
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 @@ namespace skvm {
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); }

View File

@ -170,12 +170,6 @@ namespace {
}
}
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 @@ namespace {
}
// 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 @@ namespace {
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 @@ namespace {
}
// Write it out!
SkAssertResult(store(dstFormat, dst_ptr, src));
store(dstFormat, dst_ptr, src);
}
@ -365,8 +357,7 @@ namespace {
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 @@ SkBlitter* SkCreateSkVMSpriteBlitter(const SkPixmap& device,
// 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);

View File

@ -879,15 +879,6 @@ skvm::Color SkImageShader::onProgram(skvm::Builder* p,
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 @@ skvm::Color SkImageShader::onProgram(skvm::Builder* p,
};
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

View File

@ -1803,8 +1803,7 @@ bool testingOnly_ProgramToSkVMShader(const Program& program, skvm::Builder* buil
}
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);

View File

@ -2251,10 +2251,8 @@ DEF_TEST(SkVM_128bit, r) {
floats[i] = i * (1/255.0f);
}
skvm::PixelFormat rgba_ffff,
rgba_8888;
skvm::SkColorType_to_PixelFormat(kRGBA_F32_SkColorType , &rgba_ffff);
skvm::SkColorType_to_PixelFormat(kRGBA_8888_SkColorType, &rgba_8888);
skvm::PixelFormat rgba_ffff = skvm::SkColorType_to_PixelFormat(kRGBA_F32_SkColorType),
rgba_8888 = skvm::SkColorType_to_PixelFormat(kRGBA_8888_SkColorType);
{ // Convert RGBA F32 to RGBA 8888, testing 128-bit loads.
skvm::Builder b;