Fix field access and indexing of complex expressions

Evaluating either kind of expression now works like all other
expressions - evaluate the inner part, then work with the resulting
values. Added unit tests for both of these that previously failed.

With this change, writeVariableExpression is only used for
VariableReference expressions, so adjust that, too.

Bug: skia:11178
Change-Id: Ia595be473b55f4bb03ec25897f9929835177257c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/358529
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
This commit is contained in:
Brian Osman 2021-01-25 12:03:40 -05:00 committed by Skia Commit-Bot
parent 555b7a4d77
commit f619079545
2 changed files with 77 additions and 23 deletions

View File

@ -219,17 +219,22 @@ private:
return fConditionMask & fLoopMask & ~currentFunction().fReturned;
}
size_t fieldSlotOffset(const FieldAccess& expr);
size_t indexSlotOffset(const IndexExpression& expr);
Value writeExpression(const Expression& expr);
Value writeBinaryExpression(const BinaryExpression& b);
Value writeConstructor(const Constructor& c);
Value writeFunctionCall(const FunctionCall& c);
Value writeExternalFunctionCall(const ExternalFunctionCall& c);
Value writeFieldAccess(const FieldAccess& expr);
Value writeIndexExpression(const IndexExpression& expr);
Value writeIntrinsicCall(const FunctionCall& c);
Value writePostfixExpression(const PostfixExpression& p);
Value writePrefixExpression(const PrefixExpression& p);
Value writeSwizzle(const Swizzle& swizzle);
Value writeTernaryExpression(const TernaryExpression& t);
Value writeVariableExpression(const Expression& expr);
Value writeVariableExpression(const VariableReference& expr);
void writeStatement(const Statement& s);
void writeBlock(const Block& b);
@ -501,27 +506,11 @@ SkVMGenerator::Slot SkVMGenerator::getSlot(const Expression& e) {
switch (e.kind()) {
case Expression::Kind::kFieldAccess: {
const FieldAccess& f = e.as<FieldAccess>();
Slot slot = this->getSlot(*f.base());
for (int i = 0; i < f.fieldIndex(); ++i) {
slot += slot_count(*f.base()->type().fields()[i].fType);
}
return slot;
return this->getSlot(*f.base()) + this->fieldSlotOffset(f);
}
case Expression::Kind::kIndex: {
const IndexExpression& i = e.as<IndexExpression>();
Slot baseSlot = this->getSlot(*i.base());
Value index = this->writeExpression(*i.index());
int indexValue = -1;
SkAssertResult(fBuilder->allImm(index[0], &indexValue));
// When indexing by a literal, the front-end guarantees that we don't go out of bounds.
// But when indexing by a loop variable, it's possible to generate out-of-bounds access.
// The GLSL spec leaves that behavior undefined - we'll just clamp everything here.
indexValue = SkTPin(indexValue, 0, i.base()->type().columns() - 1);
size_t stride = slot_count(i.type());
return baseSlot + indexValue * stride;
return this->getSlot(*i.base()) + this->indexSlotOffset(i);
}
case Expression::Kind::kVariableReference:
return this->getSlot(*e.as<VariableReference>().variable());
@ -824,9 +813,51 @@ Value SkVMGenerator::writeConstructor(const Constructor& c) {
return {};
}
Value SkVMGenerator::writeVariableExpression(const Expression& e) {
Slot slot = this->getSlot(e);
Value val(slot_count(e.type()));
size_t SkVMGenerator::fieldSlotOffset(const FieldAccess& expr) {
Slot offset = 0;
for (int i = 0; i < expr.fieldIndex(); ++i) {
offset += slot_count(*expr.base()->type().fields()[i].fType);
}
return offset;
}
Value SkVMGenerator::writeFieldAccess(const FieldAccess& expr) {
Value base = this->writeExpression(*expr.base());
Value field(slot_count(expr.type()));
size_t offset = this->fieldSlotOffset(expr);
for (size_t i = 0; i < field.slots(); ++i) {
field[i] = base[offset + i];
}
return field;
}
size_t SkVMGenerator::indexSlotOffset(const IndexExpression& expr) {
Value index = this->writeExpression(*expr.index());
int indexValue = -1;
SkAssertResult(fBuilder->allImm(index[0], &indexValue));
// When indexing by a literal, the front-end guarantees that we don't go out of bounds.
// But when indexing by a loop variable, it's possible to generate out-of-bounds access.
// The GLSL spec leaves that behavior undefined - we'll just clamp everything here.
indexValue = SkTPin(indexValue, 0, expr.base()->type().columns() - 1);
size_t stride = slot_count(expr.type());
return indexValue * stride;
}
Value SkVMGenerator::writeIndexExpression(const IndexExpression& expr) {
Value base = this->writeExpression(*expr.base());
Value element(slot_count(expr.type()));
size_t offset = this->indexSlotOffset(expr);
for (size_t i = 0; i < element.slots(); ++i) {
element[i] = base[offset + i];
}
return element;
}
Value SkVMGenerator::writeVariableExpression(const VariableReference& expr) {
Slot slot = this->getSlot(*expr.variable());
Value val(slot_count(expr.type()));
for (size_t i = 0; i < val.slots(); ++i) {
val[i] = fSlots[slot + i];
}
@ -1335,9 +1366,11 @@ Value SkVMGenerator::writeExpression(const Expression& e) {
case Expression::Kind::kConstructor:
return this->writeConstructor(e.as<Constructor>());
case Expression::Kind::kFieldAccess:
return this->writeFieldAccess(e.as<FieldAccess>());
case Expression::Kind::kIndex:
return this->writeIndexExpression(e.as<IndexExpression>());
case Expression::Kind::kVariableReference:
return this->writeVariableExpression(e);
return this->writeVariableExpression(e.as<VariableReference>());
case Expression::Kind::kFloatLiteral:
return fBuilder->splat(e.as<FloatLiteral>().value());
case Expression::Kind::kFunctionCall:

View File

@ -444,6 +444,27 @@ DEF_TEST(SkSLInterpreterGeneric, r) {
test(r, "float2 main(float x, float y) { return float2(x * x, y * y); }", value2, expected2);
}
DEF_TEST(SkSLInterpreterFieldAccessComplex, r) {
const char* src = R"(
struct P { float x; float y; };
P make_point() { P p; p.x = 7; p.y = 3; return p; }
float main() { return make_point().y; }
)";
float expected = 3.0f;
test(r, src, /*in=*/nullptr, &expected);
}
DEF_TEST(SkSLInterpreterIndexComplex, r) {
const char* src = R"(
float2x2 make_mtx() { return float2x2(1, 2, 3, 4); }
float main() { return make_mtx()[1][0]; }
)";
float expected = 3.0f;
test(r, src, /*in=*/nullptr, &expected);
}
DEF_TEST(SkSLInterpreterCompound, r) {
struct RectAndColor { SkIRect fRect; SkColor4f fColor; };
struct ManyRects { int fNumRects; RectAndColor fRects[4]; };