mapbox / mapbox-studio-classic

https://www.mapbox.com/mapbox-studio/
BSD 3-Clause "New" or "Revised" License
1.14k stars 229 forks source link

Updating with 0.3.7 breaks all colors #1558

Open nyurik opened 8 years ago

nyurik commented 8 years ago

Our repo kartotherian/osm-bright.tm2 gets badly messed up when editing with MBS v0.3.7 - many roads change color to pinkish. The repo was mostly updated with versions v0.3.1 and before, without any issues. cc: @MaxSem @springmeyer

springmeyer commented 8 years ago

Thanks for reporting. This is unexpected as v0.3.7 was intended to fix color problems: https://github.com/mapbox/mapbox-studio-classic/blob/mb-pages/CHANGELOG.md#037. There are two known regressions recently fixed: https://github.com/mapbox/carto/issues/442 and https://github.com/mapbox/carto/issues/443. Can you see if you can isolate what else changed and create an issue at https://github.com/mapbox/carto/issues? /cc @nebulon42

springmeyer commented 8 years ago

Also can you isolate which version of studio changed things? It is possible this is a carto regression but also possible it is due to the node-mapnik upgrade. See https://github.com/mapbox/mapbox-studio-classic/blob/mb-pages/CHANGELOG.md for when things changed.

nyurik commented 8 years ago

Attached are the styles as generated by versions 0.3.4 -- 0.3.7: v0.3.4 - last good version, no changes compared to previous ones 0.3.5, 0.3.6, and 0.3.7 each introduce different changes in colors, each differ from others, and each looks wrong in different ways.

osm-bright.tm2-broken styles.zip

springmeyer commented 8 years ago

@nyurik thanks. So in v0.3.5 both node-mapnik and carto were upgraded. So I've done a test build with upgrade node-mapnik but downgraded carto. If this one works that will indicate carto regressions. Can you test it:

1) go to https://mapbox.s3.amazonaws.com/mapbox-studio/index.html 2) Download one of the be65824 builds per your operating system.

nebulon42 commented 8 years ago

My tests indicate a problem with colour multiplication (e.g. #f8f4f0 [=land] * 0.8). I'm at it, but I can't rule out that there are other (additional) problems.

@springmeyer osm-bright.tm2 should also be affected, maybe you can confirm. I couldn't get mapbox-studio-classic to run properly.

nyurik commented 8 years ago

@springmeyer I tested your build, and it works perfectly (not a single line has changed). Also, you can easily test it yourself - it uses our public pbf tiles from maps.wikimedia.org. To test, I open that style in the studio, hit Save, and do a git diff in the repo. Lastly, I'm now in the gisdevs.slack and irc in case you have a direct question.

git clone https://github.com/kartotherian/osm-bright.tm2.git
cd osm-bright.tm2
npm install
nebulon42 commented 8 years ago

The problem is that 0.8 is another colour in RGB than in HSL (that's how colours are now represented internally). My personal opinion is that something like #f8f4f0 * 0.8 does not really make sense. Since we have to maintain backwards compatibility in all cases a fix is needed, but I think it will be getting a bit hackish. I will simply treat all single values that are converted to a color as greyscale RGB representations.

nebulon42 commented 8 years ago

Carto problem should be fixed upstream.