Make autogenerated validation produce device or builder errors

generator/templates/BackendProcTable.cpp

@@ -17,8 +17,6 @@
 
 #include "{{namespace}}/GeneratedCodeIncludes.h"
 
-#include <iostream>
-
 namespace backend {
 namespace {{namespace}} {
 
@@ -91,18 +89,30 @@ namespace {{namespace}} {
                     {%- endfor -%}
                 ) {
                     {% if type.is_builder and method.name.canonical_case() not in ("release", "reference") %}
-                        if (!self->CanBeUsed()) return false;
+                        if (!self->CanBeUsed()) {
+                            self->GetDevice()->HandleError("Builder cannot be used after GetResult");
+                            return false;
+                        }
                     {% else %}
                         (void) self;
                     {% endif %}
+                    bool error = false;
                     {% for arg in method.arguments %}
                         {% if arg.type.category == "enum" %}
-                            if (!CheckEnum{{as_cType(arg.type.name)}}({{as_varName(arg.name)}})) return false;
+                            if (!CheckEnum{{as_cType(arg.type.name)}}({{as_varName(arg.name)}})) error = true;;
                         {% elif arg.type.category == "bitmask" %}
-                            if (!CheckBitmask{{as_cType(arg.type.name)}}({{as_varName(arg.name)}})) return false;
+                            if (!CheckBitmask{{as_cType(arg.type.name)}}({{as_varName(arg.name)}})) error = true;
                         {% else %}
                             (void) {{as_varName(arg.name)}};
                         {% endif %}
+                        if (error) {
+                            {% if type.is_builder %}
+                                self->HandleError("Bad value in {{suffix}}");
+                            {% else %}
+                                self->GetDevice()->HandleError("Bad value in {{suffix}}");
+                            {% endif %}
+                            return false;
+                        }
                     {% endfor %}
                     return true;
                 }
@@ -133,25 +143,16 @@ namespace {{namespace}} {
                         }
                     {% endif %}
 
-                    //* If there is an error we forward it appropriately.
+                    //* HACK(cwallez@chromium.org): special casing GetResult so that the error callback
-                    if (!valid) {
+                    //* is called if needed. Without this, no call to HandleResult would happen, and the
-                        //* An error in a builder methods is always handled by the builder
+                    //* error callback would always get called with an Unknown status
-                        {% if type.is_builder %}
+                    {% if type.is_builder and method.name.canonical_case() == "get result" %}
-                            //* HACK(cwallez@chromium.org): special casing GetResult so that the error callback
+                        if (!valid) {
-                            //* is called if needed. Without this, no call to HandleResult would happen, and the
+                            {{as_backendType(method.return_type)}} fakeResult = nullptr;
-                            //* error callback would always get called with an Unknown status
+                            bool shouldBeFalse = self->HandleResult(fakeResult);
-                            {% if method.name.canonical_case() == "get result" %}
+                            assert(shouldBeFalse == false);
-                                {{as_backendType(method.return_type)}} fakeResult = nullptr;
+                        }
-                                bool shouldBeFalse = self->HandleResult(fakeResult);
+                    {% endif %}
-                                assert(shouldBeFalse == false);
-                            {% else %}
-                                self->HandleError("Error in {{suffix}}");
-                            {% endif %}
-                        {% else %}
-                            // TODO get the device or builder and give it the error?
-                            std::cout << "Error in {{suffix}}" << std::endl;
-                        {% endif %}
-                    }
 
                     {% if method.return_type.name.canonical_case() == "void" %}
                         if (!valid) return;

src/backend/common/Buffer.cpp

@@ -34,6 +34,10 @@ namespace backend {
         return new BufferViewBuilder(device, this);
     }
 
+    DeviceBase* BufferBase::GetDevice() {
+        return device;
+    }
+
     uint32_t BufferBase::GetSize() const {
         return size;
     }

src/backend/common/Buffer.h

@@ -36,6 +36,8 @@ namespace backend {
             bool HasFrozenUsage(nxt::BufferUsageBit usage) const;
             void TransitionUsageImpl(nxt::BufferUsageBit usage);
 
+            DeviceBase* GetDevice();
+
             // NXT API
             BufferViewBuilder* CreateBufferViewBuilder();
             void SetSubData(uint32_t start, uint32_t count, const uint32_t* data);

src/backend/common/Builder.cpp

@@ -22,6 +22,10 @@ namespace backend {
         return !consumed && !gotStatus;
     }
 
+    DeviceBase* BuilderBase::GetDevice() {
+        return device;
+    }
+
     void BuilderBase::HandleError(const char* message) {
         SetStatus(nxt::BuilderErrorStatus::Error, message);
     }

src/backend/common/Builder.h

@@ -42,6 +42,7 @@ namespace backend {
             // Used by the auto-generated validation to prevent usage of the builder
             // after GetResult or an error.
             bool CanBeUsed() const;
+            DeviceBase* GetDevice();
 
             // Set the status of the builder to an error.
             void HandleError(const char* message);

src/backend/common/Device.cpp

@@ -61,6 +61,10 @@ namespace backend {
         this->errorUserdata = userdata;
     }
 
+    DeviceBase* DeviceBase::GetDevice() {
+        return this;
+    }
+
     BindGroupLayoutBase* DeviceBase::GetOrCreateBindGroupLayout(const BindGroupLayoutBase* blueprint, BindGroupLayoutBuilder* builder) {
         // The blueprint is only used to search in the cache and is not modified. However cached
         // objects can be modified, and unordered_set cannot search for a const pointer in a non

src/backend/common/Device.h

@@ -32,6 +32,9 @@ namespace backend {
             void HandleError(const char* message);
             void SetErrorCallback(nxt::DeviceErrorCallback callback, nxt::CallbackUserdata userdata);
 
+            // Used by autogenerated code, returns itself
+            DeviceBase* GetDevice();
+
             virtual BindGroupBase* CreateBindGroup(BindGroupBuilder* builder) = 0;
             virtual BindGroupLayoutBase* CreateBindGroupLayout(BindGroupLayoutBuilder* builder) = 0;
             virtual BufferBase* CreateBuffer(BufferBuilder* builder) = 0;

src/backend/common/Queue.cpp

@@ -21,6 +21,13 @@ namespace backend {
 
     // QueueBase
 
+    QueueBase::QueueBase(QueueBuilder* builder) : device(builder->device) {
+    }
+
+    DeviceBase* QueueBase::GetDevice() {
+        return device;
+    }
+
     bool QueueBase::ValidateSubmitCommand(CommandBufferBase* command) {
         return command->ValidateResourceUsagesImmediate();
     }

src/backend/common/Queue.h

@@ -24,10 +24,11 @@
 namespace backend {
 
     class QueueBase : public RefCounted {
-        private:
-            bool ValidateSubmitCommand(CommandBufferBase* command);
-
         public:
+            QueueBase(QueueBuilder* builder);
+
+            DeviceBase* GetDevice();
+
             template<typename T>
             bool ValidateSubmit(uint32_t numCommands, T* const * commands) {
                 static_assert(std::is_base_of<CommandBufferBase, T>::value, "invalid command buffer type");
@@ -39,6 +40,11 @@ namespace backend {
                 }
                 return true;
             }
+
+        private:
+            bool ValidateSubmitCommand(CommandBufferBase* command);
+
+            DeviceBase* device;
     };
 
     class QueueBuilder : public Builder<QueueBase> {
@@ -46,6 +52,7 @@ namespace backend {
             QueueBuilder(DeviceBase* device);
 
         private:
+            friend class QueueBase;
             QueueBase* GetResultImpl() override;
     };
 

src/backend/common/Texture.cpp

@@ -33,6 +33,10 @@ namespace backend {
         allowedUsage(builder->allowedUsage), currentUsage(builder->currentUsage) {
     }
 
+    DeviceBase* TextureBase::GetDevice() {
+        return device;
+    }
+
     nxt::TextureDimension TextureBase::GetDimension() const {
         return dimension;
     }

src/backend/common/Texture.h

@@ -43,6 +43,8 @@ namespace backend {
             bool IsTransitionPossible(nxt::TextureUsageBit usage) const;
             void TransitionUsageImpl(nxt::TextureUsageBit usage);
 
+            DeviceBase* GetDevice();
+
             // NXT API
             TextureViewBuilder* CreateTextureViewBuilder();
             void TransitionUsage(nxt::TextureUsageBit usage);

src/backend/metal/MetalBackend.mm

@@ -803,7 +803,7 @@ namespace metal {
     // Queue
 
     Queue::Queue(Device* device, QueueBuilder* builder)
-        : device(device) {
+        : QueueBase(builder), device(device) {
         commandQueue = [device->GetMTLDevice() newCommandQueue];
     }
 

src/backend/opengl/OpenGLBackend.cpp

@@ -167,7 +167,8 @@ namespace opengl {
 
     // Queue
 
-    Queue::Queue(Device* device, QueueBuilder* builder) : device(device) {
+    Queue::Queue(Device* device, QueueBuilder* builder)
+        : QueueBase(builder), device(device) {
     }
 
     void Queue::Submit(uint32_t numCommands, CommandBuffer* const * commands) {