Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

setPaintProperty(layerId, '[icon|text]-opacity', ...) will raise on a undefined uniformName #7511

Open
nrako opened this issue Oct 30, 2018 · 7 comments

Comments

@nrako
Copy link

nrako commented Oct 30, 2018

mapbox-gl-js version: 0.50.0

browser: Chrome Version 69.0.3497.100

Context

Map with several layers (symbols and texts).

setPaintProperty(layerId, '[icon|text]-opacity', ...) on multiple layers will sometimes raise the following error:

Uncaught TypeError: Cannot read property 'set' of undefined
    at SourceExpressionBinder.setUniforms (mapbox-gl-dev.js:16727)
    at ProgramConfiguration.setUniforms (mapbox-gl-dev.js:17332)
    at Program.draw (mapbox-gl-dev.js:53626)
    at drawSymbolElements (mapbox-gl-dev.js:55164)
    at drawLayerSymbols (mapbox-gl-dev.js:55139)
    at Object.drawSymbols [as symbol] (mapbox-gl-dev.js:54913)
    at Painter.renderLayer (mapbox-gl-dev.js:59165)
    at Painter.render (mapbox-gl-dev.js:59121)
    at Map._render (mapbox-gl-dev.js:65617)
    at mapbox-gl-dev.js:65725


setUniforms           @ mapbox-gl-dev.js:16727
setUniforms           @ mapbox-gl-dev.js:17332
draw                  @ mapbox-gl-dev.js:53626
drawSymbolElements    @ mapbox-gl-dev.js:55164
drawLayerSymbols      @ mapbox-gl-dev.js:55139
drawSymbols           @ mapbox-gl-dev.js:54913
renderLayer           @ mapbox-gl-dev.js:59165
render                @ mapbox-gl-dev.js:59121
_render               @ mapbox-gl-dev.js:65617
(anonymous)           @ mapbox-gl-dev.js:65725
requestAnimationFrame (async)
frame                 @ mapbox-gl-dev.js:2468
triggerRepaint        @ mapbox-gl-dev.js:65723
_render               @ mapbox-gl-dev.js:65654
(anonymous)           @ mapbox-gl-dev.js:65725
requestAnimationFrame (async)
frame                 @ mapbox-gl-dev.js:2468
triggerRepaint        @ mapbox-gl-dev.js:65723
_render               @ mapbox-gl-dev.js:65654
(anonymous)           @ mapbox-gl-dev.js:65725
requestAnimationFrame (async)
frame                 @ mapbox-gl-dev.js:2468
triggerRepaint        @ mapbox-gl-dev.js:65723
_update               @ mapbox-gl-dev.js:65532
(anonymous)           @ mapbox-gl-dev.js:64168
fire                  @ mapbox-gl-dev.js:2853
fire                  @ mapbox-gl-dev.js:2879
update                @ mapbox-gl-dev.js:51758
_render               @ mapbox-gl-dev.js:65596
(anonymous)           @ mapbox-gl-dev.js:65725
requestAnimationFrame (async)
frame                 @ mapbox-gl-dev.js:2468
triggerRepaint        @ mapbox-gl-dev.js:65723
_update               @ mapbox-gl-dev.js:65532
setPaintProperty      @ mapbox-gl-dev.js:65220
@nrako nrako changed the title setPaintProperty(layerId, '[icon|text]-opacity', ...) -> setUniforms raise on undefined uniformName setPaintProperty(layerId, '[icon|text]-opacity', ...) will raise on a undefined uniformName Oct 30, 2018
@ryanhamley
Copy link
Contributor

ryanhamley commented Oct 30, 2018

Hi @nrako can you provide us with a minimal reproduction that demonstrates this bug?

It looks very similar to this bug #6242 but it would be helpful to know exactly what's happening since that bug was fixed in v0.45 and you're on v0.50.

@nrako
Copy link
Author

nrako commented Oct 31, 2018

Hi @ryanhamley – I'd love to but I'm afraid this might be quite hard to isolate. There's a timing factor at play which I can't seem to isolate nor figure out. Adding a breakpoint with a condition around setUniforms won't reproduce the issue, I think it's because the breakpoint slows down the JS execution which I thought was odd but I I've been able to somehow confirm that by doing a CPU throttling at 4x in Chrome DevTools. However using all the different tricks to differ the calls of setPaintProperty won't fix the issue neither so I'm out of clues. I should add that setPaintProperty is triggered in user land by an onChange (react) event on an input – I don't think it matters but ¯_(ツ)_/¯.

@leigeber
Copy link

leigeber commented Jan 29, 2019

We just ran into this same issue as well in the context of a satellite layer switching function. Running version 0.52.0. It doesn't happen every time but does most times. I've created a stripped down example at https://jsfiddle.net/2jvtbpwr/.

If you run the Fiddle a few times you're likely to see the console error.

@mourner mourner reopened this Jan 29, 2019
@mourner
Copy link
Member

mourner commented Jan 29, 2019

@leigeber thank you for the test case! Reopening.

@mourner
Copy link
Member

mourner commented Jan 30, 2019

This seems to be a sync issue related to #4012. Here's a reduced test case:

mapboxgl.accessToken = 'pk.eyJ1IjoibGFuZHNlYXJjaCIsImEiOiJFQlRLRC1FIn0.3VCEYjxSawGQqjMKl8aitA';

var map = new mapboxgl.Map({
    container: 'map',
    style: 'mapbox://styles/landsearch/cjqx5v3ob4iyz2rpxuifuzpfd?optimize=true',
    center: [-100.04, 38.907],
    zoom: 3
});

map.on('load', function () {
    map.setPaintProperty('water-line-label', 'text-color', 'red');
});

The property which is data-driven is set to constant, and there appears to be a race condition after loading tiles partially where the program tries to set text-color as a uniform even though the binders are still referring to a data-driven attribute.

@mourner
Copy link
Member

mourner commented Jan 30, 2019

This naive guard 90fb962 seems to fix the issue but I'm not sure if it's the right approach. @ansis any thoughts on this?

@rogeraustin
Copy link

@mourner - the fix mentioned above does not seem to have made its way into the release.

I am also seeing this issue, albeit I am making rather unconventional use of the API. I don't have a stripped-down example, but basically what I am doing is composing two styles together - one a Mapbox background and one from my own server, each of which has its own sprite. This follows on from a comment by @lucaswoj in a discussion on composing styles.

I first get the style json for each style and compose the layers and sources into a new style without a sprite. I then download each of the sprites (png and json) and compose these together into a set of named images. I am then ready to call map.setStyle with my new style, except first I remove all images from the map (listImages, removeImage), add all the images from my set (addImage) and finally call setStyle. If I then want to switch the background (from, say, Streets to Dark) I simply repeat this process with the new pair of styles. Because there is no sprite in the style itself, I get a 'smart' setStyle that applies only the diffs, giving a very smooth update of the background with little or no flickering. Likewise when I switch between my user-defined styles.

However, I also get the Uncaught Typeerror when switching between certain backgrounds. The number of such errors is typically between 1 and 5 and they occur only when switching between one of Dark or Light, and one of the other styles. I don't see them when switching Dark-Light, or Streets-Outdoor. Likewise, there is no error when switching between my user-defined styles while retaining the same Mapbox background.

It's not clear what visual effect this is having, if any. There are certainly no tile-shaped holes in the map. Perhaps everything just gets sorted out on the next frame.

I am intending to continue with this approach, so it would be nice to have some reassurance that these errors are either harmless or will be fixed. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants