[core] Rework attribute binding (again)

cmake/core-files.cmake

@@ -73,8 +73,6 @@ set(MBGL_CORE_FILES
     src/mbgl/gl/program.hpp
     src/mbgl/gl/program_binary_extension.hpp
     src/mbgl/gl/renderbuffer.hpp
-    src/mbgl/gl/segment.cpp
-    src/mbgl/gl/segment.hpp
     src/mbgl/gl/state.hpp
     src/mbgl/gl/stencil_mode.cpp
     src/mbgl/gl/stencil_mode.hpp
@@ -84,6 +82,8 @@ set(MBGL_CORE_FILES
     src/mbgl/gl/uniform.hpp
     src/mbgl/gl/value.cpp
     src/mbgl/gl/value.hpp
+    src/mbgl/gl/vertex_array.cpp
+    src/mbgl/gl/vertex_array.hpp
     src/mbgl/gl/vertex_array_extension.hpp
     src/mbgl/gl/vertex_buffer.hpp
 
@@ -154,6 +154,7 @@ set(MBGL_CORE_FILES
     src/mbgl/programs/programs.hpp
     src/mbgl/programs/raster_program.cpp
     src/mbgl/programs/raster_program.hpp
+    src/mbgl/programs/segment.hpp
     src/mbgl/programs/symbol_program.cpp
     src/mbgl/programs/symbol_program.hpp
     src/mbgl/programs/uniforms.hpp
@@ -611,8 +612,8 @@ set(MBGL_CORE_FILES
     src/mbgl/util/stopwatch.cpp
     src/mbgl/util/stopwatch.hpp
     src/mbgl/util/string.cpp
-    src/mbgl/util/thread_local.hpp
     src/mbgl/util/thread.hpp
+    src/mbgl/util/thread_local.hpp
     src/mbgl/util/throttler.cpp
     src/mbgl/util/throttler.hpp
     src/mbgl/util/tile_coordinate.hpp

platform/node/test/ignores.json

@@ -0,0 +1,61 @@
+{
+  "query-tests/geometry/multilinestring": "needs investigation",
+  "query-tests/geometry/multipolygon": "needs investigation",
+  "query-tests/geometry/polygon": "needs investigation",
+  "query-tests/regressions/mapbox-gl-js#3534": "https://github.com/mapbox/mapbox-gl-native/issues/8193",
+  "query-tests/regressions/mapbox-gl-js#4417": "https://github.com/mapbox/mapbox-gl-native/issues/8007",
+  "query-tests/symbol-features-in/pitched-screen": "https://github.com/mapbox/mapbox-gl-native/issues/6817",
+  "query-tests/symbol-features-in/tilted-inside": "https://github.com/mapbox/mapbox-gl-native/issues/5056",
+  "query-tests/symbol-features-in/tilted-outside": "https://github.com/mapbox/mapbox-gl-native/issues/9435",
+  "query-tests/world-wrapping/box": "skip - needs issue",
+  "query-tests/world-wrapping/point": "skip - needs issue",
+  "render-tests/circle-pitch-alignment/map-scale-map": "https://github.com/mapbox/mapbox-gl-native/issues/9349",
+  "render-tests/circle-pitch-alignment/map-scale-viewport": "https://github.com/mapbox/mapbox-gl-native/issues/9349",
+  "render-tests/circle-pitch-alignment/viewport-scale-map": "https://github.com/mapbox/mapbox-gl-native/issues/9349",
+  "render-tests/circle-pitch-alignment/viewport-scale-viewport": "https://github.com/mapbox/mapbox-gl-native/issues/9349",
+  "render-tests/debug/collision-overscaled": "https://github.com/mapbox/mapbox-gl-native/issues/3841",
+  "render-tests/debug/collision-pitched-wrapped": "https://github.com/mapbox/mapbox-gl-native/issues/3841",
+  "render-tests/debug/collision-pitched": "https://github.com/mapbox/mapbox-gl-native/issues/3841",
+  "render-tests/debug/collision": "https://github.com/mapbox/mapbox-gl-native/issues/3841",
+  "render-tests/debug/tile-overscaled": "https://github.com/mapbox/mapbox-gl-native/issues/3841",
+  "render-tests/debug/tile": "https://github.com/mapbox/mapbox-gl-native/issues/3841",
+  "render-tests/extent/1024-circle": "needs investigation",
+  "render-tests/extent/1024-symbol": "needs investigation",
+  "render-tests/fill-extrusion-pattern/@2x": "https://github.com/mapbox/mapbox-gl-js/issues/3327",
+  "render-tests/fill-extrusion-pattern/function-2": "https://github.com/mapbox/mapbox-gl-js/issues/3327",
+  "render-tests/fill-extrusion-pattern/function": "https://github.com/mapbox/mapbox-gl-js/issues/3327",
+  "render-tests/fill-extrusion-pattern/literal": "https://github.com/mapbox/mapbox-gl-js/issues/3327",
+  "render-tests/fill-extrusion-pattern/missing": "https://github.com/mapbox/mapbox-gl-js/issues/3327",
+  "render-tests/fill-extrusion-pattern/opacity": "https://github.com/mapbox/mapbox-gl-js/issues/3327",
+  "render-tests/fill-outline-color/fill": "https://github.com/mapbox/mapbox-gl-native/pull/8760",
+  "render-tests/fill-outline-color/function": "https://github.com/mapbox/mapbox-gl-native/issues/6927",
+  "render-tests/fill-outline-color/literal": "https://github.com/mapbox/mapbox-gl-native/issues/6927",
+  "render-tests/fill-outline-color/multiply": "https://github.com/mapbox/mapbox-gl-native/issues/6927",
+  "render-tests/fill-outline-color/opacity": "https://github.com/mapbox/mapbox-gl-native/issues/6927",
+  "render-tests/fill-outline-color/zoom-and-property-function": "https://github.com/mapbox/mapbox-gl-native/issues/6927",
+  "render-tests/geojson/inline-linestring-fill": "current behavior is arbitrary",
+  "render-tests/geojson/inline-polygon-symbol": "behavior needs reconciliation with gl-js",
+  "render-tests/icon-size/composite-function-high-base-plain": "https://github.com/mapbox/mapbox-gl-native/issues/8654",
+  "render-tests/icon-size/composite-function-high-base-sdf": "https://github.com/mapbox/mapbox-gl-native/issues/8654",
+  "render-tests/icon-text-fit/both-padding": "https://github.com/mapbox/mapbox-gl-native/issues/5602",
+  "render-tests/icon-text-fit/both": "https://github.com/mapbox/mapbox-gl-native/issues/5602",
+  "render-tests/icon-text-fit/height": "https://github.com/mapbox/mapbox-gl-native/issues/5602",
+  "render-tests/icon-text-fit/width-padding": "https://github.com/mapbox/mapbox-gl-native/issues/5602",
+  "render-tests/icon-text-fit/width": "https://github.com/mapbox/mapbox-gl-native/issues/5602",
+  "render-tests/line-width/property-function": "https://github.com/mapbox/mapbox-gl-js/issues/3682#issuecomment-264348200",
+  "render-tests/regressions/mapbox-gl-js#2305": "https://github.com/mapbox/mapbox-gl-native/issues/6927",
+  "render-tests/regressions/mapbox-gl-js#3010": "skip - needs issue",
+  "render-tests/regressions/mapbox-gl-js#3548": "skip - needs issue",
+  "render-tests/regressions/mapbox-gl-js#3682": "https://github.com/mapbox/mapbox-gl-js/issues/3682",
+  "render-tests/regressions/mapbox-gl-js#4550": "skip - https://github.com/mapbox/mapbox-gl-native/issues/1350",
+  "render-tests/regressions/mapbox-gl-js#4551": "skip - https://github.com/mapbox/mapbox-gl-native/issues/1350",
+  "render-tests/regressions/mapbox-gl-js#4573": "skip - https://github.com/mapbox/mapbox-gl-native/issues/1350",
+  "render-tests/regressions/mapbox-gl-native#7357": "https://github.com/mapbox/mapbox-gl-native/issues/7357",
+  "render-tests/runtime-styling/paint-property-fill-flat-to-extrude": "skip - https://github.com/mapbox/mapbox-gl-native/issues/6745",
+  "render-tests/runtime-styling/set-style-paint-property-fill-flat-to-extrude": "skip - needs issue",
+  "render-tests/runtime-styling/source-add-geojson-inline": "skip - needs issue",
+  "render-tests/symbol-placement/line": "needs issue",
+  "render-tests/text-keep-upright/line-placement-true-offset": "https://github.com/mapbox/mapbox-gl-native/issues/9271",
+  "render-tests/text-size/composite-function-high-base": "https://github.com/mapbox/mapbox-gl-native/issues/8654",
+  "render-tests/video/default": "skip - needs issue"
+}

platform/node/test/query.test.js

@@ -1,12 +1,13 @@
 'use strict';
 
-var suite = require('../../../mapbox-gl-js/test/integration').query;
-var suiteImplementation = require('./suite_implementation');
+const suite = require('../../../mapbox-gl-js/test/integration').query;
+const suiteImplementation = require('./suite_implementation');
+const ignores = require('./ignores.json');
 
-var tests;
+let tests;
 
 if (process.argv[1] === __filename && process.argv.length > 2) {
     tests = process.argv.slice(2);
 }
 
-suite.run('native', {tests: tests}, suiteImplementation);
+suite.run('native', {tests: tests, ignores: ignores}, suiteImplementation);

platform/node/test/render.test.js

@@ -1,12 +1,13 @@
 'use strict';
 
-var suite = require('../../../mapbox-gl-js/test/integration').render;
-var suiteImplementation = require('./suite_implementation');
+const suite = require('../../../mapbox-gl-js/test/integration').render;
+const suiteImplementation = require('./suite_implementation');
+const ignores = require('./ignores.json');
 
-var tests;
+let tests;
 
 if (process.argv[1] === __filename && process.argv.length > 2) {
     tests = process.argv.slice(2);
 }
 
-suite.run('native', {tests: tests}, suiteImplementation);
+suite.run('native', {tests: tests, ignores: ignores}, suiteImplementation);

src/mbgl/gl/attribute.cpp

@@ -1,108 +1,39 @@
 #include <mbgl/gl/attribute.hpp>
-#include <mbgl/gl/context.hpp>
 #include <mbgl/gl/gl.hpp>
 
-#include <cstring>
-
 namespace mbgl {
 namespace gl {
 
-AttributeLocation bindAttributeLocation(ProgramID id, AttributeLocation location, const char* name) {
-    assert(location < 8);
+void bindAttributeLocation(ProgramID id, AttributeLocation location, const char* name) {
+    if (location >= MAX_ATTRIBUTES) {
+        throw gl::Error("too many vertex attributes");
+    }
     MBGL_CHECK_ERROR(glBindAttribLocation(id, location, name));
-    return location;
 }
 
-int32_t getActiveAttributeCount(ProgramID id) {
-    GLint numAttributes;
-    MBGL_CHECK_ERROR(glGetProgramiv(id, GL_ACTIVE_ATTRIBUTES, &numAttributes));
-    return numAttributes;
-}
+std::set<std::string> getActiveAttributes(ProgramID id) {
+    std::set<std::string> activeAttributes;
 
-int32_t getMaxAttributeNameLength(ProgramID id) {
-    GLint nameLength;
-    MBGL_CHECK_ERROR(glGetProgramiv(id, GL_ACTIVE_ATTRIBUTE_MAX_LENGTH, &nameLength));
-    return nameLength;
-}
+    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 getAttributeName(ProgramID id, int32_t maxLength, AttributeLocation location) {
     std::string attributeName;
-    attributeName.resize(maxLength);
+    attributeName.resize(maxAttributeLength);
+
     GLsizei actualLength;
     GLint size;
     GLenum type;
-    MBGL_CHECK_ERROR(glGetActiveAttrib(id, static_cast<GLuint>(location),
-                                       static_cast<GLsizei>(maxLength), &actualLength, &size, &type,
-                                       const_cast<char*>(attributeName.data())));
-    attributeName.resize(actualLength);
-    return attributeName;
-}
-
-std::set<std::string> getActiveAttributes(ProgramID id) {
-    std::set<std::string> activeAttributes;
-
-    GLint attributeCount = getActiveAttributeCount(id);
-    GLint maxAttributeLength = getMaxAttributeNameLength(id);
 
     for (int32_t i = 0; i < attributeCount; i++) {
-        activeAttributes.emplace(getAttributeName(id, maxAttributeLength, i));
+        MBGL_CHECK_ERROR(glGetActiveAttrib(id, i, maxAttributeLength, &actualLength, &size, &type, &attributeName[0]));
+        activeAttributes.emplace(std::string(attributeName, 0, actualLength));
     }
 
     return activeAttributes;
 }
 
-void DisabledAttribute::bind(Context&, AttributeLocation location, std::size_t) const {
-    MBGL_CHECK_ERROR(glDisableVertexAttribArray(location));
-}
-
-template <class T>
-constexpr DataType DataTypeOf() {
-    return std::is_same<T,   int8_t>::value ? DataType::Byte :
-           std::is_same<T,  uint8_t>::value ? DataType::UnsignedByte :
-           std::is_same<T,  int16_t>::value ? DataType::Short :
-           std::is_same<T, uint16_t>::value ? DataType::UnsignedShort :
-           std::is_same<T,  int32_t>::value ? DataType::Integer :
-           std::is_same<T, uint32_t>::value ? DataType::UnsignedInteger :
-           std::is_same<T,    float>::value ? DataType::Float : static_cast<DataType>(0);
-}
-
-template <class T, std::size_t N>
-void AttributeBinding<T, N>::bind(Context& context, AttributeLocation location, std::size_t vertexOffset) const {
-    // FillProgram will attempt to bind at location -1 because it includes
-    // a fill-outline-color paint property but does not use it or have an
-    // a_outline_color shader attribute - in this case, we have nothing to bind.
-    if (location == -1) return;
-
-    context.vertexBuffer = vertexBuffer;
-    MBGL_CHECK_ERROR(glEnableVertexAttribArray(location));
-    MBGL_CHECK_ERROR(glVertexAttribPointer(
-        location,
-        static_cast<GLint>(attributeSize),
-        static_cast<GLenum>(DataTypeOf<T>()),
-        static_cast<GLboolean>(false),
-        static_cast<GLsizei>(vertexSize),
-        reinterpret_cast<GLvoid*>(attributeOffset + (vertexSize * vertexOffset))));
-}
-
-template class AttributeBinding<uint8_t, 1>;
-template class AttributeBinding<uint8_t, 2>;
-template class AttributeBinding<uint8_t, 3>;
-template class AttributeBinding<uint8_t, 4>;
-
-template class AttributeBinding<uint16_t, 1>;
-template class AttributeBinding<uint16_t, 2>;
-template class AttributeBinding<uint16_t, 3>;
-template class AttributeBinding<uint16_t, 4>;
-
-template class AttributeBinding<int16_t, 1>;
-template class AttributeBinding<int16_t, 2>;
-template class AttributeBinding<int16_t, 3>;
-template class AttributeBinding<int16_t, 4>;
-
-template class AttributeBinding<float, 1>;
-template class AttributeBinding<float, 2>;
-template class AttributeBinding<float, 3>;
-template class AttributeBinding<float, 4>;
-
 } // namespace gl
 } // namespace mbgl