commit e435bd7c9b0ef7cc8b292a37e54480313bd6999f
author Arman Uguray <armansito@google.com> Wed May 18 10:11:44 2022 -0400
committer SkCQ <skcq-be@skia-corp.google.com.iam.gserviceaccount.com> Wed May 18 16:57:15 2022 +0000
tree 4628f5f8073f5c257041e3e0e755a6901378b6f5
parent 6fc86db5e46c16eed47532a2b5669974ee4433df

[sksl] Add SK_POINTSIZE_BUILTIN constant

GLSL, Metal, and WGSL code generators now determine the sk_PointSize
builtin by modifier value instead of by name. Also updated GLSL code
generator to use SK_POSITION_BUILTIN when processing sk_Position.

Change-Id: I57418f66786dd2bac8c4a7d8c34f83221daf505c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/541719
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Arman Uguray <armansito@google.com>

src/sksl/SkSLCompiler.h

diff --git a/src/sksl/SkSLCompiler.h b/src/sksl/SkSLCompiler.h
index eb20bdf..e1a6450 100644
--- a/src/sksl/SkSLCompiler.h
+++ b/src/sksl/SkSLCompiler.h
@@ -40,6 +40,7 @@
 #define SK_VERTEXID_BUILTIN               42
 #define SK_INSTANCEID_BUILTIN             43
 #define SK_POSITION_BUILTIN                0
+#define SK_POINTSIZE_BUILTIN               1
 
 class SkSLCompileBench;
 

src/sksl/codegen/SkSLGLSLCodeGenerator.cpp

diff --git a/src/sksl/codegen/SkSLGLSLCodeGenerator.cpp b/src/sksl/codegen/SkSLGLSLCodeGenerator.cpp
index ac6d6b1..e7e75c6 100644
--- a/src/sksl/codegen/SkSLGLSLCodeGenerator.cpp
+++ b/src/sksl/codegen/SkSLGLSLCodeGenerator.cpp
@@ -61,7 +61,6 @@
 #include "src/sksl/spirv.h"
 
 #include <memory>
-#include <type_traits>
 #include <vector>
 
 namespace SkSL {
@@ -853,7 +852,8 @@
 }
 
 bool is_sk_position(const FieldAccess& f) {
-    return "sk_Position" == f.base()->type().fields()[f.fieldIndex()].fName;
+    return f.base()->type().fields()[f.fieldIndex()].fModifiers.fLayout.fBuiltin ==
+           SK_POSITION_BUILTIN;
 }
 
 void GLSLCodeGenerator::writeFieldAccess(const FieldAccess& f) {
@@ -862,10 +862,10 @@
         this->write(".");
     }
     const Type& baseType = f.base()->type();
-    std::string_view name = baseType.fields()[f.fieldIndex()].fName;
-    if (name == "sk_Position") {
+    int builtin = baseType.fields()[f.fieldIndex()].fModifiers.fLayout.fBuiltin;
+    if (builtin == SK_POSITION_BUILTIN) {
         this->write("gl_Position");
-    } else if (name == "sk_PointSize") {
+    } else if (builtin == SK_POINTSIZE_BUILTIN) {
         this->write("gl_PointSize");
     } else {
         this->write(baseType.fields()[f.fieldIndex()].fName);

src/sksl/codegen/SkSLMetalCodeGenerator.cpp

diff --git a/src/sksl/codegen/SkSLMetalCodeGenerator.cpp b/src/sksl/codegen/SkSLMetalCodeGenerator.cpp
index 63d2a9b..6df6934 100644
--- a/src/sksl/codegen/SkSLMetalCodeGenerator.cpp
+++ b/src/sksl/codegen/SkSLMetalCodeGenerator.cpp
@@ -1416,17 +1416,16 @@
         case SK_POSITION_BUILTIN:
             this->write("_out.sk_Position");
             break;
+        case SK_POINTSIZE_BUILTIN:
+            this->write("_out.sk_PointSize");
+            break;
         default:
-            if (field->fName == "sk_PointSize") {
-                this->write("_out.sk_PointSize");
-            } else {
-                if (FieldAccess::OwnerKind::kAnonymousInterfaceBlock == f.ownerKind()) {
-                    this->write("_globals.");
-                    this->write(fInterfaceBlockNameMap[fInterfaceBlockMap[field]]);
-                    this->write("->");
-                }
-                this->writeName(field->fName);
+            if (FieldAccess::OwnerKind::kAnonymousInterfaceBlock == f.ownerKind()) {
+                this->write("_globals.");
+                this->write(fInterfaceBlockNameMap[fInterfaceBlockMap[field]]);
+                this->write("->");
             }
+            this->writeName(field->fName);
     }
 }
 

src/sksl/codegen/SkSLWGSLCodeGenerator.cpp

diff --git a/src/sksl/codegen/SkSLWGSLCodeGenerator.cpp b/src/sksl/codegen/SkSLWGSLCodeGenerator.cpp
index 72e3af7..da962ba 100644
--- a/src/sksl/codegen/SkSLWGSLCodeGenerator.cpp
+++ b/src/sksl/codegen/SkSLWGSLCodeGenerator.cpp
@@ -819,7 +819,8 @@
         const Variable& v = *f.base()->as<VariableReference>().variable();
         if (v.modifiers().fFlags & Modifiers::kIn_Flag) {
             this->write("_stageIn.");
-        } else if (v.modifiers().fFlags & Modifiers::kOut_Flag && field->fName != "sk_PointSize") {
+        } else if (v.modifiers().fFlags & Modifiers::kOut_Flag &&
+                   field->fModifiers.fLayout.fBuiltin != SK_POINTSIZE_BUILTIN) {
             this->write("(*_stageOut).");
         } else {
             // TODO(skia:13902): Reference the variable using the base name used for its
@@ -1048,7 +1049,7 @@
                             f.fModifiers, *f.fType, f.fName, Delimiter::kComma);
                     if (f.fModifiers.fLayout.fBuiltin == SK_POSITION_BUILTIN) {
                         declaredPositionBuiltin = true;
-                    } else if (f.fName == "sk_PointSize") {
+                    } else if (f.fModifiers.fLayout.fBuiltin == SK_POINTSIZE_BUILTIN) {
                         // sk_PointSize is explicitly not supported by `builtin_from_sksl_name` so
                         // writePipelineIODeclaration will never write it. We mark it here if the
                         // declaration is needed so we can synthesize it below.