added support for layout(offset=...) to skslc

BUG=skia:

Change-Id: Idc1118503f9703496a0c852f0d8840b26e08b9fb
Reviewed-on: https://skia-review.googlesource.com/5283
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>

13 files changed, 100 insertions, 42 deletions

diff --git a/src/sksl/SkSLCodeGenerator.h b/src/sksl/SkSLCodeGenerator.h
index 211cf0aadf..ed93778e13 100644
--- a/src/sksl/SkSLCodeGenerator.h
+++ b/src/sksl/SkSLCodeGenerator.h
@@ -19,8 +19,8 @@ namespace SkSL {
 class CodeGenerator {
 public:
     virtual ~CodeGenerator() {}
-    
-    virtual void generateCode(const Program& program, SkWStream& out) = 0;
+
+    virtual void generateCode(const Program& program, ErrorReporter& errors, SkWStream& out) = 0;
 };
 
 } // namespace
diff --git a/src/sksl/SkSLCompiler.cpp b/src/sksl/SkSLCompiler.cpp
index 003f154908..4990dfb9a4 100644
--- a/src/sksl/SkSLCompiler.cpp
+++ b/src/sksl/SkSLCompiler.cpp
@@ -441,7 +441,8 @@ bool Compiler::toSPIRV(Program::Kind kind, const SkString& text, SkWStream& out)
     auto program = this->convertProgram(kind, text, capsMap);
     if (fErrorCount == 0) {
         SkSL::SPIRVCodeGenerator cg(&fContext);
-        cg.generateCode(*program.get(), out);
+        cg.generateCode(*program.get(), *this, out);
+        this->writeErrorCount();
     }
     return fErrorCount == 0;
 }
@@ -484,7 +485,8 @@ bool Compiler::toGLSL(Program::Kind kind, const SkString& text, const GrGLSLCaps
     auto program = this->convertProgram(kind, text, capsMap);
     if (fErrorCount == 0) {
         SkSL::GLSLCodeGenerator cg(&fContext, &caps);
-        cg.generateCode(*program.get(), out);
+        cg.generateCode(*program.get(), *this, out);
+        this->writeErrorCount();
     }
     return fErrorCount == 0;
 }
diff --git a/src/sksl/SkSLGLSLCodeGenerator.cpp b/src/sksl/SkSLGLSLCodeGenerator.cpp
index 3a6f8c52a6..7c262871ee 100644
--- a/src/sksl/SkSLGLSLCodeGenerator.cpp
+++ b/src/sksl/SkSLGLSLCodeGenerator.cpp
@@ -75,7 +75,7 @@ void GLSLCodeGenerator::writeType(const Type& type) {
             this->writeLine(" " + f.fName + ";");
         }
         fIndentation--;
-        this->writeLine("}");
+        this->write("}");
     } else {
         this->write(type.name());
     }
@@ -655,7 +655,8 @@ void GLSLCodeGenerator::writeReturnStatement(const ReturnStatement& r) {
     this->write(";");
 }
 
-void GLSLCodeGenerator::generateCode(const Program& program, SkWStream& out) {
+void GLSLCodeGenerator::generateCode(const Program& program, ErrorReporter& errors,
+                                     SkWStream& out) {
     ASSERT(fOut == nullptr);
     fOut = &fHeader;
     fProgramKind = program.fKind;
diff --git a/src/sksl/SkSLGLSLCodeGenerator.h b/src/sksl/SkSLGLSLCodeGenerator.h
index b8cb34e84f..05c931a0a7 100644
--- a/src/sksl/SkSLGLSLCodeGenerator.h
+++ b/src/sksl/SkSLGLSLCodeGenerator.h
@@ -76,7 +76,7 @@ public:
     : fContext(*context)
     , fCaps(*caps) {}
 
-    void generateCode(const Program& program, SkWStream& out) override;
+    void generateCode(const Program& program, ErrorReporter& errors, SkWStream& out) override;
 
 private:
     void write(const char* s);
diff --git a/src/sksl/SkSLIRGenerator.cpp b/src/sksl/SkSLIRGenerator.cpp
index 882464a09f..023f1916c8 100644
--- a/src/sksl/SkSLIRGenerator.cpp
+++ b/src/sksl/SkSLIRGenerator.cpp
@@ -524,7 +524,7 @@ std::unique_ptr<InterfaceBlock> IRGenerator::convertInterfaceBlock(const ASTInte
             }
         }
     }
-    Type* type = new Type(intf.fInterfaceName, fields);
+    Type* type = new Type(intf.fPosition, intf.fInterfaceName, fields);
     fSymbolTable->takeOwnership(type);
     SkString name = intf.fValueName.size() > 0 ? intf.fValueName : intf.fInterfaceName;
     Variable* var = new Variable(intf.fPosition, intf.fModifiers, name, *type,
diff --git a/src/sksl/SkSLParser.cpp b/src/sksl/SkSLParser.cpp
index 1b7cd9d725..3920b00127 100644
--- a/src/sksl/SkSLParser.cpp
+++ b/src/sksl/SkSLParser.cpp
@@ -393,7 +393,7 @@ std::unique_ptr<ASTType> Parser::structDeclaration() {
     if (!this->expect(Token::RBRACE, "'}'")) {
         return nullptr;
     }
-    fTypes.add(name.fText, std::unique_ptr<Type>(new Type(name.fText, fields)));
+    fTypes.add(name.fText, std::unique_ptr<Type>(new Type(name.fPosition, name.fText, fields)));
     return std::unique_ptr<ASTType>(new ASTType(name.fPosition, name.fText,
                                                 ASTType::kStruct_Kind));
 }
@@ -539,6 +539,7 @@ int Parser::layoutInt() {
 /* LAYOUT LPAREN IDENTIFIER (EQ INT_LITERAL)? (COMMA IDENTIFIER (EQ INT_LITERAL)?)* RPAREN */
 Layout Parser::layout() {
     int location = -1;
+    int offset = -1;
     int binding = -1;
     int index = -1;
     int set = -1;
@@ -552,7 +553,7 @@ Layout Parser::layout() {
     if (this->peek().fKind == Token::LAYOUT) {
         this->nextToken();
         if (!this->expect(Token::LPAREN, "'('")) {
-            return Layout(location, binding, index, set, builtin, inputAttachmentIndex,
+            return Layout(location, offset, binding, index, set, builtin, inputAttachmentIndex,
                           originUpperLeft, overrideCoverage, blendSupportAllEquations, format,
                           pushConstant);
         }
@@ -560,6 +561,8 @@ Layout Parser::layout() {
             Token t = this->nextToken();
             if (t.fText == "location") {
                 location = this->layoutInt();
+            } else if (t.fText == "offset") {
+                offset = this->layoutInt();
             } else if (t.fText == "binding") {
                 binding = this->layoutInt();
             } else if (t.fText == "index") {
@@ -593,8 +596,9 @@ Layout Parser::layout() {
             }
         }
     }
-    return Layout(location, binding, index, set, builtin, inputAttachmentIndex, originUpperLeft,
-                     overrideCoverage, blendSupportAllEquations, format, pushConstant);
+    return Layout(location, offset, binding, index, set, builtin, inputAttachmentIndex,
+                  originUpperLeft, overrideCoverage, blendSupportAllEquations, format,
+                  pushConstant);
 }
 
 /* layout? (UNIFORM | CONST | IN | OUT | INOUT | LOWP | MEDIUMP | HIGHP | FLAT | NOPERSPECTIVE)* */
diff --git a/src/sksl/SkSLSPIRVCodeGenerator.cpp b/src/sksl/SkSLSPIRVCodeGenerator.cpp
index 089b899bfe..62c0c507ae 100644
--- a/src/sksl/SkSLSPIRVCodeGenerator.cpp
+++ b/src/sksl/SkSLSPIRVCodeGenerator.cpp
@@ -973,13 +973,14 @@ SpvId SPIRVCodeGenerator::nextId() {
     return fIdCount++;
 }
 
-void SPIRVCodeGenerator::writeStruct(const Type& type, const MemoryLayout& layout, SpvId resultId) {
+void SPIRVCodeGenerator::writeStruct(const Type& type, const MemoryLayout& memoryLayout,
+                                     SpvId resultId) {
     this->writeInstruction(SpvOpName, resultId, type.name().c_str(), fNameBuffer);
     // go ahead and write all of the field types, so we don't inadvertently write them while we're
     // in the middle of writing the struct instruction
     std::vector<SpvId> types;
     for (const auto& f : type.fields()) {
-        types.push_back(this->getType(*f.fType, layout));
+        types.push_back(this->getType(*f.fType, memoryLayout));
     }
     this->writeOpCode(SpvOpTypeStruct, 2 + (int32_t) types.size(), fConstantBuffer);
     this->writeWord(resultId, fConstantBuffer);
@@ -988,15 +989,30 @@ void SPIRVCodeGenerator::writeStruct(const Type& type, const MemoryLayout& layou
     }
     size_t offset = 0;
     for (int32_t i = 0; i < (int32_t) type.fields().size(); i++) {
-        size_t size = layout.size(*type.fields()[i].fType);
-        size_t alignment = layout.alignment(*type.fields()[i].fType);
-        size_t mod = offset % alignment;
-        if (mod != 0) {
-            offset += alignment - mod;
+        size_t size = memoryLayout.size(*type.fields()[i].fType);
+        size_t alignment = memoryLayout.alignment(*type.fields()[i].fType);
+        const Layout& fieldLayout = type.fields()[i].fModifiers.fLayout;
+        if (fieldLayout.fOffset >= 0) {
+            if (fieldLayout.fOffset <= (int) offset) {
+                fErrors->error(type.fPosition,
+                               "offset of field '" + type.fields()[i].fName + "' must be at "
+                               "least " + to_string((int) offset));
+            }
+            if (fieldLayout.fOffset % alignment) {
+                fErrors->error(type.fPosition,
+                               "offset of field '" + type.fields()[i].fName + "' must be a multiple"
+                               " of " + to_string((int) alignment));
+            }
+            offset = fieldLayout.fOffset;
+        } else {
+            size_t mod = offset % alignment;
+            if (mod) {
+                offset += alignment - mod;
+            }
         }
         this->writeInstruction(SpvOpMemberName, resultId, i, type.fields()[i].fName.c_str(),
                                fNameBuffer);
-        this->writeLayout(type.fields()[i].fModifiers.fLayout, resultId, i);
+        this->writeLayout(fieldLayout, resultId, i);
         if (type.fields()[i].fModifiers.fLayout.fBuiltin < 0) {
             this->writeInstruction(SpvOpMemberDecorate, resultId, (SpvId) i, SpvDecorationOffset,
                                    (SpvId) offset, fDecorationBuffer);
@@ -1005,7 +1021,7 @@ void SPIRVCodeGenerator::writeStruct(const Type& type, const MemoryLayout& layou
             this->writeInstruction(SpvOpMemberDecorate, resultId, i, SpvDecorationColMajor,
                                    fDecorationBuffer);
             this->writeInstruction(SpvOpMemberDecorate, resultId, i, SpvDecorationMatrixStride,
-                                   (SpvId) layout.stride(*type.fields()[i].fType),
+                                   (SpvId) memoryLayout.stride(*type.fields()[i].fType),
                                    fDecorationBuffer);
         }
         offset += size;
@@ -2711,7 +2727,9 @@ void SPIRVCodeGenerator::writeInstructions(const Program& program, SkWStream& ou
     write_data(*body.detachAsData(), out);
 }
 
-void SPIRVCodeGenerator::generateCode(const Program& program, SkWStream& out) {
+void SPIRVCodeGenerator::generateCode(const Program& program, ErrorReporter& errors,
+                                      SkWStream& out) {
+    fErrors = &errors;
     this->writeWord(SpvMagicNumber, out);
     this->writeWord(SpvVersion, out);
     this->writeWord(SKSL_MAGIC, out);
@@ -2720,6 +2738,7 @@ void SPIRVCodeGenerator::generateCode(const Program& program, SkWStream& out) {
     this->writeWord(fIdCount, out);
     this->writeWord(0, out); // reserved, always zero
     write_data(*buffer.detachAsData(), out);
+    fErrors = nullptr;
 }
 
 }
diff --git a/src/sksl/SkSLSPIRVCodeGenerator.h b/src/sksl/SkSLSPIRVCodeGenerator.h
index fa7c022100..5567ec5404 100644
--- a/src/sksl/SkSLSPIRVCodeGenerator.h
+++ b/src/sksl/SkSLSPIRVCodeGenerator.h
@@ -73,7 +73,7 @@ public:
         this->setupIntrinsics();
     }
 
-    void generateCode(const Program& program, SkWStream& out) override;
+    void generateCode(const Program& program, ErrorReporter& errors, SkWStream& out) override;
 
 private:
     enum IntrinsicKind {
@@ -236,6 +236,7 @@ private:
 
     const Context& fContext;
     const MemoryLayout fDefaultLayout;
+    ErrorReporter* fErrors;
 
     uint64_t fCapabilities;
     SpvId fIdCount;
diff --git a/src/sksl/ir/SkSLLayout.h b/src/sksl/ir/SkSLLayout.h
index b2c158eeef..75650e271d 100644
--- a/src/sksl/ir/SkSLLayout.h
+++ b/src/sksl/ir/SkSLLayout.h
@@ -77,10 +77,11 @@ struct Layout {
         return false;
     }
 
-    Layout(int location, int binding, int index, int set, int builtin, int inputAttachmentIndex,
-           bool originUpperLeft, bool overrideCoverage, bool blendSupportAllEquations,
-           Format format, bool pushconstant)
+    Layout(int location, int offset, int binding, int index, int set, int builtin,
+           int inputAttachmentIndex, bool originUpperLeft, bool overrideCoverage,
+           bool blendSupportAllEquations, Format format, bool pushconstant)
     : fLocation(location)
+    , fOffset(offset)
     , fBinding(binding)
     , fIndex(index)
     , fSet(set)
@@ -94,6 +95,7 @@ struct Layout {
 
     Layout()
     : fLocation(-1)
+    , fOffset(-1)
     , fBinding(-1)
     , fIndex(-1)
     , fSet(-1)
@@ -112,6 +114,10 @@ struct Layout {
             result += separator + "location = " + to_string(fLocation);
             separator = ", ";
         }
+        if (fOffset >= 0) {
+            result += separator + "offset = " + to_string(fOffset);
+            separator = ", ";
+        }
         if (fBinding >= 0) {
             result += separator + "binding = " + to_string(fBinding);
             separator = ", ";
@@ -160,6 +166,7 @@ struct Layout {
 
     bool operator==(const Layout& other) const {
         return fLocation                 == other.fLocation &&
+               fOffset                   == other.fOffset &&
                fBinding                  == other.fBinding &&
                fIndex                    == other.fIndex &&
                fSet                      == other.fSet &&
@@ -176,6 +183,7 @@ struct Layout {
     }
 
     int fLocation;
+    int fOffset;
     int fBinding;
     int fIndex;
     int fSet;
diff --git a/src/sksl/ir/SkSLType.h b/src/sksl/ir/SkSLType.h
index 056022bed8..05e48b01be 100644
--- a/src/sksl/ir/SkSLType.h
+++ b/src/sksl/ir/SkSLType.h
@@ -68,8 +68,8 @@ public:
     }
 
     // Create a struct type with the given fields.
-    Type(SkString name, std::vector<Field> fields)
-    : INHERITED(Position(), kType_Kind, std::move(name))
+    Type(Position position, SkString name, std::vector<Field> fields)
+    : INHERITED(position, kType_Kind, std::move(name))
     , fTypeKind(kStruct_Kind)
     , fFields(std::move(fields)) {}
 
diff --git a/tests/SkSLErrorTest.cpp b/tests/SkSLErrorTest.cpp
index 896b32ccb8..c653072dd1 100644
--- a/tests/SkSLErrorTest.cpp
+++ b/tests/SkSLErrorTest.cpp
@@ -378,4 +378,13 @@ DEF_TEST(SkSLBadCap, r) {
                  "error: 1: unknown capability flag 'bugFreeDriver'\n1 error\n");
 }
 
+DEF_TEST(SkSLBadOffset, r) {
+    test_failure(r,
+                 "struct Bad { layout (offset = 5) int x; } bad; void main() { bad.x = 5; }",
+                 "error: 1: offset of field 'x' must be a multiple of 4\n1 error\n");
+    test_failure(r,
+                 "struct Bad { int x; layout (offset = 0) int y; } bad; void main() { bad.x = 5; }",
+                 "error: 1: offset of field 'y' must be at least 4\n1 error\n");
+}
+
 #endif
diff --git a/tests/SkSLGLSLTest.cpp b/tests/SkSLGLSLTest.cpp
index 52638a5795..490c4379fb 100644
--- a/tests/SkSLGLSLTest.cpp
+++ b/tests/SkSLGLSLTest.cpp
@@ -211,15 +211,13 @@ DEF_TEST(SkSLStructs, r) {
          "struct A {\n"
          "    int x;\n"
          "    int y;\n"
-         "}\n"
-         " a1, a2;\n"
+         "} a1, a2;\n"
          "A a3;\n"
          "struct B {\n"
          "    float x;\n"
          "    float[2] y;\n"
          "    layout (binding = 1) A z;\n"
-         "}\n"
-         " b1, b2, b3;\n"
+         "} b1, b2, b3;\n"
          "void main() {\n"
          "}\n");
 }
@@ -589,4 +587,20 @@ DEF_TEST(SkSLTexture, r) {
          "}\n");
 }
 
+DEF_TEST(SkSLOffset, r) {
+    test(r,
+         "struct Test {"
+         "layout(offset = 0) int x;"
+         "layout(offset = 4) int y;"
+         "int z;"
+         "} test;",
+         *SkSL::GLSLCapsFactory::Default(),
+         "#version 400\n"
+         "out vec4 sk_FragColor;\n"
+         "struct Test {\n"
+         "    layout (offset = 0) int x;\n"
+         "    layout (offset = 4) int y;\n"
+         "    int z;\n"
+         "} test;\n");
+}
 #endif
diff --git a/tests/SkSLMemoryLayoutTest.cpp b/tests/SkSLMemoryLayoutTest.cpp
index 336d8aa477..ff3d0105bd 100644
--- a/tests/SkSLMemoryLayoutTest.cpp
+++ b/tests/SkSLMemoryLayoutTest.cpp
@@ -55,29 +55,29 @@ DEF_TEST(SkSLMemoryLayout140Test, r) {
     // struct 1
     std::vector<SkSL::Type::Field> fields1;
     fields1.emplace_back(SkSL::Modifiers(), SkString("a"), context.fVec3_Type.get());
-    SkSL::Type s1(SkString("s1"), fields1);
+    SkSL::Type s1(SkSL::Position(), SkString("s1"), fields1);
     REPORTER_ASSERT(r, 16 == layout.size(s1));
     REPORTER_ASSERT(r, 16 == layout.alignment(s1));
 
     fields1.emplace_back(SkSL::Modifiers(), SkString("b"), context.fFloat_Type.get());
-    SkSL::Type s2(SkString("s2"), fields1);
+    SkSL::Type s2(SkSL::Position(), SkString("s2"), fields1);
     REPORTER_ASSERT(r, 16 == layout.size(s2));
     REPORTER_ASSERT(r, 16 == layout.alignment(s2));
 
     fields1.emplace_back(SkSL::Modifiers(), SkString("c"), context.fBool_Type.get());
-    SkSL::Type s3(SkString("s3"), fields1);
+    SkSL::Type s3(SkSL::Position(), SkString("s3"), fields1);
     REPORTER_ASSERT(r, 32 == layout.size(s3));
     REPORTER_ASSERT(r, 16 == layout.alignment(s3));
 
     // struct 2
     std::vector<SkSL::Type::Field> fields2;
     fields2.emplace_back(SkSL::Modifiers(), SkString("a"), context.fInt_Type.get());
-    SkSL::Type s4(SkString("s4"), fields2);
+    SkSL::Type s4(SkSL::Position(), SkString("s4"), fields2);
     REPORTER_ASSERT(r, 16 == layout.size(s4));
     REPORTER_ASSERT(r, 16 == layout.alignment(s4));
 
     fields2.emplace_back(SkSL::Modifiers(), SkString("b"), context.fVec3_Type.get());
-    SkSL::Type s5(SkString("s5"), fields2);
+    SkSL::Type s5(SkSL::Position(), SkString("s5"), fields2);
     REPORTER_ASSERT(r, 32 == layout.size(s5));
     REPORTER_ASSERT(r, 16 == layout.alignment(s5));
 
@@ -136,29 +136,29 @@ DEF_TEST(SkSLMemoryLayout430Test, r) {
     // struct 1
     std::vector<SkSL::Type::Field> fields1;
     fields1.emplace_back(SkSL::Modifiers(), SkString("a"), context.fVec3_Type.get());
-    SkSL::Type s1(SkString("s1"), fields1);
+    SkSL::Type s1(SkSL::Position(), SkString("s1"), fields1);
     REPORTER_ASSERT(r, 16 == layout.size(s1));
     REPORTER_ASSERT(r, 16 == layout.alignment(s1));
 
     fields1.emplace_back(SkSL::Modifiers(), SkString("b"), context.fFloat_Type.get());
-    SkSL::Type s2(SkString("s2"), fields1);
+    SkSL::Type s2(SkSL::Position(), SkString("s2"), fields1);
     REPORTER_ASSERT(r, 16 == layout.size(s2));
     REPORTER_ASSERT(r, 16 == layout.alignment(s2));
 
     fields1.emplace_back(SkSL::Modifiers(), SkString("c"), context.fBool_Type.get());
-    SkSL::Type s3(SkString("s3"), fields1);
+    SkSL::Type s3(SkSL::Position(), SkString("s3"), fields1);
     REPORTER_ASSERT(r, 32 == layout.size(s3));
     REPORTER_ASSERT(r, 16 == layout.alignment(s3));
 
     // struct 2
     std::vector<SkSL::Type::Field> fields2;
     fields2.emplace_back(SkSL::Modifiers(), SkString("a"), context.fInt_Type.get());
-    SkSL::Type s4(SkString("s4"), fields2);
+    SkSL::Type s4(SkSL::Position(), SkString("s4"), fields2);
     REPORTER_ASSERT(r, 4 == layout.size(s4));
     REPORTER_ASSERT(r, 4 == layout.alignment(s4));
 
     fields2.emplace_back(SkSL::Modifiers(), SkString("b"), context.fVec3_Type.get());
-    SkSL::Type s5(SkString("s5"), fields2);
+    SkSL::Type s5(SkSL::Position(), SkString("s5"), fields2);
     REPORTER_ASSERT(r, 32 == layout.size(s5));
     REPORTER_ASSERT(r, 16 == layout.alignment(s5));