From a07d2024178106df99b82fe21a34402c5200e8f6 Mon Sep 17 00:00:00 2001 From: Dan Goodliffe Date: Sat, 15 Apr 2023 00:21:32 +0100 Subject: Add the dynamicPoint shader for instancing Same as dynamicPoint, but the model matrix is a vertex input --- gfx/gl/sceneShader.cpp | 7 ++++++- gfx/gl/sceneShader.h | 1 + gfx/gl/shaders/dynamicPointInst.vs | 21 +++++++++++++++++++++ 3 files changed, 28 insertions(+), 1 deletion(-) create mode 100644 gfx/gl/shaders/dynamicPointInst.vs diff --git a/gfx/gl/sceneShader.cpp b/gfx/gl/sceneShader.cpp index bcd0590..54f5737 100644 --- a/gfx/gl/sceneShader.cpp +++ b/gfx/gl/sceneShader.cpp @@ -8,6 +8,7 @@ #include #include #include +#include #include #include #include @@ -18,7 +19,11 @@ #include #include -SceneShader::SceneShader() : landmass {fixedPoint_vs, landmass_fs}, absolute {fixedPoint_vs, material_fs} { } +SceneShader::SceneShader() : + basicInst {dynamicPointInst_vs, material_fs}, landmass {fixedPoint_vs, landmass_fs}, absolute {fixedPoint_vs, + material_fs} +{ +} void SceneShader::setViewProjection(const glm::mat4 & viewProjection) const diff --git a/gfx/gl/sceneShader.h b/gfx/gl/sceneShader.h index ed1bb79..0ccf152 100644 --- a/gfx/gl/sceneShader.h +++ b/gfx/gl/sceneShader.h @@ -80,6 +80,7 @@ public: SceneShader(); BasicProgram basic; + SceneProgram basicInst; WaterProgram water; AbsolutePosProgram landmass, absolute; PointLightShader pointLight; diff --git a/gfx/gl/shaders/dynamicPointInst.vs b/gfx/gl/shaders/dynamicPointInst.vs new file mode 100644 index 0000000..016153a --- /dev/null +++ b/gfx/gl/shaders/dynamicPointInst.vs @@ -0,0 +1,21 @@ +#version 330 core + +include(`meshIn.glsl') +layout(location = 6) in mat4 model; +include(`materialInterface.glsl') + +uniform mat4 viewProjection; + +void +main() +{ + vec4 worldPos = model * vec4(position, 1.0); + + FragPos = worldPos.xyz; + TexCoords = texCoord; + Normal = (model * vec4(normal, 0.0)).xyz; + Colour = colour; + Material = material; + + gl_Position = viewProjection * worldPos; +} -- cgit v1.2.3 From 85bf771ea6d8d584f1c735cc963e17571ca0a0d1 Mon Sep 17 00:00:00 2001 From: Dan Goodliffe Date: Mon, 17 Apr 2023 11:54:07 +0100 Subject: First cut of instance vertices and proxy Untested outside unit test, allows the use of a glBuffer as a storage container. To be combined with a vertex array and/or mesh etc for massing drawing with glDrawElementsInstanced --- gfx/gl/instanceVertices.h | 147 ++++++++++++++++++++++++++++++++++++++++++++++ test/Jamfile.jam | 1 + test/test-instancing.cpp | 76 ++++++++++++++++++++++++ 3 files changed, 224 insertions(+) create mode 100644 gfx/gl/instanceVertices.h create mode 100644 test/test-instancing.cpp diff --git a/gfx/gl/instanceVertices.h b/gfx/gl/instanceVertices.h new file mode 100644 index 0000000..cf3b75b --- /dev/null +++ b/gfx/gl/instanceVertices.h @@ -0,0 +1,147 @@ +#pragma once + +#include "glArrays.h" +#include +#include +#include +#include +#include + +template class InstanceVertices { +public: + InstanceVertices(size_t initialSize = 16) : data {allocBuffer(buffer, initialSize)}, next {} { } + + ~InstanceVertices() + { + glUnmapNamedBuffer(buffer); + } + + class InstanceProxy { + public: + InstanceProxy(InstanceVertices * iv, std::size_t idx) : instances {iv}, index {idx} { } + InstanceProxy(InstanceProxy && other) : instances {std::exchange(other.instances, nullptr)}, index {other.index} + { + } + NO_COPY(InstanceProxy); + + ~InstanceProxy() + { + if (instances) { + instances->release(*this); + } + } + + InstanceProxy & + operator=(InstanceProxy && other) + { + instances = std::exchange(other.instances, nullptr); + index = other.index; + } + + operator T &() + { + return *get(); + } + operator const T &() const + { + return *get(); + } + template + T & + operator=(U && v) + { + return instances->data[index] = std::forward(v); + } + + T * + get() + { + return &instances->data[index]; + } + const T * + get() const + { + return &instances->data[index]; + } + T * + operator->() + { + return get(); + } + const T * + operator->() const + { + return get(); + } + T & + operator*() + { + return *get(); + } + const T & + operator*() const + { + return *get(); + } + + private: + friend InstanceVertices; + + InstanceVertices * instances; + std::size_t index; + }; + + template + InstanceProxy + acquire(Params &&... params) + { + if (!unused.empty()) { + auto idx = unused.back(); + unused.pop_back(); + new (&data[idx]) T(std::forward(params)...); + return InstanceProxy {this, idx}; + } + if (next >= data.size()) { + resize(data.size() * 2); + } + new (&data[next]) T(std::forward(params)...); + return InstanceProxy {this, next++}; + } + + void + release(const InstanceProxy & p) + { + data[p.index].~T(); + unused.push_back(p.index); + } + +protected: + friend InstanceProxy; + + static std::span + allocBuffer(const glBuffer & buffer, std::size_t count) + { + glBindBuffer(GL_ARRAY_BUFFER, buffer); + glBufferData(GL_ARRAY_BUFFER, static_cast(sizeof(T) * count), nullptr, GL_DYNAMIC_DRAW); + auto data = static_cast(glMapNamedBuffer(buffer, GL_READ_WRITE)); + glBindBuffer(GL_ARRAY_BUFFER, 0); + return {data, count}; + } + + void + resize(size_t newCapacity) + { + const auto maintain = std::min(newCapacity, data.size()); + const auto maintaind = static_cast(maintain); + std::vector existing; + existing.reserve(maintain); + std::move(data.begin(), data.begin() + maintaind, std::back_inserter(existing)); + data = allocBuffer(buffer, newCapacity); + std::move(existing.begin(), existing.begin() + maintaind, data.begin()); + } + + glBuffer buffer; + std::span data; + std::size_t next; + std::vector unused; +}; diff --git a/test/Jamfile.jam b/test/Jamfile.jam index b0eed5e..9cdcef4 100644 --- a/test/Jamfile.jam +++ b/test/Jamfile.jam @@ -57,6 +57,7 @@ run test-assetFactory.cpp : -- : [ sequence.insertion-sort [ glob-tree $(res) : run perf-assetFactory.cpp : -- : test-assetFactory : benchmark test ; run perf-persistence.cpp : -- : test-persistence : benchmark test ; run test-worker.cpp ; +run test-instancing.cpp : : : test ; compile test-static-enumDetails.cpp ; compile test-static-stream_support.cpp ; explicit perf-assetFactory ; diff --git a/test/test-instancing.cpp b/test/test-instancing.cpp new file mode 100644 index 0000000..7191567 --- /dev/null +++ b/test/test-instancing.cpp @@ -0,0 +1,76 @@ +#define BOOST_TEST_MODULE instancing + +#include "testHelpers.h" +#include "testMainWindow.h" +#include "ui/applicationBase.h" +#include +#include + +#include + +BOOST_GLOBAL_FIXTURE(ApplicationBase); +BOOST_GLOBAL_FIXTURE(TestMainWindow); + +BOOST_FIXTURE_TEST_SUITE(i, InstanceVertices) + +BOOST_AUTO_TEST_CASE(createDestroy) +{ + BOOST_REQUIRE(data.data()); + BOOST_CHECK_EQUAL(0, next); + BOOST_CHECK(unused.empty()); +} + +BOOST_AUTO_TEST_CASE(storeRetreive) +{ // Read write raw buffer, not normally allowed + std::vector test(data.size()); + std::copy(test.begin(), test.end(), data.begin()); + BOOST_CHECK_EQUAL_COLLECTIONS(test.begin(), test.end(), data.begin(), data.end()); +} + +BOOST_AUTO_TEST_CASE(acquireRelease) +{ + { + auto proxy = acquire(); + *proxy = 20; + BOOST_CHECK_EQUAL(1, next); + } + BOOST_CHECK_EQUAL(1, next); + BOOST_CHECK_EQUAL(1, unused.size()); + BOOST_CHECK_EQUAL(0, unused.front()); +} + +BOOST_AUTO_TEST_CASE(acquireReleaseMove) +{ + { + auto proxy1 = acquire(); + *proxy1 = 20; + BOOST_CHECK_EQUAL(1, next); + auto proxy2 = std::move(proxy1); + *proxy2 = 40; + BOOST_CHECK_EQUAL(1, next); + } + BOOST_CHECK_EQUAL(1, next); + BOOST_CHECK_EQUAL(1, unused.size()); + BOOST_CHECK_EQUAL(0, unused.front()); +} + +BOOST_AUTO_TEST_CASE(initialize) +{ + auto proxy = acquire(5); + BOOST_CHECK_EQUAL(*proxy, 5); +} + +BOOST_AUTO_TEST_CASE(resize) +{ + constexpr auto COUNT = 500; + std::vector proxies; + std::vector expected; + for (auto n = 0; n < COUNT; n++) { + proxies.push_back(acquire(n)); + expected.emplace_back(n); + } + BOOST_CHECK_EQUAL_COLLECTIONS(expected.begin(), expected.end(), data.begin(), data.begin() + COUNT); + BOOST_CHECK_EQUAL_COLLECTIONS(expected.begin(), expected.end(), proxies.begin(), proxies.end()); +} + +BOOST_AUTO_TEST_SUITE_END() -- cgit v1.2.3 From ed7e68e58ffdc369dbd6f7ac3fbebe19afeb56a2 Mon Sep 17 00:00:00 2001 From: Dan Goodliffe Date: Mon, 17 Apr 2023 11:57:13 +0100 Subject: Create a large test forest --- application/main.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/application/main.cpp b/application/main.cpp index aea3d2e..a0b2dfd 100644 --- a/application/main.cpp +++ b/application/main.cpp @@ -79,7 +79,11 @@ public: train->currentActivity = train->orders.current()->createActivity(); auto foliage = std::dynamic_pointer_cast(assets.at("Tree-01-1")); - world.create(foliage, Location {{-1100, -1100, 0}}); + for (float x = 900; x < 1100; x += 3) { + for (float y = 900; y < 1100; y += 3) { + world.create(foliage, Location {geoData->positionAt({-x, -y})}); + } + } } auto t_start = std::chrono::high_resolution_clock::now(); -- cgit v1.2.3 From f898bdeae6a9f8d6525f73c51996062f8c5e8f57 Mon Sep 17 00:00:00 2001 From: Dan Goodliffe Date: Mon, 17 Apr 2023 12:26:22 +0100 Subject: Assets moved to global game state --- application/main.cpp | 2 +- game/gamestate.h | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/application/main.cpp b/application/main.cpp index a0b2dfd..9feb80b 100644 --- a/application/main.cpp +++ b/application/main.cpp @@ -44,9 +44,9 @@ public: windows.create(DISPLAY_WIDTH, DISPLAY_HEIGHT); world.create(geoData); + assets = AssetFactory::loadAll("res"); { - const auto assets = AssetFactory::loadAll("res"); auto rl = world.create(); const glm::vec3 j {-1120, -1100, 3}, k {-1100, -1000, 15}, l {-1000, -800, 20}, m {-900, -600, 30}, n {-600, -500, 32}, o {-500, -800, 30}, p {-600, -900, 25}, q {-1025, -1175, 10}, diff --git a/game/gamestate.h b/game/gamestate.h index 605aac4..db223c0 100644 --- a/game/gamestate.h +++ b/game/gamestate.h @@ -1,5 +1,6 @@ #pragma once +#include "assetFactory/assetFactory.h" #include #include #include @@ -16,5 +17,6 @@ public: Collection world; std::shared_ptr geoData; + AssetFactory::Assets assets; }; extern GameState * gameState; -- cgit v1.2.3 From e4414150da485a8aacdd221e23cad801f8d532c6 Mon Sep 17 00:00:00 2001 From: Dan Goodliffe Date: Mon, 17 Apr 2023 17:04:32 +0100 Subject: Fix up the way spotlight shader works Was mostly through lack of understanding and coincidences. Position is now the only vertex data, direction is moved to a uniform. Instancing will address this by making everything instance data. --- gfx/gl/sceneShader.cpp | 10 +++++----- gfx/gl/sceneShader.h | 1 + gfx/gl/shaders/spotLight.vs | 2 +- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/gfx/gl/sceneShader.cpp b/gfx/gl/sceneShader.cpp index 54f5737..37eacbd 100644 --- a/gfx/gl/sceneShader.cpp +++ b/gfx/gl/sceneShader.cpp @@ -20,8 +20,8 @@ #include SceneShader::SceneShader() : - basicInst {dynamicPointInst_vs, material_fs}, landmass {fixedPoint_vs, landmass_fs}, absolute {fixedPoint_vs, - material_fs} + basicInst {dynamicPointInst_vs, material_fs}, landmass {fixedPoint_vs, landmass_fs}, + absolute {fixedPoint_vs, material_fs} { } @@ -104,8 +104,8 @@ SceneShader::PointLightShader::add(const glm::vec3 & position, const glm::vec3 & } SceneShader::SpotLightShader::SpotLightShader() : - SceneProgram {spotLight_vs, spotLight_gs, spotLight_fs}, colourLoc {*this, "colour"}, kqLoc {*this, "kq"}, - arcLoc {*this, "arc"} + SceneProgram {spotLight_vs, spotLight_gs, spotLight_fs}, directionLoc {*this, "v_direction"}, + colourLoc {*this, "colour"}, kqLoc {*this, "kq"}, arcLoc {*this, "arc"} { using v3pair = std::pair; VertexArrayObject::configure<&v3pair::first, &v3pair::second>(va, b); @@ -119,9 +119,9 @@ SceneShader::SpotLightShader::add(const glm::vec3 & position, const glm::vec3 & glBindVertexArray(va); glBindBuffer(GL_ARRAY_BUFFER, b); glUniform3fv(colourLoc, 1, glm::value_ptr(colour)); + glUniform3fv(directionLoc, 1, glm::value_ptr(direction)); glUniform1f(kqLoc, kq); glUniform1f(arcLoc, arc); glBufferSubData(GL_ARRAY_BUFFER, 0, sizeof(glm::vec3), glm::value_ptr(position)); - glBufferSubData(GL_ARRAY_BUFFER, sizeof(glm::vec3), sizeof(glm::vec3), glm::value_ptr(direction)); glDrawArrays(GL_POINTS, 0, 1); } diff --git a/gfx/gl/sceneShader.h b/gfx/gl/sceneShader.h index 0ccf152..c135050 100644 --- a/gfx/gl/sceneShader.h +++ b/gfx/gl/sceneShader.h @@ -69,6 +69,7 @@ class SceneShader { const float arc) const; private: + UniformLocation directionLoc; UniformLocation colourLoc; UniformLocation kqLoc; UniformLocation arcLoc; diff --git a/gfx/gl/shaders/spotLight.vs b/gfx/gl/shaders/spotLight.vs index e648553..dca0854 100644 --- a/gfx/gl/shaders/spotLight.vs +++ b/gfx/gl/shaders/spotLight.vs @@ -1,8 +1,8 @@ #version 330 core layout(location = 0) in vec3 v_position; -layout(location = 1) in vec3 v_direction; +uniform vec3 v_direction; uniform vec3 colour; uniform float kq; uniform float arc; -- cgit v1.2.3 From da1c1336596d361678b3f641a1d23ab89e078789 Mon Sep 17 00:00:00 2001 From: Dan Goodliffe Date: Mon, 17 Apr 2023 17:09:40 +0100 Subject: Revamp how VertexArrayObject configures attributes and data --- gfx/gl/sceneRenderer.cpp | 6 ++-- gfx/gl/sceneShader.cpp | 8 ++--- gfx/gl/vertexArrayObject.hpp | 78 ++++++++++++++++++++++++-------------------- gfx/models/mesh.cpp | 7 ++-- 4 files changed, 54 insertions(+), 45 deletions(-) diff --git a/gfx/gl/sceneRenderer.cpp b/gfx/gl/sceneRenderer.cpp index 873dc5b..6542bea 100644 --- a/gfx/gl/sceneRenderer.cpp +++ b/gfx/gl/sceneRenderer.cpp @@ -18,7 +18,7 @@ SceneRenderer::SceneRenderer(glm::ivec2 s, GLuint o) : lighting {lighting_vs, lighting_fs}, shadowMapper {{2048, 2048}} { shader.setViewPort({0, 0, size.x, size.y}); - VertexArrayObject::configure(displayVAO, displayVBO, displayVAOdata); + VertexArrayObject {displayVAO}.addAttribs(displayVBO, displayVAOdata); glBindFramebuffer(GL_FRAMEBUFFER, gBuffer); const auto configuregdata @@ -128,8 +128,8 @@ SceneRenderer::renderQuad() const SceneRenderer::DirectionalLightProgram::DirectionalLightProgram() : Program {lighting_vs, directionalLight_fs}, directionLoc {*this, "lightDirection"}, colourLoc {*this, "lightColour"}, lightViewProjectionLoc {*this, "lightViewProjection"}, - lightViewProjectionCountLoc {*this, "lightViewProjectionCount"}, lightViewShadowMapRegionLoc { - *this, "shadowMapRegion"} + lightViewProjectionCountLoc {*this, "lightViewProjectionCount"}, + lightViewShadowMapRegionLoc {*this, "shadowMapRegion"} { } diff --git a/gfx/gl/sceneShader.cpp b/gfx/gl/sceneShader.cpp index 37eacbd..bccd970 100644 --- a/gfx/gl/sceneShader.cpp +++ b/gfx/gl/sceneShader.cpp @@ -88,7 +88,7 @@ SceneShader::WaterProgram::use(float waveCycle) const SceneShader::PointLightShader::PointLightShader() : SceneProgram {pointLight_vs, pointLight_gs, pointLight_fs}, colourLoc {*this, "colour"}, kqLoc {*this, "kq"} { - VertexArrayObject::configure(va, b); + VertexArrayObject {va}.addAttribs(b); } void @@ -99,7 +99,7 @@ SceneShader::PointLightShader::add(const glm::vec3 & position, const glm::vec3 & glBindBuffer(GL_ARRAY_BUFFER, b); glUniform3fv(colourLoc, 1, glm::value_ptr(colour)); glUniform1f(kqLoc, kq); - glBufferSubData(GL_ARRAY_BUFFER, 0, sizeof(glm::vec3), glm::value_ptr(position)); + glBufferData(GL_ARRAY_BUFFER, sizeof(glm::vec3), glm::value_ptr(position), GL_DYNAMIC_DRAW); glDrawArrays(GL_POINTS, 0, 1); } @@ -108,7 +108,7 @@ SceneShader::SpotLightShader::SpotLightShader() : colourLoc {*this, "colour"}, kqLoc {*this, "kq"}, arcLoc {*this, "arc"} { using v3pair = std::pair; - VertexArrayObject::configure<&v3pair::first, &v3pair::second>(va, b); + VertexArrayObject {va}.addAttribs(b); } void @@ -122,6 +122,6 @@ SceneShader::SpotLightShader::add(const glm::vec3 & position, const glm::vec3 & glUniform3fv(directionLoc, 1, glm::value_ptr(direction)); glUniform1f(kqLoc, kq); glUniform1f(arcLoc, arc); - glBufferSubData(GL_ARRAY_BUFFER, 0, sizeof(glm::vec3), glm::value_ptr(position)); + glBufferData(GL_ARRAY_BUFFER, sizeof(glm::vec3), glm::value_ptr(position), GL_DYNAMIC_DRAW); glDrawArrays(GL_POINTS, 0, 1); } diff --git a/gfx/gl/vertexArrayObject.hpp b/gfx/gl/vertexArrayObject.hpp index 5b9fc60..3e2a18b 100644 --- a/gfx/gl/vertexArrayObject.hpp +++ b/gfx/gl/vertexArrayObject.hpp @@ -2,47 +2,56 @@ #include "collections.hpp" #include "gl_traits.hpp" +#include "special_members.hpp" #include -#include -template class VertexArrayObject { +class VertexArrayObject { public: - template - static void - configure(const GLuint arrayObject, const GLuint arrayBuffer, const GLuint indexBuffer, - const SequentialCollection auto & vertices, const SequentialCollection auto & indices) + [[nodiscard]] VertexArrayObject(const GLuint arrayObject) { glBindVertexArray(arrayObject); - - configure_attribs(arrayBuffer); - data(vertices, arrayBuffer, GL_ARRAY_BUFFER); - data(indices, indexBuffer, GL_ELEMENT_ARRAY_BUFFER); - + } + ~VertexArrayObject() + { glBindVertexArray(0); } + NO_MOVE(VertexArrayObject); + NO_COPY(VertexArrayObject); - template - static void - configure(const GLuint arrayObject, const GLuint arrayBuffer, const SequentialCollection auto & vertices) - { - glBindVertexArray(arrayObject); + template struct MP { + constexpr MP(m T::*p) : P {p} { } + operator void *() const + { + return &(static_cast(nullptr)->*P); + } + m T::*P; + using value_type = m; + }; + template MP(m T::*) -> MP; - configure_attribs(arrayBuffer); + template + VertexArrayObject & + addAttribs(const GLuint arrayBuffer, const SequentialCollection auto & vertices) + { + addAttribs(arrayBuffer); data(vertices, arrayBuffer, GL_ARRAY_BUFFER); - - glBindVertexArray(0); + return *this; } - template - static void - configure(const GLuint arrayObject, const GLuint arrayBuffer) + template + VertexArrayObject & + addAttribs(const GLuint arrayBuffer) { - glBindVertexArray(arrayObject); - - configure_attribs(arrayBuffer); - glBufferData(GL_ARRAY_BUFFER, static_cast(sizeof(Vertex)), nullptr, GL_DYNAMIC_DRAW); + configure_attribs(arrayBuffer); + return *this; + } - glBindVertexArray(0); + template + VertexArrayObject & + addIndices(const GLuint arrayBuffer, const Indices & indices) + { + data(indices, arrayBuffer, GL_ELEMENT_ARRAY_BUFFER); + return *this; } private: @@ -55,34 +64,33 @@ private: glBufferData(target, static_cast(sizeof(Value) * data.size()), data.data(), GL_STATIC_DRAW); } - template + template static void set_pointer(const GLuint vertexArrayId, const void * ptr) { glEnableVertexAttribArray(vertexArrayId); using traits = gl_traits; - traits::vertexAttribFunc(vertexArrayId, traits::size, traits::type, sizeof(Vertex), ptr); + traits::vertexAttribFunc(vertexArrayId, traits::size, traits::type, sizeof(VertexT), ptr); } - template + template static void set_pointer(const GLuint vertexArrayId) { - set_pointer().*attrib)>>( - vertexArrayId, &(static_cast(nullptr)->*attrib)); + set_pointer(vertexArrayId, attrib); } - template + template static void configure_attribs(const GLuint arrayBuffer) { glBindBuffer(GL_ARRAY_BUFFER, arrayBuffer); if constexpr (sizeof...(attribs) == 0) { - set_pointer(0, nullptr); + set_pointer(0, nullptr); } else { GLuint vertexArrayId {}; - (set_pointer(vertexArrayId++), ...); + (set_pointer(vertexArrayId++), ...); } } }; diff --git a/gfx/models/mesh.cpp b/gfx/models/mesh.cpp index 2719211..4200703 100644 --- a/gfx/models/mesh.cpp +++ b/gfx/models/mesh.cpp @@ -7,9 +7,10 @@ Mesh::Mesh(const std::span vertices, const std::span indices, GLenum m) : m_numIndices {static_cast(indices.size())}, mode {m} { - VertexArrayObject::configure<&Vertex::pos, &Vertex::texCoord, &Vertex::normal, &Vertex::colour, - &Vertex::material>( - m_vertexArrayObject, m_vertexArrayBuffers[0], m_vertexArrayBuffers[1], vertices, indices); + VertexArrayObject {m_vertexArrayObject} + .addAttribs( + m_vertexArrayBuffers[0], vertices) + .addIndices(m_vertexArrayBuffers[1], indices); } void -- cgit v1.2.3 From 9b0828ea5bb6cb4e92d6019785b3ceb88e2a58be Mon Sep 17 00:00:00 2001 From: Dan Goodliffe Date: Mon, 17 Apr 2023 22:43:51 +0100 Subject: Separate storing of mesh vertex/index data from configuring VAO --- gfx/gl/vertexArrayObject.hpp | 11 +++++++++-- gfx/models/mesh.cpp | 14 +++++++++++--- gfx/models/mesh.h | 2 ++ 3 files changed, 22 insertions(+), 5 deletions(-) diff --git a/gfx/gl/vertexArrayObject.hpp b/gfx/gl/vertexArrayObject.hpp index 3e2a18b..8c828c8 100644 --- a/gfx/gl/vertexArrayObject.hpp +++ b/gfx/gl/vertexArrayObject.hpp @@ -7,7 +7,7 @@ class VertexArrayObject { public: - [[nodiscard]] VertexArrayObject(const GLuint arrayObject) + template [[nodiscard]] VertexArrayObject(const T & arrayObject) { glBindVertexArray(arrayObject); } @@ -54,7 +54,13 @@ public: return *this; } -private: + VertexArrayObject & + addIndices(const GLuint arrayBuffer) + { + glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, arrayBuffer); + return *this; + } + template static void data(const Data & data, const GLuint arrayBuffer, GLenum target) @@ -64,6 +70,7 @@ private: glBufferData(target, static_cast(sizeof(Value) * data.size()), data.data(), GL_STATIC_DRAW); } +private: template static void set_pointer(const GLuint vertexArrayId, const void * ptr) diff --git a/gfx/models/mesh.cpp b/gfx/models/mesh.cpp index 4200703..2c849e7 100644 --- a/gfx/models/mesh.cpp +++ b/gfx/models/mesh.cpp @@ -7,10 +7,18 @@ Mesh::Mesh(const std::span vertices, const std::span indices, GLenum m) : m_numIndices {static_cast(indices.size())}, mode {m} { - VertexArrayObject {m_vertexArrayObject} + VertexArrayObject::data(vertices, m_vertexArrayBuffers[0], GL_ARRAY_BUFFER); + VertexArrayObject::data(indices, m_vertexArrayBuffers[1], GL_ARRAY_BUFFER); + configureVAO(m_vertexArrayObject); +} + +VertexArrayObject & +Mesh::configureVAO(VertexArrayObject && vao) const +{ + return vao .addAttribs( - m_vertexArrayBuffers[0], vertices) - .addIndices(m_vertexArrayBuffers[1], indices); + m_vertexArrayBuffers[0]) + .addIndices(m_vertexArrayBuffers[1]); } void diff --git a/gfx/models/mesh.h b/gfx/models/mesh.h index 25a9064..0af8d70 100644 --- a/gfx/models/mesh.h +++ b/gfx/models/mesh.h @@ -7,12 +7,14 @@ #include class Vertex; +class VertexArrayObject; class Mesh : public ConstTypeDefs { public: Mesh(const std::span vertices, const std::span indices, GLenum = GL_TRIANGLES); void Draw() const; + VertexArrayObject & configureVAO(VertexArrayObject &&) const; private: glVertexArray m_vertexArrayObject; -- cgit v1.2.3 From 21459ac4c92b82f2a3eeb1d9a6b462726001084c Mon Sep 17 00:00:00 2001 From: Dan Goodliffe Date: Tue, 18 Apr 2023 00:08:18 +0100 Subject: Specialize vertexAttribFunc for matrices because there's an upper limit of size 4 on attrib pointers --- gfx/gl/vertexArrayObject.hpp | 10 +++++----- lib/gl_traits.hpp | 17 ++++++++++++++--- 2 files changed, 19 insertions(+), 8 deletions(-) diff --git a/gfx/gl/vertexArrayObject.hpp b/gfx/gl/vertexArrayObject.hpp index 8c828c8..fdc43e9 100644 --- a/gfx/gl/vertexArrayObject.hpp +++ b/gfx/gl/vertexArrayObject.hpp @@ -72,19 +72,19 @@ public: private: template - static void + static auto set_pointer(const GLuint vertexArrayId, const void * ptr) { glEnableVertexAttribArray(vertexArrayId); using traits = gl_traits; - traits::vertexAttribFunc(vertexArrayId, traits::size, traits::type, sizeof(VertexT), ptr); + return traits::vertexAttribFunc(vertexArrayId, traits::size, traits::type, sizeof(VertexT), ptr); } template - static void + static auto set_pointer(const GLuint vertexArrayId) { - set_pointer(vertexArrayId, attrib); + return set_pointer(vertexArrayId, attrib); } template @@ -97,7 +97,7 @@ private: } else { GLuint vertexArrayId {}; - (set_pointer(vertexArrayId++), ...); + ((vertexArrayId += set_pointer(vertexArrayId)), ...); } } }; diff --git a/lib/gl_traits.hpp b/lib/gl_traits.hpp index e2e689d..10b42e5 100644 --- a/lib/gl_traits.hpp +++ b/lib/gl_traits.hpp @@ -11,20 +11,23 @@ struct gl_traits_base { }; struct gl_traits_float : public gl_traits_base { static constexpr auto vertexAttribFunc { - [](GLuint index, GLint size, GLenum type, GLsizei stride, const void * pointer) { + [](GLuint index, GLint size, GLenum type, GLsizei stride, const void * pointer) -> GLuint { glVertexAttribPointer(index, size, type, GL_FALSE, stride, pointer); + return 1; }}; }; struct gl_traits_longfloat : public gl_traits_base { static constexpr auto vertexAttribFunc { - [](GLuint index, GLint size, GLenum type, GLsizei stride, const void * pointer) { + [](GLuint index, GLint size, GLenum type, GLsizei stride, const void * pointer) -> GLuint { glVertexAttribLPointer(index, size, type, stride, pointer); + return 1; }}; }; struct gl_traits_integer : public gl_traits_base { static constexpr auto vertexAttribFunc { - [](GLuint index, GLint size, GLenum type, GLsizei stride, const void * pointer) { + [](GLuint index, GLint size, GLenum type, GLsizei stride, const void * pointer) -> GLuint { glVertexAttribIPointer(index, size, type, stride, pointer); + return 1; }}; }; template<> struct gl_traits : public gl_traits_float { @@ -63,4 +66,12 @@ template struct gl_traits struct gl_traits> : public gl_traits { static constexpr GLint size {C * R}; + static constexpr auto vertexAttribFunc { + [](GLuint index, GLint, GLenum type, GLsizei stride, const void * pointer) -> GLuint { + const auto m = static_cast>(pointer); + for (glm::length_t r = 0; r < R; r++) { + glVertexAttribPointer(index, C, type, GL_FALSE, stride, &m[r]); + } + return R; + }}; }; -- cgit v1.2.3 From 97cab8953bfcc8a6d4ae6e327ec57a6d70f8d7e4 Mon Sep 17 00:00:00 2001 From: Dan Goodliffe Date: Tue, 18 Apr 2023 01:00:41 +0100 Subject: Fix location of model matrix in vertex shader --- gfx/gl/shaders/dynamicPointInst.vs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gfx/gl/shaders/dynamicPointInst.vs b/gfx/gl/shaders/dynamicPointInst.vs index 016153a..1c66979 100644 --- a/gfx/gl/shaders/dynamicPointInst.vs +++ b/gfx/gl/shaders/dynamicPointInst.vs @@ -1,7 +1,7 @@ #version 330 core include(`meshIn.glsl') -layout(location = 6) in mat4 model; +layout(location = 5) in mat4 model; include(`materialInterface.glsl') uniform mat4 viewProjection; -- cgit v1.2.3 From 93f11a006b94ccecd7a1fd1dfba0adff56ae7c38 Mon Sep 17 00:00:00 2001 From: Dan Goodliffe Date: Wed, 19 Apr 2023 00:51:35 +0100 Subject: Drop performance debug to warning and stop disabling error checking for texture save --- test/test-assetFactory.cpp | 1 - test/test-render.cpp | 3 --- test/testMainWindow.cpp | 3 ++- 3 files changed, 2 insertions(+), 5 deletions(-) diff --git a/test/test-assetFactory.cpp b/test/test-assetFactory.cpp index 3d79213..82543f3 100644 --- a/test/test-assetFactory.cpp +++ b/test/test-assetFactory.cpp @@ -29,7 +29,6 @@ public: FactoryFixture() : sceneRenderer {size, output} { } ~FactoryFixture() { - glDisable(GL_DEBUG_OUTPUT); auto outpath = (TMP / boost::unit_test::framework::current_test_case().full_name()).replace_extension(".tga"); std::filesystem::create_directories(outpath.parent_path()); Texture::save(outImage, outpath.c_str()); diff --git a/test/test-render.cpp b/test/test-render.cpp index 45acab5..1643068 100644 --- a/test/test-render.cpp +++ b/test/test-render.cpp @@ -86,7 +86,6 @@ BOOST_AUTO_TEST_CASE(basic) ss.camera.setView({-10, -10, 60}, glm::normalize(glm::vec3 {1, 1, -0.5F})); const TestScene scene; ss.render(scene); - glDisable(GL_DEBUG_OUTPUT); Texture::save(outImage, "/tmp/basic.tga"); } @@ -114,7 +113,6 @@ BOOST_AUTO_TEST_CASE(pointlight) }; const PointLightScene scene; ss.render(scene); - glDisable(GL_DEBUG_OUTPUT); Texture::save(outImage, "/tmp/pointlight.tga"); } @@ -141,7 +139,6 @@ BOOST_AUTO_TEST_CASE(spotlight) }; const PointLightScene scene; ss.render(scene); - glDisable(GL_DEBUG_OUTPUT); Texture::save(outImage, "/tmp/spotlight.tga"); } diff --git a/test/testMainWindow.cpp b/test/testMainWindow.cpp index 49e18f1..57e3473 100644 --- a/test/testMainWindow.cpp +++ b/test/testMainWindow.cpp @@ -12,11 +12,12 @@ TestMainWindow::TestMainWindow() : Window {1, 1, __FILE__, SDL_WINDOW_OPENGL | S (type == GL_DEBUG_TYPE_ERROR ? "** GL ERROR **" : ""), type, severity, message); switch (type) { case GL_DEBUG_TYPE_ERROR: - case GL_DEBUG_TYPE_PERFORMANCE: case GL_DEBUG_TYPE_PORTABILITY: case GL_DEBUG_TYPE_DEPRECATED_BEHAVIOR: case GL_DEBUG_TYPE_UNDEFINED_BEHAVIOR: BOOST_TEST_ERROR(msg.get()); + case GL_DEBUG_TYPE_PERFORMANCE: + BOOST_TEST_WARN(msg.get()); default: BOOST_TEST_MESSAGE(msg.get()); } -- cgit v1.2.3 From 4360094ff6e9f4d245f832c58f673ae4d1f75950 Mon Sep 17 00:00:00 2001 From: Dan Goodliffe Date: Wed, 19 Apr 2023 01:45:47 +0100 Subject: Fixup vertexAttribFunc for matrices --- lib/gl_traits.hpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/gl_traits.hpp b/lib/gl_traits.hpp index 10b42e5..6ff8905 100644 --- a/lib/gl_traits.hpp +++ b/lib/gl_traits.hpp @@ -68,9 +68,9 @@ struct gl_traits> : public gl_traits { static constexpr GLint size {C * R}; static constexpr auto vertexAttribFunc { [](GLuint index, GLint, GLenum type, GLsizei stride, const void * pointer) -> GLuint { - const auto m = static_cast>(pointer); - for (glm::length_t r = 0; r < R; r++) { - glVertexAttribPointer(index, C, type, GL_FALSE, stride, &m[r]); + const auto base = static_cast(pointer); + for (GLuint r = 0; r < R; r++) { + glVertexAttribPointer(index + r, C, type, GL_FALSE, stride, base + (r * C)); } return R; }}; -- cgit v1.2.3 From f3343e1cc8a56f039888d4d375a6d5a088a68494 Mon Sep 17 00:00:00 2001 From: Dan Goodliffe Date: Wed, 19 Apr 2023 01:52:46 +0100 Subject: Export mesh size and primitive type --- gfx/models/mesh.cpp | 12 ++++++++++++ gfx/models/mesh.h | 2 ++ 2 files changed, 14 insertions(+) diff --git a/gfx/models/mesh.cpp b/gfx/models/mesh.cpp index 2c849e7..55759cb 100644 --- a/gfx/models/mesh.cpp +++ b/gfx/models/mesh.cpp @@ -21,6 +21,18 @@ Mesh::configureVAO(VertexArrayObject && vao) const .addIndices(m_vertexArrayBuffers[1]); } +GLsizei +Mesh::count() const +{ + return m_numIndices; +} + +GLenum +Mesh::type() const +{ + return mode; +} + void Mesh::Draw() const { diff --git a/gfx/models/mesh.h b/gfx/models/mesh.h index 0af8d70..472b7ed 100644 --- a/gfx/models/mesh.h +++ b/gfx/models/mesh.h @@ -15,6 +15,8 @@ public: void Draw() const; VertexArrayObject & configureVAO(VertexArrayObject &&) const; + GLsizei count() const; + GLenum type() const; private: glVertexArray m_vertexArrayObject; -- cgit v1.2.3 From 2c4198bc83b5007a3ba7c00f85e4e70fcb9f6177 Mon Sep 17 00:00:00 2001 From: Dan Goodliffe Date: Wed, 19 Apr 2023 02:05:56 +0100 Subject: Fix type of basic instanced shader It's now AbsolutePosProgram, which is kind of right, in that it takes no location data via uniform, it's all in the vertex data. --- gfx/gl/sceneShader.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/gfx/gl/sceneShader.h b/gfx/gl/sceneShader.h index c135050..ead184e 100644 --- a/gfx/gl/sceneShader.h +++ b/gfx/gl/sceneShader.h @@ -81,9 +81,8 @@ public: SceneShader(); BasicProgram basic; - SceneProgram basicInst; WaterProgram water; - AbsolutePosProgram landmass, absolute; + AbsolutePosProgram basicInst, landmass, absolute; PointLightShader pointLight; SpotLightShader spotLight; -- cgit v1.2.3 From 49cd81dbe4044d2cc58615b78b7941484359ffb7 Mon Sep 17 00:00:00 2001 From: Dan Goodliffe Date: Wed, 19 Apr 2023 02:12:49 +0100 Subject: Add basic instanced shader to those which get the viewProjection configured --- gfx/gl/sceneShader.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/gfx/gl/sceneShader.cpp b/gfx/gl/sceneShader.cpp index bccd970..00d9826 100644 --- a/gfx/gl/sceneShader.cpp +++ b/gfx/gl/sceneShader.cpp @@ -28,8 +28,8 @@ SceneShader::SceneShader() : void SceneShader::setViewProjection(const glm::mat4 & viewProjection) const { - for (const auto & prog : - std::array {&basic, &water, &landmass, &absolute, &pointLight, &spotLight}) { + for (const auto & prog : std::array { + &basic, &basicInst, &water, &landmass, &absolute, &pointLight, &spotLight}) { prog->setViewProjection(viewProjection); } } @@ -37,8 +37,8 @@ SceneShader::setViewProjection(const glm::mat4 & viewProjection) const void SceneShader::setViewPort(const glm::ivec4 & viewPort) const { - for (const auto & prog : - std::array {&basic, &water, &landmass, &absolute, &pointLight, &spotLight}) { + for (const auto & prog : std::array { + &basic, &basicInst, &water, &landmass, &absolute, &pointLight, &spotLight}) { prog->setViewPort(viewPort); } } -- cgit v1.2.3 From 6b0b13d4b61eb95fb515bd49a8a70a06ea0748ab Mon Sep 17 00:00:00 2001 From: Dan Goodliffe Date: Wed, 19 Apr 2023 14:36:57 +0100 Subject: Persist vertexArrayId across multiple calls Allows chaining together to build VAO from multiple buffers --- gfx/gl/vertexArrayObject.hpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/gfx/gl/vertexArrayObject.hpp b/gfx/gl/vertexArrayObject.hpp index fdc43e9..2285ed0 100644 --- a/gfx/gl/vertexArrayObject.hpp +++ b/gfx/gl/vertexArrayObject.hpp @@ -88,16 +88,17 @@ private: } template - static void + void configure_attribs(const GLuint arrayBuffer) { glBindBuffer(GL_ARRAY_BUFFER, arrayBuffer); if constexpr (sizeof...(attribs) == 0) { - set_pointer(0, nullptr); + vertexArrayId += set_pointer(vertexArrayId, nullptr); } else { - GLuint vertexArrayId {}; ((vertexArrayId += set_pointer(vertexArrayId)), ...); } } + + GLuint vertexArrayId {}; }; -- cgit v1.2.3 From c7396955e9d0437dc24d5ede09724fa47e0693af Mon Sep 17 00:00:00 2001 From: Dan Goodliffe Date: Wed, 19 Apr 2023 14:42:11 +0100 Subject: Enable all vertex array attribs configured by vertexAttribFunc --- gfx/gl/vertexArrayObject.hpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/gfx/gl/vertexArrayObject.hpp b/gfx/gl/vertexArrayObject.hpp index 2285ed0..92e0325 100644 --- a/gfx/gl/vertexArrayObject.hpp +++ b/gfx/gl/vertexArrayObject.hpp @@ -75,9 +75,13 @@ private: static auto set_pointer(const GLuint vertexArrayId, const void * ptr) { - glEnableVertexAttribArray(vertexArrayId); using traits = gl_traits; - return traits::vertexAttribFunc(vertexArrayId, traits::size, traits::type, sizeof(VertexT), ptr); + const auto usedAttribs + = traits::vertexAttribFunc(vertexArrayId, traits::size, traits::type, sizeof(VertexT), ptr); + for (GLuint i {}; i < usedAttribs; i++) { + glEnableVertexAttribArray(vertexArrayId + i); + } + return usedAttribs; } template -- cgit v1.2.3 From 5e0233646ef0170e54dfd9c9b991ffdc7674ba84 Mon Sep 17 00:00:00 2001 From: Dan Goodliffe Date: Wed, 19 Apr 2023 14:48:04 +0100 Subject: Support setting vertex attrib divisor --- gfx/gl/vertexArrayObject.hpp | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/gfx/gl/vertexArrayObject.hpp b/gfx/gl/vertexArrayObject.hpp index 92e0325..7ded03e 100644 --- a/gfx/gl/vertexArrayObject.hpp +++ b/gfx/gl/vertexArrayObject.hpp @@ -31,18 +31,18 @@ public: template VertexArrayObject & - addAttribs(const GLuint arrayBuffer, const SequentialCollection auto & vertices) + addAttribs(const GLuint arrayBuffer, const SequentialCollection auto & vertices, const GLuint divisor = 0) { - addAttribs(arrayBuffer); + addAttribs(arrayBuffer, divisor); data(vertices, arrayBuffer, GL_ARRAY_BUFFER); return *this; } template VertexArrayObject & - addAttribs(const GLuint arrayBuffer) + addAttribs(const GLuint arrayBuffer, const GLuint divisor = 0) { - configure_attribs(arrayBuffer); + configure_attribs(arrayBuffer, divisor); return *this; } @@ -73,34 +73,35 @@ public: private: template static auto - set_pointer(const GLuint vertexArrayId, const void * ptr) + set_pointer(const GLuint vertexArrayId, const void * ptr, const GLuint divisor) { using traits = gl_traits; const auto usedAttribs = traits::vertexAttribFunc(vertexArrayId, traits::size, traits::type, sizeof(VertexT), ptr); for (GLuint i {}; i < usedAttribs; i++) { glEnableVertexAttribArray(vertexArrayId + i); + glVertexAttribDivisor(vertexArrayId + i, divisor); } return usedAttribs; } template static auto - set_pointer(const GLuint vertexArrayId) + set_pointer(const GLuint vertexArrayId, const GLuint divisor) { - return set_pointer(vertexArrayId, attrib); + return set_pointer(vertexArrayId, attrib, divisor); } template void - configure_attribs(const GLuint arrayBuffer) + configure_attribs(const GLuint arrayBuffer, const GLuint divisor) { glBindBuffer(GL_ARRAY_BUFFER, arrayBuffer); if constexpr (sizeof...(attribs) == 0) { - vertexArrayId += set_pointer(vertexArrayId, nullptr); + vertexArrayId += set_pointer(vertexArrayId, nullptr, divisor); } else { - ((vertexArrayId += set_pointer(vertexArrayId)), ...); + ((vertexArrayId += set_pointer(vertexArrayId, divisor)), ...); } } -- cgit v1.2.3 From e9dd8fafcb4ea073fce9910f7cb139afc52e7c9a Mon Sep 17 00:00:00 2001 From: Dan Goodliffe Date: Thu, 20 Apr 2023 20:11:05 +0100 Subject: Expose bufferName and count from InstanceVertices --- gfx/gl/instanceVertices.h | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/gfx/gl/instanceVertices.h b/gfx/gl/instanceVertices.h index cf3b75b..dc28a4d 100644 --- a/gfx/gl/instanceVertices.h +++ b/gfx/gl/instanceVertices.h @@ -115,6 +115,18 @@ public: unused.push_back(p.index); } + const auto & + bufferName() const + { + return buffer; + } + + auto + count() const + { + return next; + } + protected: friend InstanceProxy; -- cgit v1.2.3 From ac05fbbc71282b059164b51efd68ee6e372870cb Mon Sep 17 00:00:00 2001 From: Dan Goodliffe Date: Thu, 20 Apr 2023 20:27:43 +0100 Subject: Switch to render trees in bulk through foliage asset rendering --- game/scenary/foliage.cpp | 22 ++++++++++++++-------- game/scenary/foliage.h | 10 +++++++--- game/scenary/plant.cpp | 12 +++--------- game/scenary/plant.h | 12 +++++------- test/test-assetFactory.cpp | 2 +- ui/gameMainWindow.cpp | 10 ++++++++++ 6 files changed, 40 insertions(+), 28 deletions(-) diff --git a/game/scenary/foliage.cpp b/game/scenary/foliage.cpp index d39d500..35be051 100644 --- a/game/scenary/foliage.cpp +++ b/game/scenary/foliage.cpp @@ -1,7 +1,9 @@ #include "foliage.h" #include "gfx/gl/sceneShader.h" #include "gfx/gl/shadowMapper.h" +#include "gfx/gl/vertexArrayObject.hpp" #include "gfx/models/texture.h" +#include "location.hpp" bool Foliage::persist(Persistence::PersistenceStore & store) @@ -13,21 +15,25 @@ void Foliage::postLoad() { texture = getTexture(); + bodyMesh->configureVAO(instanceVAO).addAttribs(instances.bufferName(), 1); } void -Foliage::render(const SceneShader & shader, const Location & loc) const +Foliage::render(const SceneShader & shader) const { - shader.basic.use(loc); - if (texture) { - texture->bind(); + if (const auto count = instances.count()) { + shader.basicInst.use(); + if (texture) { + texture->bind(); + } + glBindVertexArray(instanceVAO); + glDrawElementsInstanced( + bodyMesh->type(), bodyMesh->count(), GL_UNSIGNED_INT, nullptr, static_cast(count)); + glBindVertexArray(0); } - bodyMesh->Draw(); } void -Foliage::shadows(const ShadowMapper & mapper, const Location & loc) const +Foliage::shadows(const ShadowMapper &) const { - mapper.dynamicPoint.use(loc); - bodyMesh->Draw(); } diff --git a/game/scenary/foliage.h b/game/scenary/foliage.h index b85aab2..b72a9c2 100644 --- a/game/scenary/foliage.h +++ b/game/scenary/foliage.h @@ -1,19 +1,23 @@ #pragma once #include "assetFactory/asset.h" +#include "gfx/gl/instanceVertices.h" +#include "gfx/renderable.h" class SceneShader; class ShadowMapper; class Location; class Texture; -class Foliage : public Asset, public StdTypeDefs { +class Foliage : public Asset, public Renderable, public StdTypeDefs { Mesh::Ptr bodyMesh; std::shared_ptr texture; + glVertexArray instanceVAO; public: - void render(const SceneShader &, const Location &) const; - void shadows(const ShadowMapper &, const Location &) const; + mutable InstanceVertices instances; + void render(const SceneShader &) const override; + void shadows(const ShadowMapper &) const override; protected: friend Persistence::SelectionPtrBase>; diff --git a/game/scenary/plant.cpp b/game/scenary/plant.cpp index 2b01bee..678d4a7 100644 --- a/game/scenary/plant.cpp +++ b/game/scenary/plant.cpp @@ -1,13 +1,7 @@ #include "plant.h" -void -Plant::render(const SceneShader & shader) const +Plant::Plant(std::shared_ptr type, Location position) : + type {std::move(type)}, + location {this->type->instances.acquire(glm::translate(position.pos) * rotate_ypr(position.rot))} { - type->render(shader, position); -} - -void -Plant::shadows(const ShadowMapper & mapper) const -{ - type->shadows(mapper, position); } diff --git a/game/scenary/plant.h b/game/scenary/plant.h index 55acca1..77c5979 100644 --- a/game/scenary/plant.h +++ b/game/scenary/plant.h @@ -2,15 +2,13 @@ #include "foliage.h" #include "game/worldobject.h" -#include "gfx/renderable.h" #include "location.hpp" +#include "maths.h" +#include -class Plant : public Renderable, public WorldObject { +class Plant : public WorldObject { std::shared_ptr type; - Location position; - - void render(const SceneShader & shader) const override; - void shadows(const ShadowMapper & shadowMapper) const override; + InstanceVertices::InstanceProxy location; void tick(TickDuration) override @@ -18,5 +16,5 @@ class Plant : public Renderable, public WorldObject { } public: - Plant(std::shared_ptr type, Location position) : type(std::move(type)), position(position) { } + Plant(std::shared_ptr type, Location position); }; diff --git a/test/test-assetFactory.cpp b/test/test-assetFactory.cpp index 82543f3..817654b 100644 --- a/test/test-assetFactory.cpp +++ b/test/test-assetFactory.cpp @@ -108,7 +108,7 @@ BOOST_AUTO_TEST_CASE(foliage, *boost::unit_test::timeout(5)) BOOST_REQUIRE(tree_01_1_f); auto plant = std::make_shared(tree_01_1_f, Location {{-2, 2, 0}, {}}); - objects.objects.push_back(plant); + objects.objects.push_back(tree_01_1_f); render(5); } diff --git a/ui/gameMainWindow.cpp b/ui/gameMainWindow.cpp index 0b30cad..3f85a4f 100644 --- a/ui/gameMainWindow.cpp +++ b/ui/gameMainWindow.cpp @@ -51,6 +51,11 @@ GameMainWindow::render() const void GameMainWindow::content(const SceneShader & shader) const { + for (const auto & [id, asset] : gameState->assets) { + if (const auto r = std::dynamic_pointer_cast(asset)) { + r->render(shader); + } + } gameState->world.apply(&Renderable::render, shader); uiComponents.apply(&WorldOverlay::render, shader); } @@ -68,5 +73,10 @@ GameMainWindow::lights(const SceneShader & shader) const void GameMainWindow::shadows(const ShadowMapper & shadowMapper) const { + for (const auto & [id, asset] : gameState->assets) { + if (const auto r = std::dynamic_pointer_cast(asset)) { + r->shadows(shadowMapper); + } + } gameState->world.apply(&Renderable::shadows, shadowMapper); } -- cgit v1.2.3 From ab5b46dbe58ae0401c24ead55a2627b9a577fc1a Mon Sep 17 00:00:00 2001 From: Dan Goodliffe Date: Fri, 21 Apr 2023 23:52:56 +0100 Subject: Instances buffer data needs to be unmapped before use Here we change quite a bit to support mapping and unmapping the buffer as required. The instance/free referencing is still broken though. We add several instances of tree in the render. --- gfx/gl/instanceVertices.h | 66 ++++++++++++++++++++++++++++++++-------------- test/Jamfile.jam | 2 +- test/test-assetFactory.cpp | 5 +++- test/test-instancing.cpp | 15 +++++++---- 4 files changed, 61 insertions(+), 27 deletions(-) diff --git a/gfx/gl/instanceVertices.h b/gfx/gl/instanceVertices.h index dc28a4d..c2ddb1d 100644 --- a/gfx/gl/instanceVertices.h +++ b/gfx/gl/instanceVertices.h @@ -9,11 +9,9 @@ template class InstanceVertices { public: - InstanceVertices(size_t initialSize = 16) : data {allocBuffer(buffer, initialSize)}, next {} { } - - ~InstanceVertices() + InstanceVertices(size_t initialSize = 16) { - glUnmapNamedBuffer(buffer); + allocBuffer(initialSize); } class InstanceProxy { @@ -50,17 +48,20 @@ public: T & operator=(U && v) { + instances->map(); return instances->data[index] = std::forward(v); } T * get() { + instances->map(); return &instances->data[index]; } const T * get() const { + instances->map(); return &instances->data[index]; } T * @@ -95,22 +96,24 @@ public: InstanceProxy acquire(Params &&... params) { + map(); if (!unused.empty()) { auto idx = unused.back(); unused.pop_back(); - new (&data[idx]) T(std::forward(params)...); + new (data + idx) T(std::forward(params)...); return InstanceProxy {this, idx}; } - if (next >= data.size()) { - resize(data.size() * 2); + if (next >= capacity) { + resize(capacity * 2); } - new (&data[next]) T(std::forward(params)...); + new (data + next) T(std::forward(params)...); return InstanceProxy {this, next++}; } void release(const InstanceProxy & p) { + map(); data[p.index].~T(); unused.push_back(p.index); } @@ -118,42 +121,65 @@ public: const auto & bufferName() const { + unmap(); return buffer; } auto count() const { + unmap(); return next; } protected: friend InstanceProxy; - static std::span - allocBuffer(const glBuffer & buffer, std::size_t count) + void + allocBuffer(std::size_t newCapacity) { glBindBuffer(GL_ARRAY_BUFFER, buffer); - glBufferData(GL_ARRAY_BUFFER, static_cast(sizeof(T) * count), nullptr, GL_DYNAMIC_DRAW); - auto data = static_cast(glMapNamedBuffer(buffer, GL_READ_WRITE)); + glBufferData(GL_ARRAY_BUFFER, static_cast(sizeof(T) * newCapacity), nullptr, GL_DYNAMIC_DRAW); glBindBuffer(GL_ARRAY_BUFFER, 0); - return {data, count}; + capacity = newCapacity; + data = nullptr; } void resize(size_t newCapacity) { - const auto maintain = std::min(newCapacity, data.size()); - const auto maintaind = static_cast(maintain); + const auto maintain = std::min(newCapacity, capacity); std::vector existing; + const auto maintaind = static_cast(maintain); existing.reserve(maintain); - std::move(data.begin(), data.begin() + maintaind, std::back_inserter(existing)); - data = allocBuffer(buffer, newCapacity); - std::move(existing.begin(), existing.begin() + maintaind, data.begin()); + map(); + std::move(data, data + maintain, std::back_inserter(existing)); + allocBuffer(newCapacity); + map(); + std::move(existing.begin(), existing.begin() + maintaind, data); + capacity = newCapacity; + } + + void + map() const + { + if (!data) { + data = static_cast(glMapNamedBuffer(buffer, GL_READ_WRITE)); + } + } + + void + unmap() const + { + if (data) { + glUnmapNamedBuffer(buffer); + data = nullptr; + } } glBuffer buffer; - std::span data; - std::size_t next; + mutable T * data {}; + std::size_t capacity {}; + std::size_t next {}; std::vector unused; }; diff --git a/test/Jamfile.jam b/test/Jamfile.jam index 9cdcef4..1ce73b7 100644 --- a/test/Jamfile.jam +++ b/test/Jamfile.jam @@ -53,7 +53,7 @@ run test-text.cpp ; run test-enumDetails.cpp ; run test-render.cpp : -- : test-assetFactory : test ; run test-glContextBhvr.cpp ; -run test-assetFactory.cpp : -- : [ sequence.insertion-sort [ glob-tree $(res) : *.* ] fixtures/rgb.txt ] : test ; +run test-assetFactory.cpp : -- : [ sequence.insertion-sort [ glob-tree $(res) : *.* ] fixtures/rgb.txt test-instancing ] : test ; run perf-assetFactory.cpp : -- : test-assetFactory : benchmark test ; run perf-persistence.cpp : -- : test-persistence : benchmark test ; run test-worker.cpp ; diff --git a/test/test-assetFactory.cpp b/test/test-assetFactory.cpp index 817654b..82a1825 100644 --- a/test/test-assetFactory.cpp +++ b/test/test-assetFactory.cpp @@ -107,7 +107,10 @@ BOOST_AUTO_TEST_CASE(foliage, *boost::unit_test::timeout(5)) auto tree_01_1_f = std::dynamic_pointer_cast(tree_01_1); BOOST_REQUIRE(tree_01_1_f); - auto plant = std::make_shared(tree_01_1_f, Location {{-2, 2, 0}, {}}); + auto plant1 = std::make_shared(tree_01_1_f, Location {{-2, 2, 0}, {0, 0, 0}}); + auto plant2 = std::make_shared(tree_01_1_f, Location {{3, -4, 0}, {0, 1, 0}}); + auto plant3 = std::make_shared(tree_01_1_f, Location {{-2, -4, 0}, {0, 2, 0}}); + auto plant4 = std::make_shared(tree_01_1_f, Location {{3, 2, 0}, {0, 3, 0}}); objects.objects.push_back(tree_01_1_f); render(5); diff --git a/test/test-instancing.cpp b/test/test-instancing.cpp index 7191567..2eb3647 100644 --- a/test/test-instancing.cpp +++ b/test/test-instancing.cpp @@ -15,16 +15,21 @@ BOOST_FIXTURE_TEST_SUITE(i, InstanceVertices) BOOST_AUTO_TEST_CASE(createDestroy) { - BOOST_REQUIRE(data.data()); + BOOST_CHECK(!data); + map(); + BOOST_REQUIRE(data); BOOST_CHECK_EQUAL(0, next); BOOST_CHECK(unused.empty()); + unmap(); + BOOST_CHECK(!data); } BOOST_AUTO_TEST_CASE(storeRetreive) { // Read write raw buffer, not normally allowed - std::vector test(data.size()); - std::copy(test.begin(), test.end(), data.begin()); - BOOST_CHECK_EQUAL_COLLECTIONS(test.begin(), test.end(), data.begin(), data.end()); + std::vector test(capacity); + map(); + std::copy(test.begin(), test.end(), data); + BOOST_CHECK_EQUAL_COLLECTIONS(test.begin(), test.end(), data, data + capacity); } BOOST_AUTO_TEST_CASE(acquireRelease) @@ -69,7 +74,7 @@ BOOST_AUTO_TEST_CASE(resize) proxies.push_back(acquire(n)); expected.emplace_back(n); } - BOOST_CHECK_EQUAL_COLLECTIONS(expected.begin(), expected.end(), data.begin(), data.begin() + COUNT); + BOOST_CHECK_EQUAL_COLLECTIONS(expected.begin(), expected.end(), data, data + COUNT); BOOST_CHECK_EQUAL_COLLECTIONS(expected.begin(), expected.end(), proxies.begin(), proxies.end()); } -- cgit v1.2.3 From b8889129c6c2b747ff771b711cfbbf91bca93bb6 Mon Sep 17 00:00:00 2001 From: Dan Goodliffe Date: Sat, 22 Apr 2023 11:44:38 +0100 Subject: Fix the instancing maintenance --- gfx/gl/instanceVertices.h | 83 ++++++++++++++++++++++++------------ test/test-instancing.cpp | 105 +++++++++++++++++++++++++++++++++++++++++----- 2 files changed, 152 insertions(+), 36 deletions(-) diff --git a/gfx/gl/instanceVertices.h b/gfx/gl/instanceVertices.h index c2ddb1d..036b48e 100644 --- a/gfx/gl/instanceVertices.h +++ b/gfx/gl/instanceVertices.h @@ -32,37 +32,38 @@ public: InstanceProxy & operator=(InstanceProxy && other) { + if (instances) { + instances->release(*this); + } instances = std::exchange(other.instances, nullptr); index = other.index; + return *this; + } + template + T & + operator=(U && v) + { + return instances->at(index) = std::forward(v); } operator T &() { - return *get(); + return instances->at(index); } operator const T &() const { - return *get(); - } - template - T & - operator=(U && v) - { - instances->map(); - return instances->data[index] = std::forward(v); + return instances->at(index); } T * get() { - instances->map(); - return &instances->data[index]; + return &instances->at(index); } const T * get() const { - instances->map(); - return &instances->data[index]; + return &instances->at(index); } T * operator->() @@ -77,12 +78,12 @@ public: T & operator*() { - return *get(); + return instances->at(index); } const T & operator*() const { - return *get(); + return instances->at(index); } private: @@ -100,22 +101,16 @@ public: if (!unused.empty()) { auto idx = unused.back(); unused.pop_back(); - new (data + idx) T(std::forward(params)...); + index[idx] = next++; + new (&at(idx)) T(std::forward(params)...); return InstanceProxy {this, idx}; } if (next >= capacity) { resize(capacity * 2); } - new (data + next) T(std::forward(params)...); - return InstanceProxy {this, next++}; - } - - void - release(const InstanceProxy & p) - { - map(); - data[p.index].~T(); - unused.push_back(p.index); + index.emplace_back(next++); + new (data + index.back()) T(std::forward(params)...); + return InstanceProxy {this, index.size() - 1}; } const auto & @@ -135,6 +130,30 @@ public: protected: friend InstanceProxy; + void + release(const InstanceProxy & p) + { + // Destroy p's object + at(p.index).~T(); + if (next-- > index[p.index]) { + // Remember p.index is free index now + unused.push_back(p.index); + // Move last object into p's slot + new (&at(p.index)) T {std::move(data[next])}; + (data[next]).~T(); + for (auto & i : index) { + if (i == next) { + i = index[p.index]; + break; + } + } + index[p.index] = 100000; + } + else { + index.pop_back(); + } + } + void allocBuffer(std::size_t newCapacity) { @@ -160,6 +179,13 @@ protected: capacity = newCapacity; } + T & + at(size_t pindex) + { + map(); + return data[index[pindex]]; + } + void map() const { @@ -179,7 +205,12 @@ protected: glBuffer buffer; mutable T * data {}; + // Size of buffer std::size_t capacity {}; + // # used of capacity std::size_t next {}; + // Index into buffer given to nth proxy + std::vector index; + // List of free spaces in index std::vector unused; }; diff --git a/test/test-instancing.cpp b/test/test-instancing.cpp index 2eb3647..8837071 100644 --- a/test/test-instancing.cpp +++ b/test/test-instancing.cpp @@ -1,10 +1,12 @@ #define BOOST_TEST_MODULE instancing +#include "stream_support.hpp" #include "testHelpers.h" #include "testMainWindow.h" #include "ui/applicationBase.h" #include #include +#include #include @@ -20,28 +22,25 @@ BOOST_AUTO_TEST_CASE(createDestroy) BOOST_REQUIRE(data); BOOST_CHECK_EQUAL(0, next); BOOST_CHECK(unused.empty()); + BOOST_CHECK(index.empty()); unmap(); BOOST_CHECK(!data); } -BOOST_AUTO_TEST_CASE(storeRetreive) -{ // Read write raw buffer, not normally allowed - std::vector test(capacity); - map(); - std::copy(test.begin(), test.end(), data); - BOOST_CHECK_EQUAL_COLLECTIONS(test.begin(), test.end(), data, data + capacity); -} - BOOST_AUTO_TEST_CASE(acquireRelease) { { auto proxy = acquire(); *proxy = 20; BOOST_CHECK_EQUAL(1, next); + BOOST_REQUIRE_EQUAL(1, index.size()); + BOOST_CHECK_EQUAL(0, index.front()); + BOOST_CHECK(unused.empty()); } - BOOST_CHECK_EQUAL(1, next); + BOOST_CHECK_EQUAL(0, next); BOOST_CHECK_EQUAL(1, unused.size()); BOOST_CHECK_EQUAL(0, unused.front()); + BOOST_CHECK_EQUAL(1, index.size()); } BOOST_AUTO_TEST_CASE(acquireReleaseMove) @@ -54,9 +53,10 @@ BOOST_AUTO_TEST_CASE(acquireReleaseMove) *proxy2 = 40; BOOST_CHECK_EQUAL(1, next); } - BOOST_CHECK_EQUAL(1, next); + BOOST_CHECK_EQUAL(0, next); BOOST_CHECK_EQUAL(1, unused.size()); BOOST_CHECK_EQUAL(0, unused.front()); + BOOST_CHECK_EQUAL(1, index.size()); } BOOST_AUTO_TEST_CASE(initialize) @@ -78,4 +78,89 @@ BOOST_AUTO_TEST_CASE(resize) BOOST_CHECK_EQUAL_COLLECTIONS(expected.begin(), expected.end(), proxies.begin(), proxies.end()); } +BOOST_AUTO_TEST_CASE(shuffle) +{ + std::vector proxies; + BOOST_CHECK_EQUAL(0, proxies.emplace_back(acquire(0))); + BOOST_CHECK_EQUAL(1, proxies.emplace_back(acquire(1))); + BOOST_CHECK_EQUAL(2, proxies.emplace_back(acquire(2))); + BOOST_CHECK_EQUAL(3, proxies.emplace_back(acquire(3))); + BOOST_CHECK_EQUAL(4, next); + BOOST_CHECK_EQUAL(data + 0, proxies[0].get()); + BOOST_CHECK_EQUAL(data + 1, proxies[1].get()); + BOOST_CHECK_EQUAL(data + 2, proxies[2].get()); + BOOST_CHECK_EQUAL(data + 3, proxies[3].get()); + BOOST_CHECK(unused.empty()); + BOOST_REQUIRE_EQUAL(4, index.size()); + BOOST_CHECK_EQUAL(0, index[0]); + BOOST_CHECK_EQUAL(1, index[1]); + BOOST_CHECK_EQUAL(2, index[2]); + BOOST_CHECK_EQUAL(3, index[3]); + // Remove 1, 3 moves to [1] + proxies.erase(proxies.begin() + 1); + BOOST_REQUIRE_EQUAL(4, index.size()); + BOOST_REQUIRE_EQUAL(1, unused.size()); + BOOST_CHECK_EQUAL(1, unused[0]); + BOOST_CHECK_EQUAL(data + 0, proxies[0].get()); + BOOST_CHECK_EQUAL(data + 2, proxies[1].get()); + BOOST_CHECK_EQUAL(data + 1, proxies[2].get()); + // Remove 1, 2 moves to [1] + proxies.erase(proxies.begin() + 1); + BOOST_REQUIRE_EQUAL(4, index.size()); + BOOST_REQUIRE_EQUAL(2, unused.size()); + BOOST_CHECK_EQUAL(1, unused[0]); + BOOST_CHECK_EQUAL(2, unused[1]); + BOOST_CHECK_EQUAL(data + 0, proxies[0].get()); + BOOST_CHECK_EQUAL(data + 1, proxies[1].get()); + // Add new, takes 2 at [2] + BOOST_CHECK_EQUAL(4, proxies.emplace_back(acquire(4))); + BOOST_REQUIRE_EQUAL(4, index.size()); + BOOST_REQUIRE_EQUAL(1, unused.size()); + BOOST_CHECK_EQUAL(1, unused[0]); + BOOST_CHECK_EQUAL(data + 0, proxies[0].get()); + BOOST_CHECK_EQUAL(data + 1, proxies[1].get()); + BOOST_CHECK_EQUAL(data + 2, proxies[2].get()); +} + +BOOST_DATA_TEST_CASE(shuffle_random, boost::unit_test::data::xrange(0, 10), x) +{ + std::ignore = x; + std::mt19937 gen(std::random_device {}()); + std::map::InstanceProxy> proxies; + const std::string_view actions = "aaaaaaaarararrraarrrararararaarrrarararararararararraarrrraaaarararaararar"; + int n {}; + for (const auto action : actions) { + switch (action) { + case 'a': + BOOST_REQUIRE_EQUAL(n, proxies.emplace(n, acquire(n)).first->second); + n++; + break; + case 'r': + BOOST_REQUIRE(!proxies.empty()); + auto e = std::next(proxies.begin(), + std::uniform_int_distribution<> {0, static_cast(proxies.size() - 1)}(gen)); + proxies.erase(e); + break; + } + + BOOST_REQUIRE_EQUAL(next, proxies.size()); + for (const auto & [n, p] : proxies) { + BOOST_REQUIRE_EQUAL(n, p); + } + std::set iused; + for (size_t i {}; i < index.size(); i++) { + if (std::find(unused.begin(), unused.end(), i) == unused.end()) { + iused.emplace(index[i]); + } + } + BOOST_TEST_CONTEXT(index) { + BOOST_REQUIRE_EQUAL(iused.size(), next); + if (!iused.empty()) { + BOOST_REQUIRE_EQUAL(*iused.begin(), 0); + BOOST_REQUIRE_EQUAL(*iused.rbegin(), next - 1); + } + } + } +} + BOOST_AUTO_TEST_SUITE_END() -- cgit v1.2.3 From c26d3ecfe9779e9cce13edfd981246c146c6f3a1 Mon Sep 17 00:00:00 2001 From: Dan Goodliffe Date: Sat, 22 Apr 2023 12:14:26 +0100 Subject: Streamline the instancing maintenance --- gfx/gl/instanceVertices.h | 26 ++++++++++---------------- 1 file changed, 10 insertions(+), 16 deletions(-) diff --git a/gfx/gl/instanceVertices.h b/gfx/gl/instanceVertices.h index 036b48e..cf4f0ae 100644 --- a/gfx/gl/instanceVertices.h +++ b/gfx/gl/instanceVertices.h @@ -25,7 +25,7 @@ public: ~InstanceProxy() { if (instances) { - instances->release(*this); + instances->release(index); } } @@ -33,7 +33,7 @@ public: operator=(InstanceProxy && other) { if (instances) { - instances->release(*this); + instances->release(index); } instances = std::exchange(other.instances, nullptr); index = other.index; @@ -87,8 +87,6 @@ public: } private: - friend InstanceVertices; - InstanceVertices * instances; std::size_t index; }; @@ -131,23 +129,19 @@ protected: friend InstanceProxy; void - release(const InstanceProxy & p) + release(const size_t pidx) { // Destroy p's object - at(p.index).~T(); - if (next-- > index[p.index]) { + at(pidx).~T(); + if (next-- > index[pidx]) { // Remember p.index is free index now - unused.push_back(p.index); + unused.push_back(pidx); // Move last object into p's slot - new (&at(p.index)) T {std::move(data[next])}; + new (&at(pidx)) T {std::move(data[next])}; (data[next]).~T(); - for (auto & i : index) { - if (i == next) { - i = index[p.index]; - break; - } - } - index[p.index] = 100000; + *std::find(index.begin(), index.end(), next) = index[pidx]; + // Not strictly required, but needed for uniqueness unit test assertion + index[pidx] = static_cast(-1); } else { index.pop_back(); -- cgit v1.2.3 From bcd0e98739974248e6d29f72e8b281e21fd59657 Mon Sep 17 00:00:00 2001 From: Dan Goodliffe Date: Sat, 22 Apr 2023 13:17:05 +0100 Subject: Test instancing automatic unmap when count is called, add some nodiscard --- gfx/gl/instanceVertices.h | 27 +++++++++++++-------------- test/test-instancing.cpp | 19 ++++++++++++++++++- 2 files changed, 31 insertions(+), 15 deletions(-) diff --git a/gfx/gl/instanceVertices.h b/gfx/gl/instanceVertices.h index cf4f0ae..edcaef4 100644 --- a/gfx/gl/instanceVertices.h +++ b/gfx/gl/instanceVertices.h @@ -14,7 +14,7 @@ public: allocBuffer(initialSize); } - class InstanceProxy { + class [[nodiscard]] InstanceProxy { public: InstanceProxy(InstanceVertices * iv, std::size_t idx) : instances {iv}, index {idx} { } InstanceProxy(InstanceProxy && other) : instances {std::exchange(other.instances, nullptr)}, index {other.index} @@ -46,41 +46,41 @@ public: return instances->at(index) = std::forward(v); } + [[nodiscard]] operator T &() { return instances->at(index); } - operator const T &() const + [[nodiscard]] operator const T &() const { return instances->at(index); } - - T * + [[nodiscard]] T * get() { return &instances->at(index); } - const T * + [[nodiscard]] const T * get() const { return &instances->at(index); } - T * + [[nodiscard]] T * operator->() { return get(); } - const T * + [[nodiscard]] const T * operator->() const { return get(); } - T & + [[nodiscard]] T & operator*() { return instances->at(index); } - const T & + [[nodiscard]] const T & operator*() const { return instances->at(index); @@ -92,7 +92,7 @@ public: }; template - InstanceProxy + [[nodiscard]] InstanceProxy acquire(Params &&... params) { map(); @@ -111,14 +111,13 @@ public: return InstanceProxy {this, index.size() - 1}; } - const auto & + [[nodiscard]] const auto & bufferName() const { - unmap(); return buffer; } - auto + [[nodiscard]] auto count() const { unmap(); @@ -173,7 +172,7 @@ protected: capacity = newCapacity; } - T & + [[nodiscard]] T & at(size_t pindex) { map(); diff --git a/test/test-instancing.cpp b/test/test-instancing.cpp index 8837071..70b20fa 100644 --- a/test/test-instancing.cpp +++ b/test/test-instancing.cpp @@ -50,8 +50,9 @@ BOOST_AUTO_TEST_CASE(acquireReleaseMove) *proxy1 = 20; BOOST_CHECK_EQUAL(1, next); auto proxy2 = std::move(proxy1); - *proxy2 = 40; + proxy2 = 40; BOOST_CHECK_EQUAL(1, next); + BOOST_CHECK_EQUAL(data[0], 40); } BOOST_CHECK_EQUAL(0, next); BOOST_CHECK_EQUAL(1, unused.size()); @@ -59,10 +60,26 @@ BOOST_AUTO_TEST_CASE(acquireReleaseMove) BOOST_CHECK_EQUAL(1, index.size()); } +BOOST_AUTO_TEST_CASE(autoMapUnmap) +{ + { + auto proxy = acquire(); + BOOST_CHECK(data); + std::ignore = bufferName(); + BOOST_CHECK(data); + BOOST_CHECK_EQUAL(1, count()); + BOOST_CHECK(!data); + } + BOOST_CHECK_EQUAL(0, count()); +} + BOOST_AUTO_TEST_CASE(initialize) { auto proxy = acquire(5); + const auto & constProxy = proxy; BOOST_CHECK_EQUAL(*proxy, 5); + BOOST_CHECK_EQUAL(*constProxy, 5); + BOOST_CHECK_EQUAL(constProxy.get(), constProxy.get()); } BOOST_AUTO_TEST_CASE(resize) -- cgit v1.2.3 From 59d85f9b87fc82b52f2c7438e38768aeddc3cc87 Mon Sep 17 00:00:00 2001 From: Dan Goodliffe Date: Sat, 22 Apr 2023 14:56:08 +0100 Subject: Don't fill the instances unused vector unnecessarily --- gfx/gl/instanceVertices.h | 17 ++++++++++------- test/test-instancing.cpp | 10 ++++------ 2 files changed, 14 insertions(+), 13 deletions(-) diff --git a/gfx/gl/instanceVertices.h b/gfx/gl/instanceVertices.h index edcaef4..51bf4bc 100644 --- a/gfx/gl/instanceVertices.h +++ b/gfx/gl/instanceVertices.h @@ -132,19 +132,22 @@ protected: { // Destroy p's object at(pidx).~T(); - if (next-- > index[pidx]) { - // Remember p.index is free index now - unused.push_back(pidx); + if (--next != index[pidx]) { // Move last object into p's slot new (&at(pidx)) T {std::move(data[next])}; (data[next]).~T(); - *std::find(index.begin(), index.end(), next) = index[pidx]; - // Not strictly required, but needed for uniqueness unit test assertion - index[pidx] = static_cast(-1); + *std::find_if(index.begin(), index.end(), [this](const auto & i) { + const auto n = &i - index.data(); + return i == next && std::find(unused.begin(), unused.end(), n) == unused.end(); + }) = index[pidx]; } - else { + if (pidx == index.size() - 1) { index.pop_back(); } + else { + // Remember p.index is free index now + unused.push_back(pidx); + } } void diff --git a/test/test-instancing.cpp b/test/test-instancing.cpp index 70b20fa..c743ce0 100644 --- a/test/test-instancing.cpp +++ b/test/test-instancing.cpp @@ -38,9 +38,8 @@ BOOST_AUTO_TEST_CASE(acquireRelease) BOOST_CHECK(unused.empty()); } BOOST_CHECK_EQUAL(0, next); - BOOST_CHECK_EQUAL(1, unused.size()); - BOOST_CHECK_EQUAL(0, unused.front()); - BOOST_CHECK_EQUAL(1, index.size()); + BOOST_CHECK(unused.empty()); + BOOST_CHECK(index.empty()); } BOOST_AUTO_TEST_CASE(acquireReleaseMove) @@ -55,9 +54,8 @@ BOOST_AUTO_TEST_CASE(acquireReleaseMove) BOOST_CHECK_EQUAL(data[0], 40); } BOOST_CHECK_EQUAL(0, next); - BOOST_CHECK_EQUAL(1, unused.size()); - BOOST_CHECK_EQUAL(0, unused.front()); - BOOST_CHECK_EQUAL(1, index.size()); + BOOST_CHECK(unused.empty()); + BOOST_CHECK(index.empty()); } BOOST_AUTO_TEST_CASE(autoMapUnmap) -- cgit v1.2.3 From 6737c8b4c5f804e23be38212295e789ff534822b Mon Sep 17 00:00:00 2001 From: Dan Goodliffe Date: Sat, 22 Apr 2023 15:19:40 +0100 Subject: Keep the instance unused vector sorted and binary search it --- gfx/gl/instanceVertices.h | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/gfx/gl/instanceVertices.h b/gfx/gl/instanceVertices.h index 51bf4bc..228020d 100644 --- a/gfx/gl/instanceVertices.h +++ b/gfx/gl/instanceVertices.h @@ -137,16 +137,15 @@ protected: new (&at(pidx)) T {std::move(data[next])}; (data[next]).~T(); *std::find_if(index.begin(), index.end(), [this](const auto & i) { - const auto n = &i - index.data(); - return i == next && std::find(unused.begin(), unused.end(), n) == unused.end(); + return i == next && !std::binary_search(unused.begin(), unused.end(), &i - index.data()); }) = index[pidx]; } if (pidx == index.size() - 1) { index.pop_back(); } else { - // Remember p.index is free index now - unused.push_back(pidx); + // Remember p.index is free index now, keeping it sorted + unused.insert(std::upper_bound(unused.begin(), unused.end(), pidx), pidx); } } -- cgit v1.2.3