Reland "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.
Reland now safe, after fix to Value::operator[]
This reverts commit 1ea6d6051e325204516661caee0efd4be4a48ddb.
Bug: skia:11178
Cq-Include-Trybots: luci.skia.skia.primary:Test-Debian10-GCC-GCE-CPU-AVX2-x86-Debug-All-Docker
Change-Id: I14782fcdfef33a47a46334447c5847976721b21f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/359564
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
diff --git a/src/sksl/SkSLVMGenerator.cpp b/src/sksl/SkSLVMGenerator.cpp
index 352c64d..152d39b 100644
--- a/src/sksl/SkSLVMGenerator.cpp
+++ b/src/sksl/SkSLVMGenerator.cpp
@@ -235,17 +235,22 @@
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);
@@ -523,27 +528,11 @@
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());
@@ -846,9 +835,51 @@
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];
}
@@ -1399,9 +1430,11 @@
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:
diff --git a/tests/SkSLInterpreterTest.cpp b/tests/SkSLInterpreterTest.cpp
index f1ecb20..04342a9 100644
--- a/tests/SkSLInterpreterTest.cpp
+++ b/tests/SkSLInterpreterTest.cpp
@@ -444,6 +444,27 @@
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]; };
