Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

Commit

Permalink
[core] Remove re-linking programs approach (#14482)
Browse files Browse the repository at this point in the history
Remove re-linking programs as redundant.
It costs (cheaper to link once than twice) and (subjective) is not that common GL API
usage pattern, although perfectly legal and permitted.
Initial idea, behind the removed code, was to enable work on optimization
that would reduce number of attrib setup calls in case when VAO is not
available (as described in #9433).

As such optimization is not implemented, and it is arguable if it makes sense
to do it now, we can remove re-linking.

Related to closed PRs #9433 and PR #11583.

I have [measured the time spent just on relinking](https://gist.github.com/astojilj/29bd5a5c5dc0b2d9f29ecb660da07fbf) using release build on iPhone SE (A9, same as iPhone 6S):

- 1st run after reboot or installation
Total 37.14ms, average per program:1.86ms

- reopening
Total: 2.47ms, average per program: 0.12ms

This time we save using the patch here.
  • Loading branch information
astojilj committed May 20, 2019
1 parent 8d91264 commit 2a3a1c7
Show file tree
Hide file tree
Showing 5 changed files with 35 additions and 71 deletions.
38 changes: 5 additions & 33 deletions src/mbgl/gl/attribute.cpp
Original file line number Diff line number Diff line change
@@ -1,47 +1,19 @@
#include <mbgl/gl/attribute.hpp>
#include <mbgl/gl/context.hpp>
#include <mbgl/gl/defines.hpp>

namespace mbgl {
namespace gl {

using namespace platform;

void bindAttributeLocation(Context& context, ProgramID id, AttributeLocation location, const char* name) {
// We're using sequentially numberered attribute locations starting with 0. Therefore we can use
// the location as a proxy for the number of attributes.
if (location >= context.maximumVertexBindingCount) {
// Don't bind the location on this hardware since it exceeds the limit (otherwise we'd get
// an OpenGL error). This means we'll see rendering errors, and possibly slow rendering due
// to unbound attributes.
optional<AttributeLocation> queryLocation(ProgramID id, const char* name) {
GLint attributeLocation = MBGL_CHECK_ERROR(glGetAttribLocation(id, name));
if (attributeLocation != -1) {
return attributeLocation;
} else {
MBGL_CHECK_ERROR(glBindAttribLocation(id, location, name));
return {};
}
}

std::set<std::string> getActiveAttributes(ProgramID id) {
std::set<std::string> activeAttributes;

GLint attributeCount;
MBGL_CHECK_ERROR(glGetProgramiv(id, GL_ACTIVE_ATTRIBUTES, &attributeCount));

GLint maxAttributeLength;
MBGL_CHECK_ERROR(glGetProgramiv(id, GL_ACTIVE_ATTRIBUTE_MAX_LENGTH, &maxAttributeLength));

std::string attributeName;
attributeName.resize(maxAttributeLength);

GLsizei actualLength;
GLint size;
GLenum type;

for (int32_t i = 0; i < attributeCount; i++) {
MBGL_CHECK_ERROR(glGetActiveAttrib(id, i, maxAttributeLength, &actualLength, &size, &type, &attributeName[0]));
activeAttributes.emplace(std::string(attributeName, 0, actualLength));
}

return activeAttributes;
}

} // namespace gl
} // namespace mbgl
47 changes: 19 additions & 28 deletions src/mbgl/gl/attribute.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -2,28 +2,20 @@

#include <mbgl/gfx/attribute.hpp>
#include <mbgl/gl/types.hpp>
#include <mbgl/util/ignore.hpp>
#include <mbgl/programs/attributes.hpp>
#include <mbgl/util/literal.hpp>
#include <mbgl/util/indexed_tuple.hpp>
#include <mbgl/util/optional.hpp>

#include <cstddef>
#include <vector>
#include <set>
#include <functional>
#include <string>
#include <array>
#include <limits>

namespace mbgl {
namespace gl {

using AttributeBindingArray = std::vector<optional<gfx::AttributeBinding>>;
using NamedAttributeLocations = std::vector<std::pair<const std::string, AttributeLocation>>;

class Context;
void bindAttributeLocation(Context&, ProgramID, AttributeLocation, const char * name);
std::set<std::string> getActiveAttributes(ProgramID);
optional<AttributeLocation> queryLocation(ProgramID id, const char* name);

template <class>
class AttributeLocations;
Expand All @@ -37,31 +29,30 @@ class AttributeLocations<TypeList<As...>> final {
Locations locations;

public:
AttributeLocations(Context& context, const ProgramID& id)
: locations([&] {
std::set<std::string> activeAttributes = getActiveAttributes(id);

AttributeLocation location = 0;
auto maybeBindLocation = [&](const char* name) -> optional<AttributeLocation> {
if (activeAttributes.count(name)) {
bindAttributeLocation(context, id, location, name);
return location++;
} else {
return {};
}
};

return Locations{ maybeBindLocation(
concat_literals<&string_literal<'a', '_'>::value, &As::name>::value())... };
}()) {
}
AttributeLocations() = default;

template <class BinaryProgram>
AttributeLocations(const BinaryProgram& program)
: locations{ program.attributeLocation(
concat_literals<&string_literal<'a', '_'>::value, &As::name>::value())... } {
}

void queryLocations(const ProgramID& id) {
locations = Locations{
queryLocation(id, concat_literals<&string_literal<'a', '_'>::value, &As::name>::value())... };
using TypeOfFirst = typename std::tuple_element<0, std::tuple<As...>>::type;
auto first = locations.template get<TypeOfFirst>();
assert(first && first.value() == 0);
}

static constexpr const char* getFirstAttribName() {
// Static assert that attribute list starts with position: we bind it on location 0.
using TypeOfFirst = typename std::tuple_element<0, std::tuple<As...>>::type;
static_assert(std::is_same<attributes::pos, TypeOfFirst>::value || std::is_same<attributes::pos_offset, TypeOfFirst>::value ||
std::is_same<attributes::pos_normal, TypeOfFirst>::value, "Program must start with position related attribute.");
return concat_literals<&string_literal<'a', '_'>::value, TypeOfFirst::name>::value();
}

NamedAttributeLocations getNamedLocations() const {
NamedAttributeLocations result;

Expand Down
8 changes: 7 additions & 1 deletion src/mbgl/gl/context.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -169,12 +169,18 @@ UniqueShader Context::createShader(ShaderType type, const std::initializer_list<
throw std::runtime_error("shader failed to compile");
}

UniqueProgram Context::createProgram(ShaderID vertexShader, ShaderID fragmentShader) {
UniqueProgram Context::createProgram(ShaderID vertexShader, ShaderID fragmentShader, const char* location0AttribName) {
UniqueProgram result { MBGL_CHECK_ERROR(glCreateProgram()), { this } };

MBGL_CHECK_ERROR(glAttachShader(result, vertexShader));
MBGL_CHECK_ERROR(glAttachShader(result, fragmentShader));

// It is important to have attribute at position 0 enabled: conveniently,
// position attribute is always first and always enabled. The integrity of
// this assumption is verified in AttributeLocations::queryLocations and
// AttributeLocations::getFirstAttribName.
MBGL_CHECK_ERROR(glBindAttribLocation(result, 0, location0AttribName));

linkProgram(result);

return result;
Expand Down
2 changes: 1 addition & 1 deletion src/mbgl/gl/context.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ class Context final : public gfx::Context {
void enableDebugging();

UniqueShader createShader(ShaderType type, const std::initializer_list<const char*>& sources);
UniqueProgram createProgram(ShaderID vertexShader, ShaderID fragmentShader);
UniqueProgram createProgram(ShaderID vertexShader, ShaderID fragmentShader, const char* location0AttribName);
UniqueProgram createProgram(BinaryProgramFormat binaryFormat, const std::string& binaryProgram);
void verifyProgramLinkage(ProgramID);
void linkProgram(ProgramID);
Expand Down
11 changes: 3 additions & 8 deletions src/mbgl/gl/program.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -49,15 +49,10 @@ class Program final : public gfx::Program<Name> {
const std::initializer_list<const char*>& fragmentSource)
: program(context.createProgram(
context.createShader(ShaderType::Vertex, vertexSource),
context.createShader(ShaderType::Fragment, fragmentSource))),
attributeLocations(context, program) {
// Re-link program after manually binding only active attributes in Attributes::queryLocations
context.linkProgram(program);

// We have to re-initialize the uniforms state from the bindings as the uniform locations
// get shifted on some implementations
context.createShader(ShaderType::Fragment, fragmentSource),
attributeLocations.getFirstAttribName())) {
attributeLocations.queryLocations(program);
uniformStates.queryLocations(program);

// Texture units are specified via uniforms as well, so we need query their locations
textureStates.queryLocations(program);
}
Expand Down

0 comments on commit 2a3a1c7

Please sign in to comment.