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

Icons for Places and Highways do not Render in Safari #4728

Closed
caneroj1 opened this issue May 20, 2017 · 10 comments · Fixed by #4793
Closed

Icons for Places and Highways do not Render in Safari #4728

caneroj1 opened this issue May 20, 2017 · 10 comments · Fixed by #4793
Assignees

Comments

@caneroj1
Copy link

mapbox-gl-js version: master

Steps to Trigger Behavior

  1. Use mapbox-gl-js to render the mapbox://styles/mapbox/streets-v9 style
  2. Open the map in Safari Version 10.1
  3. See that there are no icons that mark places.

Expected Behavior

There are black dots signifying the location of places on the map, like "Brak" and "Sabha", and there are also the icons behind the highway numbers.
screen shot 2017-05-20 at 11 32 31 am

Actual Behavior

There are no black dots signifying the location of places on the map, like "Brak" and "Sabha", and highway icons are missing.
screen shot 2017-05-20 at 11 32 58 am

I am using the mapbox-gl-js from master so I can get the fix from #4688, otherwise the labels wouldn't even be displaying.

@kkaefer
Copy link
Member

kkaefer commented May 23, 2017

This seems to be the same issue as in #4688

@lbud, I think we need a longer-term solution for binding attributes in the correct order, and possibly about enabling/disabling them correctly. In Native, we explicitly specify the order of attributes, and bind them in ascending order.

@jfirebaugh
Copy link
Contributor

This seems to be the same issue as in #4688

I'm not so sure. @caneroj1 you said "I am using the mapbox-gl-js from master so I can get the fix from #4688". Can you confirm that you see this issue even after the fix from #4688?

@kkaefer
Copy link
Member

kkaefer commented May 24, 2017

@jfirebaugh I'm seeing the problem as well, and it has the same sluggish behavior as #4688. It only happens on every 200th reload or so.

mapbox gl js debug page 2017-05-24 11-47-05

@mollymerp
Copy link
Contributor

here's the map.painter.cache symbolIcon readout for when I can reproduce the issue

{
    "program": {},
    "numAttributes": 3,
    "a_size": 0,
    "a_data": 1,
    "a_pos_offset": 2,
    ...
}

and the same for when the map is working correctly

{
    "program": {},
    "numAttributes": 3,
    "a_pos_offset": 0,
    "a_size": 1,
    "a_data": 2,
    ...
}

@mollymerp
Copy link
Contributor

Slightly different, but also occurred since the last release, so #4747 could be related.

@caneroj1
Copy link
Author

@jfirebaugh Yes, I can confirm it is still happening.

@kkaefer Interesting. It happens more frequently than every 200th reload for me, though. They're never there.

I can try getting another build off of master sometime today and see what the status is.

@kkaefer
Copy link
Member

kkaefer commented May 24, 2017

@caneroj1

It happens more frequently than every 200th reload for me, though. They're never there.

That's possible. The attribute assignments are dependent only on the GPU driver/Shader compiler, so if your machine has a different GPU, you're likely to see different results. E.g. it's also possible that your shader cache has cached a version of the program with the bad attribute assignment, so you're now seeing it on every reload because it's retrieving the cached program.

@lbud
Copy link
Contributor

lbud commented Jun 6, 2017

Sorry for not taking a look at this sooner — this is the same root cause as #4688; that PR only fixed it for the symbol_sdf shader. For now I'm going to implement the same fix for the symbol_icon shader (manually binding a_data to attribute location 0 since it is always defined), and as @kkaefer says we should revisit the idea of manually binding all attributes in a fixed order as we do in native.

@jfirebaugh
Copy link
Contributor

I think we should pursue the more complete fix now -- I think this issue could come up for any of the shaders.

If createProgram had access to a "program interface", it could iterate over layoutAttributes. We could give it access via configuration, by passing the full programInterface here and storing a reference.

@lbud
Copy link
Contributor

lbud commented Jun 6, 2017

👉 #4794

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

Successfully merging a pull request may close this issue.

5 participants