From 7fe22b37260b92b6f82c45be6cc8b39b143f54e7 Mon Sep 17 00:00:00 2001 From: Lauren Budorick Date: Mon, 8 May 2017 13:47:49 -0700 Subject: [PATCH 1/3] Determine order of symbol attribute binding (Binding unused/undefined attribs to position 0 causes symbols to not render in Safari) --- src/render/painter.js | 23 +++++++++++++++++++---- src/shaders/symbol_sdf.vertex.glsl | 3 +++ 2 files changed, 22 insertions(+), 4 deletions(-) diff --git a/src/render/painter.js b/src/render/painter.js index 04c7ade994b..abd5ba1ab53 100644 --- a/src/render/painter.js +++ b/src/render/painter.js @@ -393,16 +393,31 @@ class Painter { assert(gl.getShaderParameter(vertexShader, gl.COMPILE_STATUS), gl.getShaderInfoLog(vertexShader)); gl.attachShader(program, vertexShader); + const result = {program}; + + // Manually determine the order of attributes for the symbol program (see https://github.com/mapbox/mapbox-gl-js/issues/4607). + // We bind a_data to position 0 as it is always used (binding a_size can cause rendering to fail). + if (name === 'symbolSDF') { + result.numAttributes = 3; + ['a_data', 'a_pos_offset', 'a_size'].forEach((name, i) => { + gl.bindAttribLocation(program, i, name); + result[name] = i; + }); + } + gl.linkProgram(program); assert(gl.getProgramParameter(program, gl.LINK_STATUS), gl.getProgramInfoLog(program)); const numAttributes = gl.getProgramParameter(program, gl.ACTIVE_ATTRIBUTES); - const result = {program, numAttributes}; - for (let i = 0; i < numAttributes; i++) { - const attribute = gl.getActiveAttrib(program, i); - result[attribute.name] = gl.getAttribLocation(program, attribute.name); + if (name !== 'symbolSDF') { + result.numAttributes = numAttributes; + for (let i = 0; i < numAttributes; i++) { + const attribute = gl.getActiveAttrib(program, i); + result[attribute.name] = gl.getAttribLocation(program, attribute.name); + } } + const numUniforms = gl.getProgramParameter(program, gl.ACTIVE_UNIFORMS); for (let i = 0; i < numUniforms; i++) { const uniform = gl.getActiveUniform(program, i); diff --git a/src/shaders/symbol_sdf.vertex.glsl b/src/shaders/symbol_sdf.vertex.glsl index 90ed1956582..151c07398d2 100644 --- a/src/shaders/symbol_sdf.vertex.glsl +++ b/src/shaders/symbol_sdf.vertex.glsl @@ -1,5 +1,8 @@ const float PI = 3.141592653589793; +// NOTE: attribute locations in this shader are currently manually bound (see https://github.com/mapbox/mapbox-gl-js/issues/4607). +// If adding or removing attributes from this shader, modify the bindings in painter.js accordingly, making sure the attrib at +// position 0 is always used. attribute vec4 a_pos_offset; attribute vec4 a_data; From a4f26af745e4338df85354e352b82b87afa2a5c7 Mon Sep 17 00:00:00 2001 From: Lauren Budorick Date: Mon, 8 May 2017 14:54:04 -0700 Subject: [PATCH 2/3] Only manually bind one attrib (a_data at 0) and let all others bind dynamically --- src/render/painter.js | 22 +++++++++------------- src/shaders/symbol_sdf.vertex.glsl | 5 ++--- 2 files changed, 11 insertions(+), 16 deletions(-) diff --git a/src/render/painter.js b/src/render/painter.js index abd5ba1ab53..f774eb9fb57 100644 --- a/src/render/painter.js +++ b/src/render/painter.js @@ -395,14 +395,12 @@ class Painter { const result = {program}; - // Manually determine the order of attributes for the symbol program (see https://github.com/mapbox/mapbox-gl-js/issues/4607). - // We bind a_data to position 0 as it is always used (binding a_size can cause rendering to fail). + // For the symbol program, 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') { - result.numAttributes = 3; - ['a_data', 'a_pos_offset', 'a_size'].forEach((name, i) => { - gl.bindAttribLocation(program, i, name); - result[name] = i; - }); + gl.bindAttribLocation(program, 0, 'a_data'); + result['a_data'] = 0; } gl.linkProgram(program); @@ -410,12 +408,10 @@ class Painter { const numAttributes = gl.getProgramParameter(program, gl.ACTIVE_ATTRIBUTES); - if (name !== 'symbolSDF') { - result.numAttributes = numAttributes; - for (let i = 0; i < numAttributes; i++) { - const attribute = gl.getActiveAttrib(program, i); - result[attribute.name] = gl.getAttribLocation(program, attribute.name); - } + result.numAttributes = numAttributes; + for (let i = 0; i < numAttributes; i++) { + const attribute = gl.getActiveAttrib(program, i); + result[attribute.name] = gl.getAttribLocation(program, attribute.name); } const numUniforms = gl.getProgramParameter(program, gl.ACTIVE_UNIFORMS); diff --git a/src/shaders/symbol_sdf.vertex.glsl b/src/shaders/symbol_sdf.vertex.glsl index 151c07398d2..8de5e56492e 100644 --- a/src/shaders/symbol_sdf.vertex.glsl +++ b/src/shaders/symbol_sdf.vertex.glsl @@ -1,8 +1,7 @@ const float PI = 3.141592653589793; -// NOTE: attribute locations in this shader are currently manually bound (see https://github.com/mapbox/mapbox-gl-js/issues/4607). -// If adding or removing attributes from this shader, modify the bindings in painter.js accordingly, making sure the attrib at -// position 0 is always used. +// 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 vec4 a_data; From d808166bd47dce71a8a4ed9e84489f984572c4ae Mon Sep 17 00:00:00 2001 From: Lauren Budorick Date: Mon, 8 May 2017 15:00:26 -0700 Subject: [PATCH 3/3] Undo a few unnecessary changes --- src/render/painter.js | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/render/painter.js b/src/render/painter.js index f774eb9fb57..26d12870aaf 100644 --- a/src/render/painter.js +++ b/src/render/painter.js @@ -393,27 +393,24 @@ class Painter { assert(gl.getShaderParameter(vertexShader, gl.COMPILE_STATUS), gl.getShaderInfoLog(vertexShader)); gl.attachShader(program, vertexShader); - const result = {program}; // For the symbol program, 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') { gl.bindAttribLocation(program, 0, 'a_data'); - result['a_data'] = 0; } gl.linkProgram(program); assert(gl.getProgramParameter(program, gl.LINK_STATUS), gl.getProgramInfoLog(program)); const numAttributes = gl.getProgramParameter(program, gl.ACTIVE_ATTRIBUTES); + const result = {program, numAttributes}; - result.numAttributes = numAttributes; for (let i = 0; i < numAttributes; i++) { const attribute = gl.getActiveAttrib(program, i); result[attribute.name] = gl.getAttribLocation(program, attribute.name); } - const numUniforms = gl.getProgramParameter(program, gl.ACTIVE_UNIFORMS); for (let i = 0; i < numUniforms; i++) { const uniform = gl.getActiveUniform(program, i);