diff --git a/src/backend/CMakeLists.txt b/src/backend/CMakeLists.txt index b09a05aa4d..837ac6a762 100644 --- a/src/backend/CMakeLists.txt +++ b/src/backend/CMakeLists.txt @@ -48,6 +48,8 @@ if (NXT_ENABLE_OPENGL) ${OPENGL_DIR}/ComputePipelineGL.h ${OPENGL_DIR}/DepthStencilStateGL.cpp ${OPENGL_DIR}/DepthStencilStateGL.h + ${OPENGL_DIR}/InputStateGL.cpp + ${OPENGL_DIR}/InputStateGL.h ${OPENGL_DIR}/OpenGLBackend.cpp ${OPENGL_DIR}/OpenGLBackend.h ${OPENGL_DIR}/PersistentPipelineStateGL.cpp diff --git a/src/backend/opengl/BufferGL.h b/src/backend/opengl/BufferGL.h index f05dddc0fc..bcbc2f7f11 100644 --- a/src/backend/opengl/BufferGL.h +++ b/src/backend/opengl/BufferGL.h @@ -16,7 +16,6 @@ #define BACKEND_OPENGL_BUFFERGL_H_ #include "backend/Buffer.h" -#include "common/SerialQueue.h" #include "glad/glad.h" diff --git a/src/backend/opengl/CommandBufferGL.cpp b/src/backend/opengl/CommandBufferGL.cpp index aff11a1b5a..8379ecaa2c 100644 --- a/src/backend/opengl/CommandBufferGL.cpp +++ b/src/backend/opengl/CommandBufferGL.cpp @@ -17,6 +17,7 @@ #include "backend/Commands.h" #include "backend/opengl/BufferGL.h" #include "backend/opengl/ComputePipelineGL.h" +#include "backend/opengl/InputStateGL.h" #include "backend/opengl/OpenGLBackend.h" #include "backend/opengl/PersistentPipelineStateGL.h" #include "backend/opengl/PipelineLayoutGL.h" @@ -31,6 +32,29 @@ namespace opengl { namespace { + GLenum IndexFormatType(nxt::IndexFormat format) { + switch (format) { + case nxt::IndexFormat::Uint16: + return GL_UNSIGNED_SHORT; + case nxt::IndexFormat::Uint32: + return GL_UNSIGNED_INT; + default: + UNREACHABLE(); + } + } + + GLenum VertexFormatType(nxt::VertexFormat format) { + switch (format) { + case nxt::VertexFormat::FloatR32G32B32A32: + case nxt::VertexFormat::FloatR32G32B32: + case nxt::VertexFormat::FloatR32G32: + case nxt::VertexFormat::FloatR32: + return GL_FLOAT; + default: + UNREACHABLE(); + } + } + // Push constants are implemented using OpenGL uniforms, however they aren't part of the global // OpenGL state but are part of the program state instead. This means that we have to reapply // push constants on pipeline change. @@ -44,7 +68,7 @@ namespace opengl { void OnBeginPass() { for (auto stage : IterateStages(kAllStages)) { values[stage].fill(0); - // No need to set dirty bits are a pipeline will be set before the next operation + // No need to set dirty bits as a pipeline will be set before the next operation // using push constants. } } @@ -85,9 +109,87 @@ namespace opengl { break; } } + + dirtyBits[stage].reset(); } } + }; + // Vertex buffers and index buffers are implemented as part of an OpenGL VAO that corresponds to an + // InputState. On the contrary in NXT they are part of the global state. This means that we have to + // re-apply these buffers on an InputState change. + struct InputBufferTracker { + bool indexBufferDirty = false; + Buffer* indexBuffer = nullptr; + + std::bitset dirtyVertexBuffers; + std::array vertexBuffers; + std::array vertexBufferOffsets; + + InputState* lastInputState = nullptr; + + void OnBeginPass() { + // We don't know what happened between this pass and the last one, just reset the + // input state so everything gets reapplied. + lastInputState = nullptr; + } + + void OnSetIndexBuffer(BufferBase* buffer) { + indexBufferDirty = true; + indexBuffer = ToBackend(buffer); + } + + void OnSetVertexBuffers(uint32_t startSlot, uint32_t count, Ref* buffers, uint32_t* offsets) { + for (uint32_t i = 0; i < count; ++i) { + uint32_t slot = startSlot + i; + vertexBuffers[slot] = ToBackend(buffers[i].Get()); + vertexBufferOffsets[slot] = offsets[i]; + } + + // Use 64 bit masks and make sure there are no shift UB + static_assert(kMaxVertexInputs <= 8 * sizeof(unsigned long long) - 1, ""); + dirtyVertexBuffers |= ((1ull << count) - 1ull) << startSlot; + } + + void OnSetPipeline(RenderPipelineBase* pipeline) { + InputStateBase* inputState = pipeline->GetInputState(); + if (lastInputState == inputState) { + return; + } + + indexBufferDirty = true; + dirtyVertexBuffers |= inputState->GetInputsSetMask(); + + lastInputState = ToBackend(inputState); + } + + void Apply() { + if (indexBufferDirty && indexBuffer != nullptr) { + glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, indexBuffer->GetHandle()); + indexBufferDirty = false; + } + + for (uint32_t slot : IterateBitSet(dirtyVertexBuffers & lastInputState->GetInputsSetMask())) { + for (uint32_t location : IterateBitSet(lastInputState->GetAttributesUsingInput(slot))) { + auto attribute = lastInputState->GetAttribute(location); + + GLuint buffer = vertexBuffers[slot]->GetHandle(); + uint32_t offset = vertexBufferOffsets[slot]; + + auto input = lastInputState->GetInput(slot); + auto components = VertexFormatNumComponents(attribute.format); + auto formatType = VertexFormatType(attribute.format); + + glBindBuffer(GL_ARRAY_BUFFER, buffer); + glVertexAttribPointer( + location, components, formatType, GL_FALSE, + input.stride, + reinterpret_cast(static_cast(offset + attribute.offset))); + } + } + + dirtyVertexBuffers.reset(); + } }; } @@ -100,29 +202,6 @@ namespace opengl { FreeCommands(&commands); } - static GLenum IndexFormatType(nxt::IndexFormat format) { - switch (format) { - case nxt::IndexFormat::Uint16: - return GL_UNSIGNED_SHORT; - case nxt::IndexFormat::Uint32: - return GL_UNSIGNED_INT; - default: - UNREACHABLE(); - } - } - - static GLenum VertexFormatType(nxt::VertexFormat format) { - switch (format) { - case nxt::VertexFormat::FloatR32G32B32A32: - case nxt::VertexFormat::FloatR32G32B32: - case nxt::VertexFormat::FloatR32G32: - case nxt::VertexFormat::FloatR32: - return GL_FLOAT; - default: - UNREACHABLE(); - } - } - void CommandBuffer::Execute() { Command type; PipelineBase* lastPipeline = nullptr; @@ -134,6 +213,7 @@ namespace opengl { persistentPipelineState.SetDefaultState(); PushConstantTracker pushConstants; + InputBufferTracker inputBuffers; RenderPass* currentRenderPass = nullptr; Framebuffer* currentFramebuffer = nullptr; @@ -162,6 +242,7 @@ namespace opengl { { commands.NextCommand(); pushConstants.OnBeginPass(); + inputBuffers.OnBeginPass(); // TODO(kainino@chromium.org): This is added to possibly // work around an issue seen on Windows/Intel. It should @@ -362,6 +443,7 @@ namespace opengl { { DrawArraysCmd* draw = commands.NextCommand(); pushConstants.Apply(lastPipeline, lastGLPipeline); + inputBuffers.Apply(); if (draw->firstInstance > 0) { glDrawArraysInstancedBaseInstance(lastRenderPipeline->GetGLPrimitiveTopology(), @@ -378,6 +460,7 @@ namespace opengl { { DrawElementsCmd* draw = commands.NextCommand(); pushConstants.Apply(lastPipeline, lastGLPipeline); + inputBuffers.Apply(); nxt::IndexFormat indexFormat = lastRenderPipeline->GetIndexFormat(); size_t formatSize = IndexFormatSize(indexFormat); @@ -436,7 +519,9 @@ namespace opengl { lastRenderPipeline = ToBackend(cmd->pipeline).Get(); lastGLPipeline = ToBackend(cmd->pipeline).Get(); lastPipeline = ToBackend(cmd->pipeline).Get(); + pushConstants.OnSetPipeline(lastPipeline); + inputBuffers.OnSetPipeline(lastRenderPipeline); } break; @@ -471,7 +556,6 @@ namespace opengl { const auto& indices = ToBackend(lastPipeline->GetLayout())->GetBindingIndexInfo()[groupIndex]; const auto& layout = group->GetLayout()->GetBindingInfo(); - // TODO(cwallez@chromium.org): iterate over the layout bitmask instead for (uint32_t binding : IterateBitSet(layout.mask)) { switch (layout.types[binding]) { case nxt::BindingType::UniformBuffer: @@ -527,10 +611,8 @@ namespace opengl { case Command::SetIndexBuffer: { SetIndexBufferCmd* cmd = commands.NextCommand(); - - GLuint buffer = ToBackend(cmd->buffer.Get())->GetHandle(); indexBufferOffset = cmd->offset; - glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, buffer); + inputBuffers.OnSetIndexBuffer(cmd->buffer.Get()); } break; @@ -539,37 +621,7 @@ namespace opengl { SetVertexBuffersCmd* cmd = commands.NextCommand(); auto buffers = commands.NextData>(cmd->count); auto offsets = commands.NextData(cmd->count); - - auto inputState = lastRenderPipeline->GetInputState(); - - auto& attributesSetMask = inputState->GetAttributesSetMask(); - for (uint32_t location = 0; location < attributesSetMask.size(); ++location) { - if (!attributesSetMask[location]) { - // This slot is not used in the input state - continue; - } - auto attribute = inputState->GetAttribute(location); - auto slot = attribute.bindingSlot; - ASSERT(slot < kMaxVertexInputs); - if (slot < cmd->startSlot || slot >= cmd->startSlot + cmd->count) { - // This slot is not affected by this call - continue; - } - size_t bufferIndex = slot - cmd->startSlot; - GLuint buffer = ToBackend(buffers[bufferIndex])->GetHandle(); - uint32_t bufferOffset = offsets[bufferIndex]; - - auto input = inputState->GetInput(slot); - - auto components = VertexFormatNumComponents(attribute.format); - auto formatType = VertexFormatType(attribute.format); - - glBindBuffer(GL_ARRAY_BUFFER, buffer); - glVertexAttribPointer( - location, components, formatType, GL_FALSE, - input.stride, - reinterpret_cast(static_cast(bufferOffset + attribute.offset))); - } + inputBuffers.OnSetVertexBuffers(cmd->startSlot, cmd->count, buffers, offsets); } break; diff --git a/src/backend/opengl/GeneratedCodeIncludes.h b/src/backend/opengl/GeneratedCodeIncludes.h index 34a8c256f7..a57f83307d 100644 --- a/src/backend/opengl/GeneratedCodeIncludes.h +++ b/src/backend/opengl/GeneratedCodeIncludes.h @@ -18,6 +18,7 @@ #include "backend/opengl/CommandBufferGL.h" #include "backend/opengl/ComputePipelineGL.h" #include "backend/opengl/DepthStencilStateGL.h" +#include "backend/opengl/InputStateGL.h" #include "backend/opengl/PersistentPipelineStateGL.h" #include "backend/opengl/PipelineLayoutGL.h" #include "backend/opengl/RenderPipelineGL.h" diff --git a/src/backend/opengl/InputStateGL.cpp b/src/backend/opengl/InputStateGL.cpp new file mode 100644 index 0000000000..d7aaaffe45 --- /dev/null +++ b/src/backend/opengl/InputStateGL.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 "backend/opengl/InputStateGL.h" + +#include "backend/opengl/OpenGLBackend.h" +#include "common/Assert.h" + +namespace backend { +namespace opengl { + + InputState::InputState(InputStateBuilder* builder) + : InputStateBase(builder) { + glGenVertexArrays(1, &vertexArrayObject); + glBindVertexArray(vertexArrayObject); + auto& attributesSetMask = GetAttributesSetMask(); + for (uint32_t location = 0; location < attributesSetMask.size(); ++location) { + if (!attributesSetMask[location]) { + continue; + } + auto attribute = GetAttribute(location); + glEnableVertexAttribArray(location); + + attributesUsingInput[attribute.bindingSlot][location] = true; + auto input = GetInput(attribute.bindingSlot); + + if (input.stride == 0) { + // Emulate a stride of zero (constant vertex attribute) by + // setting the attribute instance divisor to a huge number. + glVertexAttribDivisor(location, 0xffffffff); + } else { + switch (input.stepMode) { + case nxt::InputStepMode::Vertex: + break; + case nxt::InputStepMode::Instance: + glVertexAttribDivisor(location, 1); + break; + default: + UNREACHABLE(); + } + } + } + } + + std::bitset InputState::GetAttributesUsingInput(uint32_t slot) const { + return attributesUsingInput[slot]; + } + + GLuint InputState::GetVAO() { + return vertexArrayObject; + } + +} +} diff --git a/src/backend/opengl/InputStateGL.h b/src/backend/opengl/InputStateGL.h new file mode 100644 index 0000000000..098f151539 --- /dev/null +++ b/src/backend/opengl/InputStateGL.h @@ -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. + +#ifndef BACKEND_OPENGL_INPUTSTATEGL_H_ +#define BACKEND_OPENGL_INPUTSTATEGL_H_ + +#include "backend/InputState.h" + +#include "glad/glad.h" + +namespace backend { +namespace opengl { + + class Device; + + class InputState : public InputStateBase { + public: + InputState(InputStateBuilder* builder); + + std::bitset GetAttributesUsingInput(uint32_t slot) const; + GLuint GetVAO(); + + private: + GLuint vertexArrayObject; + std::array, kMaxVertexInputs> attributesUsingInput; + }; + +} +} + +#endif // BACKEND_OPENGL_INPUTSTATEGL_H_ diff --git a/src/backend/opengl/OpenGLBackend.cpp b/src/backend/opengl/OpenGLBackend.cpp index 0443271f46..96e68661a3 100644 --- a/src/backend/opengl/OpenGLBackend.cpp +++ b/src/backend/opengl/OpenGLBackend.cpp @@ -19,6 +19,7 @@ #include "backend/opengl/CommandBufferGL.h" #include "backend/opengl/ComputePipelineGL.h" #include "backend/opengl/DepthStencilStateGL.h" +#include "backend/opengl/InputStateGL.h" #include "backend/opengl/PipelineLayoutGL.h" #include "backend/opengl/RenderPipelineGL.h" #include "backend/opengl/ShaderModuleGL.h" @@ -40,6 +41,7 @@ namespace opengl { *device = reinterpret_cast(new Device); glEnable(GL_DEPTH_TEST); + glEnable(GL_PRIMITIVE_RESTART_FIXED_INDEX); } // Device @@ -117,43 +119,6 @@ namespace opengl { : BindGroupLayoutBase(builder) { } - // InputState - - InputState::InputState(InputStateBuilder* builder) - : InputStateBase(builder) { - glGenVertexArrays(1, &vertexArrayObject); - glBindVertexArray(vertexArrayObject); - auto& attributesSetMask = GetAttributesSetMask(); - for (uint32_t location = 0; location < attributesSetMask.size(); ++location) { - if (!attributesSetMask[location]) { - continue; - } - auto attribute = GetAttribute(location); - glEnableVertexAttribArray(location); - - auto input = GetInput(attribute.bindingSlot); - if (input.stride == 0) { - // Emulate a stride of zero (constant vertex attribute) by - // setting the attribute instance divisor to a huge number. - glVertexAttribDivisor(location, 0xffffffff); - } else { - switch (input.stepMode) { - case nxt::InputStepMode::Vertex: - break; - case nxt::InputStepMode::Instance: - glVertexAttribDivisor(location, 1); - break; - default: - UNREACHABLE(); - } - } - } - } - - GLuint InputState::GetVAO() { - return vertexArrayObject; - } - // Framebuffer Framebuffer::Framebuffer(FramebufferBuilder* builder) diff --git a/src/backend/opengl/OpenGLBackend.h b/src/backend/opengl/OpenGLBackend.h index 2885763341..a4449e1e33 100644 --- a/src/backend/opengl/OpenGLBackend.h +++ b/src/backend/opengl/OpenGLBackend.h @@ -125,15 +125,6 @@ namespace opengl { Framebuffer(FramebufferBuilder* builder); }; - class InputState : public InputStateBase { - public: - InputState(InputStateBuilder* builder); - GLuint GetVAO(); - - private: - GLuint vertexArrayObject; - }; - class Queue : public QueueBase { public: Queue(QueueBuilder* builder); diff --git a/src/backend/opengl/RenderPipelineGL.cpp b/src/backend/opengl/RenderPipelineGL.cpp index 4208e39f1d..a36a0a5159 100644 --- a/src/backend/opengl/RenderPipelineGL.cpp +++ b/src/backend/opengl/RenderPipelineGL.cpp @@ -16,6 +16,7 @@ #include "backend/opengl/BlendStateGL.h" #include "backend/opengl/DepthStencilStateGL.h" +#include "backend/opengl/InputStateGL.h" #include "backend/opengl/OpenGLBackend.h" #include "backend/opengl/PersistentPipelineStateGL.h"