tangrams / bubble-wrap

Bubble Wrap basemap style
http://tangrams.github.io/bubble-wrap/
MIT License
27 stars 21 forks source link

Black ramps on mobile ES renderers #97

Closed nvkelso closed 8 years ago

nvkelso commented 8 years ago

On my Samsung Galaxy S6 edge I see a lot of black ramps (links) both by highways and at other road intersections. Usually this is a mid-zoom problem, and zooming in the ramps assume the intended color. But true highway ramp to highway links don't seem as affected by this problem.

nvkelso commented 8 years ago

screenshot_2015-11-25-16-15-42 screenshot_2015-11-25-16-15-49 screenshot_2015-11-25-16-15-53 screenshot_2015-11-25-16-15-56 screenshot_2015-11-25-16-16-10 screenshot_2015-11-25-16-16-13 screenshot_2015-11-25-16-16-17 screenshot_2015-11-25-16-27-11

nvkelso commented 8 years ago

I've heard similar reports from NYC about this.

nvkelso commented 8 years ago

Still showing as an issue in master-518 build :(

bcamper commented 8 years ago

I've not been able to verify this fix on device yet, but I believe that this may be the culprit, a malformed YAML reference:

-                    color: [[8, *major_road4], [13, major_road2], [17, *major_road1]]
+                    color: [[8, *major_road4], [13, *major_road2], [17, *major_road1]]

https://github.com/tangrams/eraser-map/commit/fbbbeb0cc62202da5a51da4c65d65f4b2a6e4a62

My theory is this:

cc @blair1618 for thoughts on this theory (also someone should verify, I can't get my ES build to work at home currently :), and ideas on what default behavior should be across platforms.

tallytalwar commented 8 years ago

@bcamper Yup, this is it.

bcamper commented 8 years ago

I am on a debugging roll :D

On Thu, Dec 10, 2015 at 4:29 PM, Varun notifications@github.com wrote:

@bcamper https://github.com/bcamper Yup, this is it.

-

This without the fix: [image: screenshot from 2015-12-10 16-27-44] https://cloud.githubusercontent.com/assets/360641/11729029/2ef3afc4-9f5b-11e5-9f16-36abdb06ce5a.png

This with the fix: [image: screenshot from 2015-12-10 16-27-16] https://cloud.githubusercontent.com/assets/360641/11729023/1e473916-9f5b-11e5-95b5-564b2b6ad791.png

— Reply to this email directly or view it on GitHub https://github.com/tangrams/eraser-map/issues/97#issuecomment-163754888.

matteblair commented 8 years ago

Yeah that's exactly what was happening. The color is black because that's what css-color-parser-cpp returns when it can't match a string. We can pick a different color though. I would actually suggest some garish shade of pink or yellow because as you have just found, having a noticeable "failure" color makes it easier to spot unwanted behavior.

tallytalwar commented 8 years ago

In my previous work, I have usually used a cheddar cheese color as an error color or a default color. However thats my personal preference, because I tend to not like the color.

http://images.myperfectcolor.com/repositories/images/colors/MPC00052407-2.jpg

nvkelso commented 8 years ago

Brett fixed this (was missing a * in front of the YAML anchor ref).