Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bind empty VAO to bufferless quad rendering #1734

Merged
merged 1 commit into from
Jan 2, 2025
+22 −5
Merged

Bind empty VAO to bufferless quad rendering #1734

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion libopenage/renderer/opengl/geometry.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright 2015-2024 the openage authors. See copying.md for legal info.
// Copyright 2015-2025 the openage authors. See copying.md for legal info.

#include "geometry.h"

Expand Down Expand Up @@ -54,6 +54,7 @@ void GlGeometry::update_verts_offset(std::vector<uint8_t> const &verts, size_t o
void GlGeometry::draw() const {
switch (this->get_type()) {
case geometry_t::bufferless_quad:
// any VAO must be bound before this call
glDrawArrays(GL_TRIANGLE_STRIP, 0, 4);
break;

Expand Down
13 changes: 10 additions & 3 deletions libopenage/renderer/opengl/renderer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright 2017-2024 the openage authors. See copying.md for legal info.
// Copyright 2017-2025 the openage authors. See copying.md for legal info.

#include "renderer.h"

Expand All @@ -15,6 +15,7 @@
#include "renderer/opengl/texture.h"
#include "renderer/opengl/uniform_buffer.h"
#include "renderer/opengl/uniform_input.h"
#include "renderer/opengl/vertex_array.h"
#include "renderer/opengl/window.h"
#include "renderer/resources/buffer_info.h"

Expand All @@ -26,7 +27,8 @@ GlRenderer::GlRenderer(const std::shared_ptr<GlContext> &ctx,
gl_context{ctx},
display{std::make_shared<GlRenderTarget>(ctx,
viewport_size[0],
viewport_size[1])} {
viewport_size[1])},
shared_quad_vao{std::make_shared<GlVertexArray>(ctx)} {
// color used to clear the color buffers
glClearColor(0.0f, 0.0f, 0.0f, 0.0f);

Expand Down Expand Up @@ -100,7 +102,7 @@ std::shared_ptr<UniformBuffer> GlRenderer::add_uniform_buffer(resources::Uniform
resources::UniformBufferInfo::get_size(input, info.get_layout()),
resources::UniformBufferInfo::get_stride_size(input.type, info.get_layout()),
input.count,
input.name});
input.name});

offset += size;
}
Expand Down Expand Up @@ -169,6 +171,11 @@ void GlRenderer::render(const std::shared_ptr<RenderPass> &pass) {
auto gl_target = std::dynamic_pointer_cast<GlRenderTarget>(pass->get_target());
gl_target->bind_write();

// ensure that an (empty) VAO is bound before rendering geometry
// a bound VAO is required to render bufferless quad geometries by OpenGL
// see https://www.khronos.org/opengl/wiki/Vertex_Rendering#Causes_of_rendering_failure
shared_quad_vao->bind();

glClear(GL_COLOR_BUFFER_BIT | GL_DEPTH_BUFFER_BIT);

// TODO: Option for face culling
Expand Down
11 changes: 10 additions & 1 deletion libopenage/renderer/opengl/renderer.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright 2017-2024 the openage authors. See copying.md for legal info.
// Copyright 2017-2025 the openage authors. See copying.md for legal info.

#pragma once

Expand All @@ -17,6 +17,7 @@ namespace opengl {
class GlContext;
class GlRenderPass;
class GlRenderTarget;
class GlVertexArray;
class GlWindow;

/// The OpenGL specialization of the rendering interface.
Expand Down Expand Up @@ -67,6 +68,14 @@ class GlRenderer final : public Renderer {

/// The main screen surface as a render target.
std::shared_ptr<GlRenderTarget> display;

/// An empty vertex array object (VAO).
///
/// This VAO has to be bound at the start of a render pass to ensure
/// that bufferless quad geometry can be drawn without errors. Drawing a
/// bufferless quad requires any VAO to be bound
/// see https://www.khronos.org/opengl/wiki/Vertex_Rendering#Causes_of_rendering_failure
std::shared_ptr<GlVertexArray> shared_quad_vao;
};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can make this member a unique_ptr I think, since it's only used by the GlRenderer class.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I attempted to use unique_ptr, but encountered an issue since it requires the complete type definition. When I moved the include to the header file to replace forward declaration, it led to circular include dependencies. Did I do something wrong, or should I dive into the dependency cycle to fix it?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, you are right. I think you can change it back to shared_ptr.

I think the problem here is that unique_ptr only allows incomplete types under specific circumstances. I also didn't know the specifics for that :'D

std::unique_ptr may be constructed for an incomplete type T, such as to facilitate the use as a handle in the pImpl idiom. If the default deleter is used, T must be complete at the point in code where the deleter is invoked, which happens in the destructor, move assignment operator, and reset member function of std::unique_ptr. (In contrast, std::shared_ptr can't be constructed from a raw pointer to incomplete type, but can be destroyed where T is incomplete). Note that if T is a class template specialization, use of unique_ptr as an operand, e.g. !p requires T's parameters to be complete due to ADL.

https://en.cppreference.com/w/cpp/memory/unique_ptr

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I will change it back

ZzzhHe marked this conversation as resolved.
Show resolved Hide resolved

} // namespace opengl
Expand Down
Loading