Compat: Switch interpolate params to pipeline errors

This turned into a bigger CL because the it removes the last
need for ValidationMode which causes an error that mode_ in
Validator was unused.

Fixed: 357042303,357047899
Bug: 357042303,357047899
Change-Id: I87812d183a054a7869d04bc281cb9ea75b637976
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/201434
Reviewed-by: James Price <[email protected]>
Commit-Queue: Gregg Tavares <[email protected]>

Author: greggman

src/dawn/native/RenderPipeline.cpp

@@ -812,6 +812,29 @@ MaybeError ValidateInterStageMatching(DeviceBase* device,
             "different from the interpolation sampling (%s) of the fragment input at "
             "location %u.",
             vertexOutputInfo.interpolationSampling, i, fragmentInputInfo.interpolationSampling, i);
+
+        if (device->IsCompatibilityMode()) {
+            DAWN_INVALID_IF(
+                vertexOutputInfo.interpolationType == InterpolationType::Linear,
+                "The interpolation type (%s) of the vertex output at location %u is not "
+                "supported in compatibility mode",
+                vertexOutputInfo.interpolationType, i);
+
+            DAWN_INVALID_IF(
+                vertexOutputInfo.interpolationSampling == InterpolationSampling::Sample ||
+                    vertexOutputInfo.interpolationSampling == InterpolationSampling::First,
+                "The interpolation sampling (%s) of the vertex output at location %u is "
+                "not supported in compatibility mode",
+                vertexOutputInfo.interpolationSampling, i);
+
+            DAWN_INVALID_IF(
+                vertexOutputInfo.interpolationType == InterpolationType::Flat &&
+                    vertexOutputInfo.interpolationSampling == InterpolationSampling::None,
+                "The interpolation sampling (%s) of the vertex output at location %u when "
+                "interpolation type is (%s)"
+                "not supported in compatibility mode",
+                vertexOutputInfo.interpolationSampling, i, vertexOutputInfo.interpolationType);
+        }
     }
 
     return {};

src/dawn/native/ShaderModule.cpp

@@ -352,11 +352,9 @@ ResultOrError<PixelLocalMemberType> FromTintPixelLocalMemberType(
 
 ResultOrError<tint::Program> ParseWGSL(const tint::Source::File* file,
                                        const tint::wgsl::AllowedFeatures& allowedFeatures,
-                                       const tint::wgsl::ValidationMode mode,
                                        const std::vector<tint::wgsl::Extension>& internalExtensions,
                                        OwnedCompilationMessages* outMessages) {
     tint::wgsl::reader::Options options;
-    options.mode = mode;
     options.allowed_features = allowedFeatures;
     options.allowed_features.extensions.insert(internalExtensions.begin(),
                                                internalExtensions.end());
@@ -1204,10 +1202,8 @@ MaybeError ValidateAndParseShaderModule(
     }
 
     tint::Program program;
-    auto validationMode = device->IsCompatibilityMode() ? tint::wgsl::ValidationMode::kCompat
-                                                        : tint::wgsl::ValidationMode::kFull;
     DAWN_TRY_ASSIGN(program, ParseWGSL(tintFile.get(), device->GetWGSLAllowedFeatures(),
-                                       validationMode, internalExtensions, outMessages));
+                                       internalExtensions, outMessages));
 
     parseResult->tintProgram = AcquireRef(new TintProgram(std::move(program), std::move(tintFile)));
 

src/dawn/tests/unittests/validation/CompatValidationTests.cpp

@@ -367,15 +367,44 @@ TEST_F(CompatValidationTest, CanNotUseShaderWithUnsupportedInterpolateTypeOrSamp
                 v.color = vec4f(1);
                 return v;
             }
+            @fragment fn fsWithoutBadInterpolationUsage() -> @location(0) vec4f {
+                return vec4f(1);
+            }
+            @fragment fn fsWithBadInterpolationUsage1(v: Vertex) -> @location(0) vec4f {
+                return vec4f(1);
+            }
+            @fragment fn fsWithBadInterpolationUsage2(v: Vertex) -> @location(0) vec4f {
+                return v.pos;
+            }
+            @fragment fn fsWithBadInterpolationUsage3(v: Vertex) -> @location(0) vec4f {
+                return v.color;
+            }
         )",
                                     interpolateParam);
-        if (strcmp(interpolateParam, "perspective") == 0) {
-            wgpu::ShaderModule moduleInterpolationLinear =
-                utils::CreateShaderModule(device, wgsl.c_str());
-        } else {
-            ASSERT_DEVICE_ERROR(wgpu::ShaderModule moduleInterpolationLinear =
-                                    utils::CreateShaderModule(device, wgsl.c_str()),
-                                testing::HasSubstr("in compatibility mode"));
+        wgpu::ShaderModule moduleInterpolationLinear =
+            utils::CreateShaderModule(device, wgsl.c_str());
+
+        static const char* entryPoints[] = {
+            "fsWithoutBadInterpolationUsage",
+            "fsWithBadInterpolationUsage1",
+            "fsWithBadInterpolationUsage2",
+            "fsWithBadInterpolationUsage3",
+        };
+        for (auto entryPoint : entryPoints) {
+            utils::ComboRenderPipelineDescriptor descriptor;
+            descriptor.vertex.module = moduleInterpolationLinear;
+            descriptor.cFragment.module = moduleInterpolationLinear;
+            descriptor.cFragment.entryPoint = entryPoint;
+
+            bool shouldSucceed =
+                entryPoint == entryPoints[0] || interpolateParam == interpolateParams[0];
+
+            if (shouldSucceed) {
+                device.CreateRenderPipeline(&descriptor);
+            } else {
+                ASSERT_DEVICE_ERROR(device.CreateRenderPipeline(&descriptor),
+                                    testing::HasSubstr("in compatibility mode"));
+            }
         }
     }
 }

src/tint/cmd/common/helper.cc

@@ -190,7 +190,6 @@ ProgramInfo LoadProgramInfo(const LoadProgramOptions& opts) {
 
                 tint::wgsl::reader::Options options;
                 options.allowed_features = tint::wgsl::AllowedFeatures::Everything();
-                options.mode = opts.mode;
 
                 auto file = std::make_unique<tint::Source::File>(
                     opts.filename, std::string(data.begin(), data.end()));

src/tint/cmd/common/helper.h

@@ -33,7 +33,6 @@
 #include <string>
 #include <vector>
 
-#include "src/tint/lang/wgsl/common/validation_mode.h"
 #include "src/tint/lang/wgsl/inspector/inspector.h"
 #include "src/tint/utils/diagnostic/source.h"
 
@@ -78,8 +77,6 @@ void PrintInspectorBindings(tint::inspector::Inspector& inspector);
 struct LoadProgramOptions {
     /// The file to be loaded
     std::string filename;
-    /// The WGSL validation mode to use.
-    tint::wgsl::ValidationMode mode = tint::wgsl::ValidationMode::kFull;
 #if TINT_BUILD_SPV_READER
     /// Spirv-reader options
     bool use_ir = false;

src/tint/cmd/tint/main.cc

@@ -49,7 +49,6 @@
 #include "src/tint/lang/wgsl/ast/transform/renamer.h"
 #include "src/tint/lang/wgsl/ast/transform/single_entry_point.h"
 #include "src/tint/lang/wgsl/ast/transform/substitute_override.h"
-#include "src/tint/lang/wgsl/common/validation_mode.h"
 #include "src/tint/lang/wgsl/helpers/flatten_bindings.h"
 #include "src/tint/utils/cli/cli.h"
 #include "src/tint/utils/command/command.h"
@@ -181,7 +180,6 @@ struct Options {
     bool parse_only = false;
     bool disable_workgroup_init = false;
     bool validate = false;
-    bool compatibility_mode = false;
     bool print_hash = false;
     bool dump_inspector_bindings = false;
     bool enable_robustness = false;
@@ -362,11 +360,6 @@ When specified, automatically enables MSL validation)",
         options.Add<BoolOption>("parse-only", "Stop after parsing the input", Default{false});
     TINT_DEFER(opts->parse_only = *parse_only.value);
 
-    auto& compatibility_mode = options.Add<BoolOption>(
-        "compatibility-mode", "Validate WGSL input using \"compatibility mode\"",
-        ShortName{"compat"}, Default{false});
-    TINT_DEFER(opts->compatibility_mode = *compatibility_mode.value);
-
 #if TINT_BUILD_SPV_READER
     auto& allow_nud =
         options.Add<BoolOption>("allow-non-uniform-derivatives",
@@ -1310,8 +1303,6 @@ int main(int argc, const char** argv) {
 
     tint::cmd::LoadProgramOptions opts;
     opts.filename = options.input_filename;
-    opts.mode = options.compatibility_mode ? tint::wgsl::ValidationMode::kCompat
-                                           : tint::wgsl::ValidationMode::kFull;
     opts.printer = options.printer.get();
 #if TINT_BUILD_SPV_READER
     opts.use_ir = options.use_ir_reader;

src/tint/lang/wgsl/common/BUILD.bazel

@@ -43,7 +43,6 @@ cc_library(
   ],
   hdrs = [
     "allowed_features.h",
-    "validation_mode.h",
   ],
   deps = [
     "//src/tint/lang/wgsl",

src/tint/lang/wgsl/common/BUILD.cmake

@@ -41,7 +41,6 @@
 tint_add_target(tint_lang_wgsl_common lib
   lang/wgsl/common/allowed_features.h
   lang/wgsl/common/common.cc
-  lang/wgsl/common/validation_mode.h
 )
 
 tint_target_add_dependencies(tint_lang_wgsl_common lib

src/tint/lang/wgsl/common/BUILD.gn

@@ -47,7 +47,6 @@ libtint_source_set("common") {
   sources = [
     "allowed_features.h",
     "common.cc",
-    "validation_mode.h",
   ]
   deps = [
     "${dawn_root}/src/utils:utils",