Add validation for some null arguments and dangling render passes

And add tests for both

src/backend/common/CommandBuffer.cpp

@@ -186,6 +186,15 @@ namespace backend {
                         BeginRenderPassCmd* cmd = iterator.NextCommand<BeginRenderPassCmd>();
                         auto* renderPass = cmd->renderPass.Get();
                         auto* framebuffer = cmd->framebuffer.Get();
+                        // TODO(kainino@chromium.org): null checks should not be necessary
+                        if (renderPass == nullptr) {
+                            HandleError("Render pass is invalid");
+                            return false;
+                        }
+                        if (framebuffer == nullptr) {
+                            HandleError("Framebuffer is invalid");
+                            return false;
+                        }
                         if (!state->BeginRenderPass(renderPass, framebuffer)) {
                             return false;
                         }
@@ -353,6 +362,10 @@ namespace backend {
             }
         }
 
+        if (!state->ValidateEndCommandBuffer()) {
+            return false;
+        }
+
         return true;
     }
 

src/backend/common/CommandBufferStateTracker.cpp

@@ -113,6 +113,14 @@ namespace backend {
         return RevalidateCanDraw();
     }
 
+    bool CommandBufferStateTracker::ValidateEndCommandBuffer() const {
+        if (currentRenderPass != nullptr) {
+            builder->HandleError("Can't end command buffer with an active render pass");
+            return false;
+        }
+        return true;
+    }
+
     bool CommandBufferStateTracker::BeginSubpass() {
         if (currentRenderPass == nullptr) {
             builder->HandleError("Can't begin a subpass without an active render pass");

src/backend/common/CommandBufferStateTracker.h

@@ -34,6 +34,7 @@ namespace backend {
             bool ValidateCanDispatch();
             bool ValidateCanDrawArrays();
             bool ValidateCanDrawElements();
+            bool ValidateEndCommandBuffer() const;
 
             // State-modifying methods
             bool BeginSubpass();

src/backend/common/Framebuffer.cpp

@@ -85,6 +85,11 @@ namespace backend {
             HandleError("Framebuffer render pass property set multiple times");
             return;
         }
+        // TODO(kainino@chromium.org): null checks should not be necessary
+        if (renderPass == nullptr) {
+            HandleError("Render pass invalid");
+            return;
+        }
 
         this->renderPass = renderPass;
         this->textureViews.resize(renderPass->GetAttachmentCount());

src/tests/CMakeLists.txt

@@ -26,8 +26,11 @@ add_executable(nxt_unittests
     ${UNITTESTS_DIR}/ToBackendTests.cpp
     ${UNITTESTS_DIR}/WireTests.cpp
     ${VALIDATION_TESTS_DIR}/BufferValidationTests.cpp
+    ${VALIDATION_TESTS_DIR}/CommandBufferValidationTests.cpp
     ${VALIDATION_TESTS_DIR}/ComputeValidationTests.cpp
     ${VALIDATION_TESTS_DIR}/DepthStencilStateValidationTests.cpp
+    ${VALIDATION_TESTS_DIR}/FramebufferValidationTests.cpp
+    ${VALIDATION_TESTS_DIR}/RenderPassValidationTests.cpp
     ${VALIDATION_TESTS_DIR}/ValidationTest.cpp
     ${VALIDATION_TESTS_DIR}/ValidationTest.h
     ${TESTS_DIR}/UnittestsMain.cpp

src/tests/unittests/validation/CommandBufferValidationTests.cpp

@@ -0,0 +1,65 @@
+// Copyright 2017 The NXT 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.
+
+#include "ValidationTest.h"
+
+class CommandBufferValidationTest : public ValidationTest {
+};
+
+// Test for an empty command buffer
+TEST_F(CommandBufferValidationTest, Empty) {
+    AssertWillBeSuccess(device.CreateCommandBufferBuilder())
+        .GetResult();
+}
+
+// Tests for null arguments to the command buffer builder
+TEST_F(CommandBufferValidationTest, NullArguments) {
+    auto renderpass = AssertWillBeSuccess(device.CreateRenderPassBuilder())
+        .SetSubpassCount(1)
+        .SetAttachmentCount(0)
+        .GetResult();
+    auto framebuffer = AssertWillBeSuccess(device.CreateFramebufferBuilder())
+        .SetRenderPass(renderpass)
+        .SetDimensions(100, 100)
+        .GetResult();
+
+    AssertWillBeError(device.CreateCommandBufferBuilder())
+        .BeginRenderPass(renderpass, nullptr)
+        .EndRenderPass()
+        .GetResult();
+    AssertWillBeError(device.CreateCommandBufferBuilder())
+        .BeginRenderPass(nullptr, framebuffer)
+        .EndRenderPass()
+        .GetResult();
+}
+
+// Tests for basic render pass usage
+TEST_F(CommandBufferValidationTest, RenderPass) {
+    auto renderpass = AssertWillBeSuccess(device.CreateRenderPassBuilder())
+        .SetAttachmentCount(0)
+        .SetSubpassCount(1)
+        .GetResult();
+    auto framebuffer = AssertWillBeSuccess(device.CreateFramebufferBuilder())
+        .SetRenderPass(renderpass)
+        .SetDimensions(100, 100)
+        .GetResult();
+
+    AssertWillBeSuccess(device.CreateCommandBufferBuilder())
+        .BeginRenderPass(renderpass, framebuffer)
+        .EndRenderPass()
+        .GetResult();
+    AssertWillBeError(device.CreateCommandBufferBuilder())
+        .BeginRenderPass(renderpass, framebuffer)
+        .GetResult();
+}

src/tests/unittests/validation/FramebufferValidationTests.cpp

@@ -0,0 +1,66 @@
+// Copyright 2017 The NXT 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.
+
+#include "ValidationTest.h"
+
+class FramebufferValidationTest : public ValidationTest {
+};
+
+// Test for an empty framebuffer builder
+TEST_F(FramebufferValidationTest, Empty) {
+    auto framebuffer = AssertWillBeError(device.CreateFramebufferBuilder())
+        .GetResult();
+}
+
+// Tests for null arguments to a framebuffer builder
+TEST_F(FramebufferValidationTest, NullArguments) {
+    AssertWillBeError(device.CreateFramebufferBuilder())
+        .SetRenderPass(nullptr)
+        .GetResult();
+}
+
+// Tests for passing error-valued arguments to a framebuffer builder
+TEST_F(FramebufferValidationTest, ErrorValues) {
+    auto renderpass = AssertWillBeError(device.CreateRenderPassBuilder())
+        .GetResult();
+    AssertWillBeError(device.CreateFramebufferBuilder())
+        .SetRenderPass(renderpass)
+        .GetResult();
+}
+
+// Tests for basic framebuffer construction
+TEST_F(FramebufferValidationTest, Basic) {
+    auto renderpass = AssertWillBeSuccess(device.CreateRenderPassBuilder())
+        .SetSubpassCount(1)
+        .SetAttachmentCount(0)
+        .GetResult();
+    auto framebuffer = AssertWillBeSuccess(device.CreateFramebufferBuilder())
+        .SetRenderPass(renderpass)
+        .SetDimensions(100, 100)
+        .GetResult();
+}
+
+// Tests for framebuffer construction with an (empty) attachment
+TEST_F(FramebufferValidationTest, BasicWithEmptyAttachment) {
+    auto renderpass = AssertWillBeSuccess(device.CreateRenderPassBuilder())
+        .SetAttachmentCount(1)
+        .AttachmentSetFormat(0, nxt::TextureFormat::R8G8B8A8Unorm)
+        .SetSubpassCount(1)
+        .SubpassSetColorAttachment(0, 0, 0)
+        .GetResult();
+    auto framebuffer = AssertWillBeSuccess(device.CreateFramebufferBuilder())
+        .SetRenderPass(renderpass)
+        .SetDimensions(100, 100)
+        .GetResult();
+}

src/tests/unittests/validation/RenderPassValidationTests.cpp

@@ -0,0 +1,42 @@
+// Copyright 2017 The NXT 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.
+
+#include "ValidationTest.h"
+
+class RenderPassValidationTest : public ValidationTest {
+};
+
+// Test for an empty render pass builder
+TEST_F(RenderPassValidationTest, Empty) {
+    AssertWillBeError(device.CreateRenderPassBuilder())
+        .GetResult();
+}
+
+// Test for a render pass with one subpass and no attachments
+TEST_F(RenderPassValidationTest, OneSubpass) {
+    AssertWillBeSuccess(device.CreateRenderPassBuilder())
+        .SetSubpassCount(1)
+        .SetAttachmentCount(0)
+        .GetResult();
+}
+
+// Test for a render pass with one subpass and one attachment
+TEST_F(RenderPassValidationTest, OneSubpassOneAttachment) {
+    AssertWillBeSuccess(device.CreateRenderPassBuilder())
+        .SetSubpassCount(1)
+        .SetAttachmentCount(1)
+        .AttachmentSetFormat(0, nxt::TextureFormat::R8G8B8A8Unorm)
+        .SubpassSetColorAttachment(0, 0, 0)
+        .GetResult();
+}