Split `BindingPoint` out of MultiplanarExternalTexture.

This CL moves the `using` statement for `BindingPoint` out to its own
file in `src/tint/writer/binding_point.h`. Usages of
`tint::transform::BindingPoint` have been updated to
`tint::writer::BindingPoint`.

Bug: tint:1855
Change-Id: Ife7d92807e0a65b93f21158d73a60d5068fc5da9
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/123000
Reviewed-by: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Dan Sinclair <dsinclair@chromium.org>

include/tint/tint.h

@@ -37,6 +37,7 @@
 #include "src/tint/transform/vertex_pulling.h"
 #include "src/tint/type/manager.h"
 #include "src/tint/writer/array_length_from_uniform_options.h"
+#include "src/tint/writer/binding_point.h"
 #include "src/tint/writer/flatten_bindings.h"
 #include "src/tint/writer/writer.h"
 

src/dawn/native/StreamImplTint.cpp

@@ -44,8 +44,8 @@ void stream::Stream<tint::Program>::Write(stream::Sink* sink, const tint::Progra
 
 // static
 template <>
-void stream::Stream<tint::sem::BindingPoint>::Write(stream::Sink* sink,
+void stream::Stream<tint::writer::BindingPoint>::Write(stream::Sink* sink,
-                                                    const tint::sem::BindingPoint& point) {
+                                                       const tint::writer::BindingPoint& point) {
     StreamInTintObject(point, sink);
 }
 

src/dawn/native/d3d12/ShaderModuleD3D12.cpp

@@ -383,7 +383,7 @@ ResultOrError<std::string> TranslateToHLSL(
     options.disable_workgroup_init = r.disableWorkgroupInit;
     if (r.usesNumWorkgroups) {
         options.root_constant_binding_point =
-            tint::sem::BindingPoint{r.numWorkgroupsRegisterSpace, r.numWorkgroupsShaderRegister};
+            tint::writer::BindingPoint{r.numWorkgroupsRegisterSpace, r.numWorkgroupsShaderRegister};
     }
     // TODO(dawn:549): HLSL generation outputs the indices into the
     // array_length_from_uniform buffer that were actually used. When the blob cache can
@@ -523,8 +523,8 @@ ResultOrError<CompiledShader> ShaderModule::Compile(
         }
     }
 
-    using tint::transform::BindingPoint;
     using tint::transform::BindingRemapper;
+    using tint::writer::BindingPoint;
 
     BindingRemapper::BindingPoints remappedBindingPoints;
     BindingRemapper::AccessControls remappedAccessControls;

src/dawn/native/metal/ShaderModuleMTL.mm

@@ -111,7 +111,7 @@ ResultOrError<CacheResult<MslCompilation>> TranslateToMSL(
 
     // Remap BindingNumber to BindingIndex in WGSL shader
     using BindingRemapper = tint::transform::BindingRemapper;
-    using BindingPoint = tint::transform::BindingPoint;
+    using BindingPoint = tint::writer::BindingPoint;
     BindingRemapper::BindingPoints bindingPoints;
 
     for (BindGroupIndex group : IterateBitSet(layout->GetBindGroupLayoutsMask())) {

src/dawn/native/opengl/ShaderModuleGL.cpp

@@ -55,7 +55,7 @@ tint::writer::glsl::Version::Standard ToTintGLStandard(opengl::OpenGLVersion::St
     UNREACHABLE();
 }
 
-using BindingMap = std::unordered_map<tint::sem::BindingPoint, tint::sem::BindingPoint>;
+using BindingMap = std::unordered_map<tint::writer::BindingPoint, tint::writer::BindingPoint>;
 
 #define GLSL_COMPILATION_REQUEST_MEMBERS(X)                                                 \
     X(const tint::Program*, inputProgram)                                                   \
@@ -148,14 +148,14 @@ ResultOrError<GLuint> ShaderModule::CompileShader(const OpenGLFunctions& gl,
 
     const OpenGLVersion& version = ToBackend(GetDevice())->GetGL().GetVersion();
 
-    using tint::transform::BindingPoint;
+    using tint::writer::BindingPoint;
     // Since (non-Vulkan) GLSL does not support descriptor sets, generate a
     // mapping from the original group/binding pair to a binding-only
     // value. This mapping will be used by Tint to remap all global
     // variables to the 1D space.
     const BindingInfoArray& moduleBindingInfo =
         GetEntryPoint(programmableStage.entryPoint).bindings;
-    std::unordered_map<tint::sem::BindingPoint, tint::sem::BindingPoint> glBindings;
+    std::unordered_map<tint::writer::BindingPoint, tint::writer::BindingPoint> glBindings;
     for (BindGroupIndex group : IterateBitSet(layout->GetBindGroupLayoutsMask())) {
         const BindGroupLayoutBase* bgl = layout->GetBindGroupLayout(group);
         const auto& groupBindingInfo = moduleBindingInfo[group];

src/dawn/native/vulkan/ShaderModuleVk.cpp

@@ -205,7 +205,7 @@ ResultOrError<ShaderModule::ModuleAndSpirv> ShaderModule::GetHandleAndSpirv(
 
     // Remap BindingNumber to BindingIndex in WGSL shader
     using BindingRemapper = tint::transform::BindingRemapper;
-    using BindingPoint = tint::transform::BindingPoint;
+    using BindingPoint = tint::writer::BindingPoint;
     BindingRemapper::BindingPoints bindingPoints;
 
     const BindingInfoArray& moduleBindingInfo =

src/dawn/tests/unittests/native/StreamTests.cpp

@@ -253,9 +253,9 @@ TEST(SerializeTests, StdUnorderedMap) {
     EXPECT_CACHE_KEY_EQ(m, expected);
 }
 
-// Test that ByteVectorSink serializes tint::sem::BindingPoint as expected.
+// Test that ByteVectorSink serializes tint::writer::BindingPoint as expected.
 TEST(SerializeTests, TintSemBindingPoint) {
-    tint::sem::BindingPoint bp{3, 6};
+    tint::writer::BindingPoint bp{3, 6};
 
     ByteVectorSink expected;
     StreamIn(&expected, uint32_t(3), uint32_t(6));
@@ -263,11 +263,12 @@ TEST(SerializeTests, TintSemBindingPoint) {
     EXPECT_CACHE_KEY_EQ(bp, expected);
 }
 
-// Test that ByteVectorSink serializes tint::transform::BindingPoints as expected.
+// Test that ByteVectorSink serializes tint::transform::MultiplanarExternalTexture::BindingPoints
+// as expected.
 TEST(SerializeTests, TintTransformBindingPoints) {
     tint::transform::MultiplanarExternalTexture::BindingPoints points{
-        tint::sem::BindingPoint{1, 4},
+        tint::writer::BindingPoint{1, 4},
-        tint::sem::BindingPoint{3, 7},
+        tint::writer::BindingPoint{3, 7},
     };
 
     ByteVectorSink expected;

src/tint/BUILD.gn

@@ -895,6 +895,7 @@ libtint_source_set("libtint_writer_src") {
     "writer/append_vector.h",
     "writer/array_length_from_uniform_options.cc",
     "writer/array_length_from_uniform_options.h",
+    "writer/binding_point.h",
     "writer/check_supported_extensions.cc",
     "writer/check_supported_extensions.h",
     "writer/flatten_bindings.cc",

src/tint/CMakeLists.txt

@@ -539,6 +539,7 @@ list(APPEND TINT_LIB_SRCS
   writer/append_vector.h
   writer/array_length_from_uniform_options.cc
   writer/array_length_from_uniform_options.h
+  writer/binding_point.h
   writer/check_supported_extensions.cc
   writer/check_supported_extensions.h
   writer/flatten_bindings.cc

src/tint/cmd/main.cc

@@ -1016,7 +1016,7 @@ int main(int argc, const char** argv) {
              std::unordered_set<uint32_t> group0_bindings_in_use;
              auto allocate_binding = [&] {
                  for (uint32_t idx = 0;; idx++) {
-                     auto binding = tint::transform::BindingPoint{0u, idx};
+                     auto binding = tint::writer::BindingPoint{0u, idx};
                      if (group0_bindings_in_use.emplace(idx).second) {
                          return binding;
                      }
@@ -1034,7 +1034,7 @@ int main(int argc, const char** argv) {
              MET::BindingsMap met_bindings;
              for (auto ep : inspector.GetEntryPoints()) {
                  for (auto ext_tex : inspector.GetExternalTextureResourceBindings(ep.name)) {
-                     auto binding = tint::transform::BindingPoint{
+                     auto binding = tint::writer::BindingPoint{
                          ext_tex.bind_group,
                          ext_tex.binding,
                      };

src/tint/transform/multiplanar_external_texture.cc

@@ -121,7 +121,7 @@ struct MultiplanarExternalTexture::State {
             // The binding points for the newly introduced bindings must have been provided to this
             // transform. We fetch the new binding points by providing the original texture_external
             // binding points into the passed map.
-            BindingPoint bp = sem_var->BindingPoint();
+            sem::BindingPoint bp = sem_var->BindingPoint();
 
             BindingsMap::const_iterator it = new_binding_points->bindings_map.find(bp);
             if (it == new_binding_points->bindings_map.end()) {
@@ -494,6 +494,7 @@ struct MultiplanarExternalTexture::State {
 
 MultiplanarExternalTexture::NewBindingPoints::NewBindingPoints(BindingsMap inputBindingsMap)
     : bindings_map(std::move(inputBindingsMap)) {}
+
 MultiplanarExternalTexture::NewBindingPoints::~NewBindingPoints() = default;
 
 MultiplanarExternalTexture::MultiplanarExternalTexture() = default;

src/tint/transform/multiplanar_external_texture.h

@@ -25,9 +25,6 @@
 
 namespace tint::transform {
 
-/// BindingPoint is an alias to sem::BindingPoint
-using BindingPoint = sem::BindingPoint;
-
 /// Within the MultiplanarExternalTexture transform, each instance of a
 /// texture_external binding is unpacked into two texture_2d<f32> bindings
 /// representing two possible planes of a texture and a uniform buffer binding
@@ -46,10 +43,10 @@ class MultiplanarExternalTexture final : public Castable<MultiplanarExternalText
     struct BindingPoints {
         /// The desired binding location of the texture_2d representing plane #1 when
         /// a texture_external binding is expanded.
-        BindingPoint plane_1;
+        sem::BindingPoint plane_1;
         /// The desired binding location of the ExternalTextureParams uniform when a
         /// texture_external binding is expanded.
-        BindingPoint params;
+        sem::BindingPoint params;
 
         /// Reflect the fields of this class so that it can be used by tint::ForeachField()
         TINT_REFLECT(plane_1, params);
@@ -58,7 +55,7 @@ class MultiplanarExternalTexture final : public Castable<MultiplanarExternalText
     /// BindingsMap is a map where the key is the binding location of a
     /// texture_external and the value is a struct containing the desired
     /// locations for new bindings expanded from the texture_external instance.
-    using BindingsMap = std::unordered_map<BindingPoint, BindingPoints>;
+    using BindingsMap = std::unordered_map<sem::BindingPoint, BindingPoints>;
 
     /// NewBindingPoints is consumed by the MultiplanarExternalTexture transform.
     /// Data holds information about location of each texture_external binding and

src/tint/writer/array_length_from_uniform_options.h

@@ -17,7 +17,7 @@
 
 #include <unordered_map>
 
-#include "src/tint/sem/binding_point.h"
+#include "src/tint/writer/binding_point.h"
 
 namespace tint::writer {
 
@@ -38,10 +38,10 @@ struct ArrayLengthFromUniformOptions {
 
     /// The binding point to use to generate a uniform buffer from which to read
     /// buffer sizes.
-    sem::BindingPoint ubo_binding;
+    BindingPoint ubo_binding;
     /// The mapping from storage buffer binding points to the index into the
     /// uniform buffer where the length of the buffer is stored.
-    std::unordered_map<sem::BindingPoint, uint32_t> bindpoint_to_size_index;
+    std::unordered_map<BindingPoint, uint32_t> bindpoint_to_size_index;
 
     /// Reflect the fields of this class so that it can be used by tint::ForeachField()
     TINT_REFLECT(ubo_binding, bindpoint_to_size_index);

src/tint/writer/binding_point.h

@@ -0,0 +1,27 @@
+// Copyright 2023 The Tint Authors.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#ifndef SRC_TINT_WRITER_BINDING_POINT_H_
+#define SRC_TINT_WRITER_BINDING_POINT_H_
+
+#include "src/tint/sem/binding_point.h"
+
+namespace tint::writer {
+
+/// BindingPoint is an alias to sem::BindingPoint
+using BindingPoint = sem::BindingPoint;
+
+}  // namespace tint::writer
+
+#endif  // SRC_TINT_WRITER_BINDING_POINT_H_

src/tint/writer/flatten_bindings.cc

@@ -19,11 +19,12 @@
 #include "src/tint/inspector/inspector.h"
 #include "src/tint/transform/binding_remapper.h"
 #include "src/tint/transform/manager.h"
+#include "src/tint/writer/binding_point.h"
 
 namespace tint::writer {
+
 std::optional<Program> FlattenBindings(const Program* program) {
     // TODO(crbug.com/tint/1101): Make this more robust for multiple entry points.
-    using BindingPoint = tint::transform::BindingPoint;
     tint::transform::BindingRemapper::BindingPoints binding_points;
     uint32_t next_buffer_idx = 0;
     uint32_t next_sampler_idx = 0;
@@ -76,4 +77,5 @@ std::optional<Program> FlattenBindings(const Program* program) {
 
     return {};
 }
+
 }  // namespace tint::writer

src/tint/writer/generate_external_texture_bindings_test.cc

@@ -16,6 +16,7 @@
 
 #include "gtest/gtest.h"
 #include "src/tint/program_builder.h"
+#include "src/tint/writer/binding_point.h"
 #include "src/tint/writer/generate_external_texture_bindings.h"
 
 namespace tint::writer {
@@ -45,7 +46,7 @@ TEST_F(GenerateExternalTextureBindingsTest, One) {
     auto bindings = GenerateExternalTextureBindings(&program);
     ASSERT_EQ(bindings.size(), 1u);
 
-    auto to = bindings[transform::BindingPoint{0, 0}];
+    auto to = bindings[writer::BindingPoint{0, 0}];
     ASSERT_EQ(to.plane_1.group, 0u);
     ASSERT_EQ(to.params.group, 0u);
     ASSERT_EQ(to.plane_1.binding, 1u);
@@ -62,13 +63,13 @@ TEST_F(GenerateExternalTextureBindingsTest, Two_SameGroup) {
     auto bindings = GenerateExternalTextureBindings(&program);
     ASSERT_EQ(bindings.size(), 2u);
 
-    auto to0 = bindings[transform::BindingPoint{0, 0}];
+    auto to0 = bindings[writer::BindingPoint{0, 0}];
     ASSERT_EQ(to0.plane_1.group, 0u);
     ASSERT_EQ(to0.params.group, 0u);
     ASSERT_EQ(to0.plane_1.binding, 2u);
     ASSERT_EQ(to0.params.binding, 3u);
 
-    auto to1 = bindings[transform::BindingPoint{0, 1}];
+    auto to1 = bindings[writer::BindingPoint{0, 1}];
     ASSERT_EQ(to1.plane_1.group, 0u);
     ASSERT_EQ(to1.params.group, 0u);
     ASSERT_EQ(to1.plane_1.binding, 4u);
@@ -85,13 +86,13 @@ TEST_F(GenerateExternalTextureBindingsTest, Two_DifferentGroup) {
     auto bindings = GenerateExternalTextureBindings(&program);
     ASSERT_EQ(bindings.size(), 2u);
 
-    auto to0 = bindings[transform::BindingPoint{0, 0}];
+    auto to0 = bindings[writer::BindingPoint{0, 0}];
     ASSERT_EQ(to0.plane_1.group, 0u);
     ASSERT_EQ(to0.params.group, 0u);
     ASSERT_EQ(to0.plane_1.binding, 1u);
     ASSERT_EQ(to0.params.binding, 2u);
 
-    auto to1 = bindings[transform::BindingPoint{1, 0}];
+    auto to1 = bindings[writer::BindingPoint{1, 0}];
     ASSERT_EQ(to1.plane_1.group, 1u);
     ASSERT_EQ(to1.params.group, 1u);
     ASSERT_EQ(to1.plane_1.binding, 1u);
@@ -111,13 +112,13 @@ TEST_F(GenerateExternalTextureBindingsTest, Two_WithOtherBindingsInSameGroup) {
     auto bindings = GenerateExternalTextureBindings(&program);
     ASSERT_EQ(bindings.size(), 2u);
 
-    auto to0 = bindings[transform::BindingPoint{0, 1}];
+    auto to0 = bindings[writer::BindingPoint{0, 1}];
     ASSERT_EQ(to0.plane_1.group, 0u);
     ASSERT_EQ(to0.params.group, 0u);
     ASSERT_EQ(to0.plane_1.binding, 5u);
     ASSERT_EQ(to0.params.binding, 6u);
 
-    auto to1 = bindings[transform::BindingPoint{0, 3}];
+    auto to1 = bindings[writer::BindingPoint{0, 3}];
     ASSERT_EQ(to1.plane_1.group, 0u);
     ASSERT_EQ(to1.params.group, 0u);
     ASSERT_EQ(to1.plane_1.binding, 7u);