Rename SkSL 'srgb_unpremul' to just 'color'

Added comments to explain the semantics (both what's expected when you
set the uniform, and what you see in the shader). The old name was
confusing, because it sounded like you got an sRGB color in the shader.
This is terse, but I think it's the cleanest syntax - and for embedding
clients, they can use C++ (etc.) API to require that color uniforms are
assigned from color types.

Bug: skia:10479
Change-Id: If00ea754060494aaa83001a5b357687953de8a5f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/480577
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Derek Sollenberger <djsollen@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
This commit is contained in:
Brian Osman 2021-12-06 10:30:51 -05:00 committed by SkCQ
parent 85cc1bece7
commit 8c3d183cc2
14 changed files with 45 additions and 39 deletions

View File

@ -189,8 +189,8 @@ public:
SpiralRT() : RuntimeShaderGM("spiral_rt", {512, 512}, R"(
uniform float rad_scale;
uniform float2 in_center;
layout(srgb_unpremul) uniform float4 in_colors0;
layout(srgb_unpremul) uniform float4 in_colors1;
layout(color) uniform float4 in_colors0;
layout(color) uniform float4 in_colors1;
half4 main(float2 p) {
float2 pp = p - in_center;
@ -209,8 +209,8 @@ public:
builder.uniform("rad_scale") = std::sin(fSecs * 0.5f + 2.0f) / 5;
builder.uniform("in_center") = SkV2{256, 256};
builder.uniform("in_colors0") = SkV4{1, 0, 0, 1};
builder.uniform("in_colors1") = SkV4{0, 1, 0, 1};
builder.uniform("in_colors0") = SkColors::kRed;
builder.uniform("in_colors1") = SkColors::kGreen;
SkPaint paint;
paint.setShader(builder.makeShader(nullptr, true));

View File

@ -65,8 +65,13 @@ public:
};
enum Flags {
kArray_Flag = 0x1,
kSRGBUnpremul_Flag = 0x2,
// Uniform is an declared as an array. 'count' contains array length.
kArray_Flag = 0x1,
// Uniform is declared with layout(color). Colors should be supplied as unpremultiplied,
// extended-range (unclamped) sRGB (ie SkColor4f). The uniform will be automatically
// transformed to unpremultiplied extended-range working-space colors.
kColor_Flag = 0x2,
};
SkString name;
@ -76,6 +81,7 @@ public:
uint32_t flags;
bool isArray() const { return SkToBool(this->flags & kArray_Flag); }
bool isColor() const { return SkToBool(this->flags & kColor_Flag); }
size_t sizeInBytes() const;
};

View File

@ -22,7 +22,7 @@ struct Layout {
kOriginUpperLeft_Flag = 1 << 0,
kPushConstant_Flag = 1 << 1,
kBlendSupportAllEquations_Flag = 1 << 2,
kSRGBUnpremul_Flag = 1 << 3,
kColor_Flag = 1 << 3,
// These flags indicate if the qualifier appeared, regardless of the accompanying value.
kLocation_Flag = 1 << 4,
@ -100,8 +100,8 @@ struct Layout {
if (fFlags & kPushConstant_Flag) {
result += separator() + "push_constant";
}
if (fFlags & kSRGBUnpremul_Flag) {
result += separator() + "srgb_unpremul";
if (fFlags & kColor_Flag) {
result += separator() + "color";
}
if (result.size() > 0) {
result = "layout (" + result + ")";

View File

@ -34,8 +34,8 @@ public:
"blend_support_all_equations", pos);
}
DSLLayout& srgbUnpremul(PositionInfo pos = PositionInfo::Capture()) {
return this->flag(SkSL::Layout::kSRGBUnpremul_Flag, "srgb_unpremul", pos);
DSLLayout& color(PositionInfo pos = PositionInfo::Capture()) {
return this->flag(SkSL::Layout::kColor_Flag, "color", pos);
}
DSLLayout& location(int location, PositionInfo pos = PositionInfo::Capture()) {

View File

@ -2,7 +2,7 @@ layout (
origin_upper_left,
push_constant,
blend_support_all_equations,
srgb_unpremul,
color,
location = 1,
offset = 1,
binding = 1,
@ -17,7 +17,7 @@ layout (
origin_upper_left,
push_constant,
blend_support_all_equations,
srgb_unpremul,
color,
location = 1,
offset = 1,
binding = 1,

View File

@ -2,7 +2,7 @@ layout (
origin_upper_left,
push_constant,
blend_support_all_equations,
srgb_unpremul,
color,
location = 1,
offset = 1,
binding = 1,
@ -14,7 +14,7 @@ layout (
origin_upper_left,
push_constant,
blend_support_all_equations,
srgb_unpremul,
color,
location = 2,
offset = 2,
binding = 2,

View File

@ -344,8 +344,8 @@ SkRuntimeEffect::Result SkRuntimeEffect::MakeInternal(std::unique_ptr<SkSL::Prog
RETURN_FAILURE("Invalid uniform type: '%s'", type->displayName().c_str());
}
if (var.modifiers().fLayout.fFlags & SkSL::Layout::Flag::kSRGBUnpremul_Flag) {
uni.flags |= Uniform::kSRGBUnpremul_Flag;
if (var.modifiers().fLayout.fFlags & SkSL::Layout::Flag::kColor_Flag) {
uni.flags |= Uniform::kColor_Flag;
}
uni.offset = offset;
@ -736,7 +736,7 @@ static sk_sp<SkData> get_xformed_uniforms(const SkRuntimeEffect* effect,
};
for (const auto& v : effect->uniforms()) {
if (v.flags & Flags::kSRGBUnpremul_Flag) {
if (v.flags & Flags::kColor_Flag) {
SkASSERT(v.type == Type::kFloat3 || v.type == Type::kFloat4);
if (steps.flags.mask()) {
float* color = SkTAddOffset<float>(writableData(), v.offset);

View File

@ -92,7 +92,7 @@ void DSLParser::InitLayoutMap() {
TOKEN(ORIGIN_UPPER_LEFT, "origin_upper_left");
TOKEN(BLEND_SUPPORT_ALL_EQUATIONS, "blend_support_all_equations");
TOKEN(PUSH_CONSTANT, "push_constant");
TOKEN(SRGB_UNPREMUL, "srgb_unpremul");
TOKEN(COLOR, "color");
#undef TOKEN
}
@ -723,8 +723,8 @@ DSLLayout DSLParser::layout() {
case LayoutToken::BLEND_SUPPORT_ALL_EQUATIONS:
result.blendSupportAllEquations(this->position(t));
break;
case LayoutToken::SRGB_UNPREMUL:
result.srgbUnpremul(this->position(t));
case LayoutToken::COLOR:
result.color(this->position(t));
break;
case LayoutToken::LOCATION:
result.location(this->layoutInt(), this->position(t));

View File

@ -41,7 +41,7 @@ public:
ORIGIN_UPPER_LEFT,
BLEND_SUPPORT_ALL_EQUATIONS,
PUSH_CONSTANT,
SRGB_UNPREMUL,
COLOR,
};
DSLParser(Compiler* compiler, const ProgramSettings& settings, ProgramKind kind,

View File

@ -47,7 +47,7 @@ bool Modifiers::checkPermitted(const Context& context, int line, int permittedMo
{ Layout::kOriginUpperLeft_Flag, "origin_upper_left"},
{ Layout::kPushConstant_Flag, "push_constant"},
{ Layout::kBlendSupportAllEquations_Flag, "blend_support_all_equations"},
{ Layout::kSRGBUnpremul_Flag, "srgb_unpremul"},
{ Layout::kColor_Flag, "color"},
{ Layout::kLocation_Flag, "location"},
{ Layout::kOffset_Flag, "offset"},
{ Layout::kBinding_Flag, "binding"},

View File

@ -72,13 +72,13 @@ void VarDeclaration::ErrorCheck(const Context& context,
context.fErrors->error(line,
"variables of type '" + baseType->displayName() + "' must be uniform");
}
if (modifiers.fLayout.fFlags & Layout::kSRGBUnpremul_Flag) {
if (modifiers.fLayout.fFlags & Layout::kColor_Flag) {
if (!ProgramConfig::IsRuntimeEffect(context.fConfig->fKind)) {
context.fErrors->error(line, "'srgb_unpremul' is only permitted in runtime effects");
context.fErrors->error(line, "'layout(color)' is only permitted in runtime effects");
}
if (!(modifiers.fFlags & Modifiers::kUniform_Flag)) {
context.fErrors->error(line,
"'srgb_unpremul' is only permitted on 'uniform' variables");
"'layout(color)' is only permitted on 'uniform' variables");
}
auto validColorXformType = [](const Type& t) {
return t.isVector() && t.componentType().isFloat() &&
@ -86,8 +86,9 @@ void VarDeclaration::ErrorCheck(const Context& context,
};
if (!validColorXformType(*baseType) && !(baseType->isArray() &&
validColorXformType(baseType->componentType()))) {
context.fErrors->error(line, "'srgb_unpremul' is only permitted on half3, half4, "
"float3, or float4 variables");
context.fErrors->error(line,
"'layout(color)' is not permitted on variables of type '" +
baseType->displayName() + "'");
}
}
int permitted = Modifiers::kConst_Flag | Modifiers::kHighp_Flag | Modifiers::kMediump_Flag |

View File

@ -1896,9 +1896,8 @@ DEF_GPUTEST_FOR_MOCK_CONTEXT(DSLLayout, r, ctxInfo) {
EXPECT_EQUAL(Declare(v5), "layout (blend_support_all_equations) half4 v5;");
{
ExpectError error(r, "'srgb_unpremul' is only permitted in runtime effects");
DSLGlobalVar v(DSLModifiers(DSLLayout().srgbUnpremul(), kUniform_Modifier), kHalf4_Type,
"v");
ExpectError error(r, "'layout(color)' is only permitted in runtime effects");
DSLGlobalVar v(DSLModifiers(DSLLayout().color(), kUniform_Modifier), kHalf4_Type, "v");
Declare(v);
}
@ -1954,8 +1953,8 @@ DEF_GPUTEST_FOR_MOCK_CONTEXT(DSLLayout, r, ctxInfo) {
}
{
ExpectError error(r, "layout qualifier 'srgb_unpremul' appears more than once");
DSLLayout().srgbUnpremul().srgbUnpremul();
ExpectError error(r, "layout qualifier 'color' appears more than once");
DSLLayout().color().color();
}
}

View File

@ -3,7 +3,7 @@
error: 13: layout qualifier 'origin_upper_left' is not permitted here
error: 13: layout qualifier 'push_constant' is not permitted here
error: 13: layout qualifier 'blend_support_all_equations' is not permitted here
error: 13: layout qualifier 'srgb_unpremul' is not permitted here
error: 13: layout qualifier 'color' is not permitted here
error: 13: layout qualifier 'location' is not permitted here
error: 13: layout qualifier 'offset' is not permitted here
error: 13: layout qualifier 'binding' is not permitted here
@ -14,7 +14,7 @@ error: 13: layout qualifier 'input_attachment_index' is not permitted here
error: 27: layout qualifier 'origin_upper_left' is not permitted here
error: 27: layout qualifier 'push_constant' is not permitted here
error: 27: layout qualifier 'blend_support_all_equations' is not permitted here
error: 27: layout qualifier 'srgb_unpremul' is not permitted here
error: 27: layout qualifier 'color' is not permitted here
error: 27: layout qualifier 'location' is not permitted here
error: 27: layout qualifier 'offset' is not permitted here
error: 27: layout qualifier 'binding' is not permitted here

View File

@ -3,7 +3,7 @@
error: 14: layout qualifier 'origin_upper_left' appears more than once
error: 15: layout qualifier 'push_constant' appears more than once
error: 16: layout qualifier 'blend_support_all_equations' appears more than once
error: 17: layout qualifier 'srgb_unpremul' appears more than once
error: 17: layout qualifier 'color' appears more than once
error: 18: layout qualifier 'location' appears more than once
error: 19: layout qualifier 'offset' appears more than once
error: 20: layout qualifier 'binding' appears more than once
@ -11,7 +11,7 @@ error: 21: layout qualifier 'index' appears more than once
error: 22: layout qualifier 'set' appears more than once
error: 23: layout qualifier 'builtin' appears more than once
error: 24: layout qualifier 'input_attachment_index' appears more than once
error: 25: 'srgb_unpremul' is only permitted in runtime effects
error: 25: 'srgb_unpremul' is only permitted on 'uniform' variables
error: 25: 'srgb_unpremul' is only permitted on half3, half4, float3, or float4 variables
error: 25: 'layout(color)' is only permitted in runtime effects
error: 25: 'layout(color)' is only permitted on 'uniform' variables
error: 25: 'layout(color)' is not permitted on variables of type 'float'
14 errors