Skip to content

Commit

Permalink
GL: mesh.draw(shader) -> shader.draw(mesh).
Browse files Browse the repository at this point in the history
As usual, the old APIs are still present, but marked as deprecated.
Existing code is not updated yet to ensure I didn't break anything with
this.

This way it's much more intuitive and makes the code shorter and nicer
in many cases. Shaders are now also able to hide irrelevant
draw/dispatch APIs to avoid accidents.
  • Loading branch information
mosra committed Mar 12, 2020
1 parent 0e2a5cc commit 32d49db
Show file tree
Hide file tree
Showing 18 changed files with 505 additions and 280 deletions.
23 changes: 14 additions & 9 deletions doc/changelog.dox
Original file line number Diff line number Diff line change
Expand Up @@ -227,10 +227,11 @@ See also:
@subsubsection changelog-latest-changes-gl GL library

- Added @ref GL::AbstractTexture::bind(),
@ref GL::AbstractTexture::bindImages() and @ref GL::MeshView::draw()
overloads taking a @ref Corrade::Containers::ArrayView instead of
@ref std::initializer_list in order to allow passing runtime-sized lists
(see also [mosra/magnum#403](https://github.com/mosra/magnum/pull/403))
@ref GL::AbstractTexture::bindImages() and
@ref GL::AbstractShaderProgram::draw() overloads taking a
@ref Corrade::Containers::ArrayView instead of @ref std::initializer_list
in order to allow passing runtime-sized lists (see also
[mosra/magnum#403](https://github.com/mosra/magnum/pull/403))
- Added an ability to remove a buffer from a @ref GL::BufferTexture using
@ref GL::BufferTexture::resetBuffer() "resetBuffer()"

Expand Down Expand Up @@ -392,6 +393,10 @@ See also:

@subsection changelog-latest-deprecated Deprecated APIs

- @cpp GL::Mesh::draw() @ce and @cpp GL::MeshView::draw() @ce are deprecated
in favor of @ref GL::AbstractShaderProgram::draw() and
@ref GL::AbstractShaderProgram::drawTransformFeedback(), as the API makes
much more sense to have on a shader and not on a mesh
- @cpp GL::Attribute::DataType::HalfFloat @ce,
@cpp GL::DynamicAttribute::DataType::HalfFloat @ce and
@cpp GL::PixelType::HalfFloat @ce is deprecated in favor of
Expand Down Expand Up @@ -1806,9 +1811,9 @@ Released 2018-10-23, tagged as
@ref GL::Mesh::setIndexBuffer() can now take ownership of a passed
@ref GL::Buffer to simplify resource management on user side. See
@ref GL-Mesh-buffer-ownership for more information.
- @ref GL::Mesh::draw() and @ref GL::MeshView::draw() now return a reference
to self to make method-chained draws (for example using a different shader)
possible as well
- @cpp GL::Mesh::draw() @ce and @cpp GL::MeshView::draw() @ce now return a
reference to self to make method-chained draws (for example using a
different shader) possible as well
- The @ref GL::BufferUsage parameter in @ref GL::Buffer::setData() is now
optional, defaults to @ref GL::BufferUsage::StaticDraw

Expand Down Expand Up @@ -3237,8 +3242,8 @@ a high-level overview.
- Removed deprecated `Mesh*::set*{Range,Count}()` functions, use
@ref GL::Mesh::setCount() "Mesh*::setCount()" and
@ref GL::MeshView::setIndexRange() "MeshView::setIndexRange()" instead
- Removed deprecated parameterless @ref GL::Mesh::draw() "Mesh*::draw()"
overload, use the one with explicit shader parameter instead
- Removed deprecated parameterless @cpp GL::Mesh::draw() @ce overload, use
the one with explicit shader parameter instead
- Removed deprecated `Context::Flag::Robustness` enum value, use
@ref GL::Context::Flag::RobustAccess "Context::Flag::RobustAccess" instead
- Removed deprecated `Texture::Target` enum, use dedicated
Expand Down
5 changes: 3 additions & 2 deletions doc/method-chaining.dox
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,9 @@ configured in one run, reducing count of bind calls from 9 to 3.
@snippet MagnumGL.cpp method-chaining-texture-chained

Method chaining is not used on non-configuring functions, such as
@ref GL::Framebuffer::clear() or @ref GL::Mesh::draw(), as these won't be
commonly used in conjunction with other functions anyway.
@ref GL::Framebuffer::clear() or @ref GL::AbstractShaderProgram::draw(), as
their desired use is commonly as a last step in the chain, after everything
else.

Method chaining is also used in @ref SceneGraph and other libraries and in some
cases it allows you to just "configure and forget" without even saving the
Expand Down
8 changes: 4 additions & 4 deletions doc/opengl-mapping.dox
Original file line number Diff line number Diff line change
Expand Up @@ -138,10 +138,10 @@ OpenGL function | Matching API
@fn_gl{DispatchCompute} | @ref GL::AbstractShaderProgram::dispatchCompute()
@fn_gl_extension{DispatchComputeGroupSize,ARB,compute_variable_group_size} | |
@fn_gl{DispatchComputeIndirect} | |
@fn_gl{DrawArrays}, \n @fn_gl{DrawArraysInstanced}, \n @fn_gl{DrawArraysInstancedBaseInstance}, \n @fn_gl{DrawElements}, \n @fn_gl{DrawRangeElements}, \n @fn_gl{DrawElementsBaseVertex}, \n @fn_gl{DrawRangeElementsBaseVertex}, \n @fn_gl{DrawElementsInstanced}, \n @fn_gl{DrawElementsInstancedBaseInstance}, \n @fn_gl{DrawElementsInstancedBaseVertex}, \n @fn_gl{DrawElementsInstancedBaseVertexBaseInstance} | @ref GL::Mesh::draw(AbstractShaderProgram&), \n @ref GL::MeshView::draw(AbstractShaderProgram&)
@fn_gl{DrawArrays}, \n @fn_gl{DrawArraysInstanced}, \n @fn_gl{DrawArraysInstancedBaseInstance}, \n @fn_gl{DrawElements}, \n @fn_gl{DrawRangeElements}, \n @fn_gl{DrawElementsBaseVertex}, \n @fn_gl{DrawRangeElementsBaseVertex}, \n @fn_gl{DrawElementsInstanced}, \n @fn_gl{DrawElementsInstancedBaseInstance}, \n @fn_gl{DrawElementsInstancedBaseVertex}, \n @fn_gl{DrawElementsInstancedBaseVertexBaseInstance} | @ref GL::AbstractShaderProgram::drawTransformFeedback()
@fn_gl{DrawArraysIndirect}, \n @fn_gl{DrawElementsIndirect}, \n @fn_gl{MultiDrawArraysIndirect}, \n @fn_gl{MultiDrawElementsIndirect} | |
@fn_gl{DrawBuffer}, \n `glNamedFramebufferDrawBuffer()`, \n @fn_gl{DrawBuffers}, \n `glNamedFramebufferDrawBuffers()` | @ref GL::DefaultFramebuffer::mapForDraw(), \n @ref GL::Framebuffer::mapForDraw()
@fn_gl{DrawTransformFeedback}, \n @fn_gl{DrawTransformFeedbackInstanced}, \n @fn_gl{DrawTransformFeedbackStream}, \n @fn_gl{DrawTransformFeedbackStreamInstanced} | @ref GL::Mesh::draw(AbstractShaderProgram&, TransformFeedback&, UnsignedInt), \n @ref GL::MeshView::draw(AbstractShaderProgram&, TransformFeedback&, UnsignedInt)
@fn_gl{DrawTransformFeedback}, \n @fn_gl{DrawTransformFeedbackInstanced}, \n @fn_gl{DrawTransformFeedbackStream}, \n @fn_gl{DrawTransformFeedbackStreamInstanced} | @ref GL::AbstractShaderProgram::drawTransformFeedback()

@subsection opengl-mapping-functions-e E

Expand Down Expand Up @@ -292,7 +292,7 @@ OpenGL function | Matching API
@fn_gl{MapBuffer}, \n `glMapNamedBuffer()`, \n @fn_gl{MapBufferRange}, \n `glMapNamedBufferRange()`, \n @fn_gl{UnmapBuffer}, \n `glUnmapNamedBuffer()` | @ref GL::Buffer::map(), @ref GL::Buffer::unmap()
@fn_gl{MemoryBarrier}, \n `glMemoryBarrierByRegion()` | @ref GL::Renderer::setMemoryBarrier(), \n @ref GL::Renderer::setMemoryBarrierByRegion()
@fn_gl{MinSampleShading} | @ref GL::Renderer::setMinSampleShading()
@fn_gl{MultiDrawArrays}, \n @fn_gl{MultiDrawElements}, \n @fn_gl{MultiDrawElementsBaseVertex} | @ref GL::MeshView::draw(AbstractShaderProgram&, std::initializer_list<Containers::Reference<MeshView>>)
@fn_gl{MultiDrawArrays}, \n @fn_gl{MultiDrawElements}, \n @fn_gl{MultiDrawElementsBaseVertex} | @ref GL::AbstractShaderProgram::draw(Containers::ArrayView<const Containers::Reference<MeshView>>)
@fn_gl{MultiDrawArraysIndirectCount}, \n @fn_gl{MultiDrawElementsIndirectCount} | |

@subsection opengl-mapping-functions-o O
Expand Down Expand Up @@ -394,7 +394,7 @@ OpenGL function | Matching API
@fn_gl_extension{UniformHandle,ARB,bindless_texture}, \n @fn_gl_extension{ProgramUniformHandle,ARB,bindless_texture} | |
@fn_gl{UniformBlockBinding} | @ref GL::AbstractShaderProgram::setUniformBlockBinding()
@fn_gl{UniformSubroutines} | |
@fn_gl{UseProgram} | @ref GL::Mesh::draw(), @ref GL::MeshView::draw()
@fn_gl{UseProgram} | @ref GL::AbstractShaderProgram::draw(), \n @ref GL::AbstractShaderProgram::drawTransformFeedback(), \n @ref GL::AbstractShaderProgram::dispatchCompute()
@fn_gl{UseProgramStages} | |

@subsection opengl-mapping-functions-v V
Expand Down
9 changes: 9 additions & 0 deletions doc/snippets/MagnumGL.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,15 @@ enum: UnsignedInt {
};
/* [AbstractShaderProgram-output-attributes] */

#if !defined(MAGNUM_TARGET_GLES) && !defined(MAGNUM_TARGET_WEBGL)
/* [AbstractShaderProgram-hide-irrelevant] */
private:
using GL::AbstractShaderProgram::drawTransformFeedback;
using GL::AbstractShaderProgram::dispatchCompute;
/* [AbstractShaderProgram-hide-irrelevant] */
public:
#endif

/* [AbstractShaderProgram-constructor] */
explicit MyShader() {
/* Load shader sources */
Expand Down
79 changes: 79 additions & 0 deletions src/Magnum/GL/AbstractShaderProgram.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,15 @@

#include "Magnum/GL/Context.h"
#include "Magnum/GL/Extensions.h"
#include "Magnum/GL/Mesh.h"
#include "Magnum/GL/MeshView.h"
#include "Magnum/GL/Shader.h"
#ifndef MAGNUM_TARGET_WEBGL
#include "Magnum/GL/Implementation/DebugState.h"
#endif
#ifdef MAGNUM_TARGET_GLES
#include "Magnum/GL/Implementation/MeshState.h"
#endif
#include "Magnum/GL/Implementation/ShaderProgramState.h"
#include "Magnum/GL/Implementation/State.h"
#include "Magnum/Math/RectangularMatrix.h"
Expand Down Expand Up @@ -333,6 +338,80 @@ std::pair<bool, std::string> AbstractShaderProgram::validate() {
return {success, std::move(message)};
}

void AbstractShaderProgram::draw(Mesh& mesh) {
CORRADE_ASSERT(mesh._countSet, "GL::AbstractShaderProgram::draw(): Mesh::setCount() was never called, probably a mistake?", );

/* Nothing to draw, exit without touching any state */
if(!mesh._count || !mesh._instanceCount) return;

use();

#ifndef MAGNUM_TARGET_GLES
mesh.drawInternal(mesh._count, mesh._baseVertex, mesh._instanceCount, mesh._baseInstance, mesh._indexOffset, mesh._indexStart, mesh._indexEnd);
#elif !defined(MAGNUM_TARGET_GLES2)
mesh.drawInternal(mesh._count, mesh._baseVertex, mesh._instanceCount, mesh._indexOffset, mesh._indexStart, mesh._indexEnd);
#else
mesh.drawInternal(mesh._count, mesh._baseVertex, mesh._instanceCount, mesh._indexOffset);
#endif
}

void AbstractShaderProgram::draw(MeshView& mesh) {
CORRADE_ASSERT(mesh._countSet, "GL::AbstractShaderProgram::draw(): MeshView::setCount() was never called, probably a mistake?", );

/* Nothing to draw, exit without touching any state */
if(!mesh._count || !mesh._instanceCount) return;

use();

#ifndef MAGNUM_TARGET_GLES
mesh._original->drawInternal(mesh._count, mesh._baseVertex, mesh._instanceCount, mesh._baseInstance, mesh._indexOffset, mesh._indexStart, mesh._indexEnd);
#elif !defined(MAGNUM_TARGET_GLES2)
mesh._original->drawInternal(mesh._count, mesh._baseVertex, mesh._instanceCount, mesh._indexOffset, mesh._indexStart, mesh._indexEnd);
#else
mesh._original->drawInternal(mesh._count, mesh._baseVertex, mesh._instanceCount, mesh._indexOffset);
#endif
}

void AbstractShaderProgram::draw(Containers::ArrayView<const Containers::Reference<MeshView>> meshes) {
if(meshes.empty()) return;

use();

#ifndef CORRADE_NO_ASSERT
const Mesh* original = &meshes.begin()->get()._original.get();
for(MeshView& mesh: meshes)
CORRADE_ASSERT(&mesh._original.get() == original, "GL::AbstractShaderProgram::draw(): all meshes must be views of the same original mesh", );
#endif

#ifndef MAGNUM_TARGET_GLES
MeshView::multiDrawImplementationDefault(meshes);
#else
Context::current().state().mesh->multiDrawImplementation(meshes);
#endif
}

void AbstractShaderProgram::draw(std::initializer_list<Containers::Reference<MeshView>> meshes) {
draw(Containers::arrayView(meshes));
}

#ifndef MAGNUM_TARGET_GLES
void AbstractShaderProgram::drawTransformFeedback(Mesh& mesh, TransformFeedback& xfb, UnsignedInt stream) {
/* Nothing to draw, exit without touching any state */
if(!mesh._instanceCount) return;

use();
mesh.drawInternal(xfb, stream, mesh._instanceCount);
}

void AbstractShaderProgram::drawTransformFeedback(MeshView& mesh, TransformFeedback& xfb, UnsignedInt stream) {
/* Nothing to draw, exit without touching any state */
if(!mesh._instanceCount) return;

use();
mesh._original->drawInternal(xfb, stream, mesh._instanceCount);
}
#endif

#if !defined(MAGNUM_TARGET_GLES2) && !defined(MAGNUM_TARGET_WEBGL)
void AbstractShaderProgram::dispatchCompute(const Vector3ui& workgroupCount) {
use();
Expand Down
Loading

0 comments on commit 32d49db

Please sign in to comment.