From e1562582c6d70687f54ffda5083480b6ee46e622 Mon Sep 17 00:00:00 2001 From: Lauren Budorick Date: Tue, 6 Jun 2017 14:17:08 -0700 Subject: [PATCH] Bind vertex attributes in the order determined by their ProgramInterface --- src/data/array_group.js | 2 +- src/data/buffer_group.js | 2 +- src/data/program_configuration.js | 6 ++++-- src/render/painter.js | 13 +++++++------ src/shaders/describe_shaders | 2 +- src/shaders/symbol_icon.vertex.glsl | 2 -- src/shaders/symbol_sdf.vertex.glsl | 2 -- 7 files changed, 14 insertions(+), 15 deletions(-) diff --git a/src/data/array_group.js b/src/data/array_group.js index a118e3f9ea3..87ffd7868f3 100644 --- a/src/data/array_group.js +++ b/src/data/array_group.js @@ -47,7 +47,7 @@ class ArrayGroup { this.layerData = {}; for (const layer of layers) { const programConfiguration = ProgramConfiguration.createDynamic( - programInterface.paintAttributes || [], layer, zoom); + programInterface, layer, zoom); this.layerData[layer.id] = { layer: layer, programConfiguration: programConfiguration, diff --git a/src/data/buffer_group.js b/src/data/buffer_group.js index d4025157f63..11058afb366 100644 --- a/src/data/buffer_group.js +++ b/src/data/buffer_group.js @@ -31,7 +31,7 @@ class BufferGroup { this.layerData = {}; for (const layer of layers) { const array = arrays.paintVertexArrays && arrays.paintVertexArrays[layer.id]; - const programConfiguration = ProgramConfiguration.createDynamic(programInterface.paintAttributes || [], layer, zoom); + const programConfiguration = ProgramConfiguration.createDynamic(programInterface, layer, zoom); const paintVertexBuffer = array ? new Buffer(array.array, array.type, Buffer.BufferType.VERTEX) : null; this.layerData[layer.id] = {programConfiguration, paintVertexBuffer}; } diff --git a/src/data/program_configuration.js b/src/data/program_configuration.js index 396c045851d..25fbdc14db2 100644 --- a/src/data/program_configuration.js +++ b/src/data/program_configuration.js @@ -32,12 +32,13 @@ class ProgramConfiguration { this.interpolationUniforms = []; this.pragmas = {vertex: {}, fragment: {}}; this.cacheKey = ''; + this.interface = {}; } - static createDynamic(attributes, layer, zoom) { + static createDynamic(programInterface, layer, zoom) { const self = new ProgramConfiguration(); - for (const attributeConfig of attributes) { + for (const attributeConfig of programInterface.paintAttributes || []) { const attribute = normalizePaintAttribute(attributeConfig, layer); assert(/^a_/.test(attribute.name)); const name = attribute.name.slice(2); @@ -51,6 +52,7 @@ class ProgramConfiguration { } } self.PaintVertexArray = createVertexArrayType(self.attributes); + self.interface = programInterface; return self; } diff --git a/src/render/painter.js b/src/render/painter.js index 47cd6443ced..a921daef9f9 100644 --- a/src/render/painter.js +++ b/src/render/painter.js @@ -393,12 +393,13 @@ class Painter { assert(gl.getShaderParameter(vertexShader, gl.COMPILE_STATUS), gl.getShaderInfoLog(vertexShader)); gl.attachShader(program, vertexShader); - - // For the symbol programs, manually ensure the attrib bound to position 0 is always used (either a_data or a_pos_offset would work here). - // This is needed to fix https://github.com/mapbox/mapbox-gl-js/issues/4607 — otherwise a_size can be bound first, causing rendering to fail. - // All remaining attribs will be bound dynamically below. - if (name === 'symbolSDF' || name === 'symbolIcon') { - gl.bindAttribLocation(program, 0, 'a_data'); + // Manually bind layout attributes in the order defined by their + // ProgramInterface so that we don't dynamically link an unused + // attribute at position 0, which can cause rendering to fail for an + // entire layer (see #4607, #4728) + const layoutAttributes = configuration.interface.layoutAttributes || []; + for (let i = 0; i < layoutAttributes.length; i++) { + gl.bindAttribLocation(program, i, layoutAttributes[i].name); } gl.linkProgram(program); diff --git a/src/shaders/describe_shaders b/src/shaders/describe_shaders index 53cfddf4501..5e909e9a0a4 100755 --- a/src/shaders/describe_shaders +++ b/src/shaders/describe_shaders @@ -51,7 +51,7 @@ function addProgramInfo(layer, programInterface, shaderDefinition) { const styleLayer = new StyleLayer(layer); styleLayer.updatePaintTransitions([], {}, { zoom: zoom }, new AnimationLoop(), {}); const configuration = ProgramConfiguration.createDynamic( - programInterface.paintAttributes, styleLayer, zoom); + programInterface, styleLayer, zoom); const key = `${layer.type}${configuration.cacheKey || ''}`; const program = programs[key] = programs[key] || { layers: [] }; program.layers.push(layer); diff --git a/src/shaders/symbol_icon.vertex.glsl b/src/shaders/symbol_icon.vertex.glsl index 2bf1d1a2382..2cc973c5613 100644 --- a/src/shaders/symbol_icon.vertex.glsl +++ b/src/shaders/symbol_icon.vertex.glsl @@ -1,5 +1,3 @@ -// NOTE: the a_data attribute in this shader is manually bound (see https://github.com/mapbox/mapbox-gl-js/issues/4607 / #4728). -// If removing or renaming a_data, revisit the manual binding in painter.js accordingly. attribute vec4 a_pos_offset; attribute vec2 a_label_pos; attribute vec4 a_data; diff --git a/src/shaders/symbol_sdf.vertex.glsl b/src/shaders/symbol_sdf.vertex.glsl index cb5d44f19af..c64ba924897 100644 --- a/src/shaders/symbol_sdf.vertex.glsl +++ b/src/shaders/symbol_sdf.vertex.glsl @@ -1,7 +1,5 @@ const float PI = 3.141592653589793; -// NOTE: the a_data attribute in this shader is manually bound (see https://github.com/mapbox/mapbox-gl-js/issues/4607). -// If removing or renaming a_data, revisit the manual binding in painter.js accordingly. attribute vec4 a_pos_offset; attribute vec2 a_label_pos; attribute vec4 a_data;