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

Make tertiary road yellowish [respawn] #2085

Closed
wants to merge 1 commit into from

Conversation

kocio-pl
Copy link
Collaborator

@kocio-pl kocio-pl commented Mar 9, 2016

Resolves #1974.

This is a purely technical follow up to #2078 to make tertiary roads filled with light yellow. All the details and rendered images are there.

@pnorman
Copy link
Collaborator

pnorman commented Mar 10, 2016

What needs to be done is scripts/generate_road_colours.py needs modifying, then the output of it pasting into roads.mss

@gravitystorm
Copy link
Owner

Hmmm. That isn't the easiest of scripts to understand. It's certainly unclear to me as to what @kocio-pl would need to do in the way of script modifications, since there are constants and magic numbers defined throughout that script.

If someone who knows what that script is for, what it does, and/or how it works would explain it (either here, or inline comments) then I think that would help both me and @kocio-pl

@matthijsmelissen
Copy link
Collaborator

Agree, I had the same problem.

If I'm not mistaken, the script assumes that the colour distance between motorway and trunk should be equal to the distance between trunk and primary and the distance between primary and secondary. I don't think we should hardcode that decision.

@kocio-pl
Copy link
Collaborator Author

Just for the record - documenting this script is just another part of #2080.

@matkoniecz
Copy link
Contributor

If I'm not mistaken, the script assumes that the colour distance between motorway and trunk should be equal to the distance between trunk and primary and the distance between primary and secondary. I don't think we should hardcode that decision.

I agree that it is not a fundamental decision that should not be changed. Decision to keep differences between motorway and trunk, trunk and primary, primary and secondary the same was made to reduce numbers of parameters controlling how roads are styled.

Before that decision any change to motorways hue/saturation/lightness required manual changes of hue/saturation/lightness of trunk, primary, secondary (in variant with non-white tertiary - also tertiary) to keep these road classes sufficiently distinguishable.

In principle it is possible to make differences in h/s/l different between various road types (like it is done in this PR) but one problem with even perceptual color spaces that different people have vastly different opinion which colours are clearly different and which ones are too similar to easily distinguish (important to avoid #1946 or even worse problems). Situation is worsened by drastically different display of the same image on different screens - even on the same screen properly distinguishable elements may be impossible to differentiate with slightly sunnier weather.

To summarize: constant difference between different road types was done to simplify ridiculously complex problem, into something merely complex it is not necessary to keep it - after all, white roads are already a special case.

With generation of trunk/primary:

4 road classes, fill+casing, low/high zoom, hue/chroma/lightness: 48 parameters

With motorway and secondary set manually, trunk and primary generated: 24 parameters

(note - 48/24 parameters are ones that control just colour of major roads - full parameter count controlling road styling is around 300).

On topic of this PR - sorry, unfortunately I was unable to review it for now.

@matkoniecz
Copy link
Contributor

tl;dr previous comment

This script is used solely to avoid manually defining trunk and primary colours based on motorway and secondary colours, white roads are special case.

I see nothing fundamentally wrong in special case of yellowish tertiary roads.

I want to test this PR but unfortunately I need to solve some computer problems requiring funny things like migrating data to a new computer (TileMill stopped working without clear reason, kosmtik is not usable on 32bit systems).

@matthijsmelissen
Copy link
Collaborator

I need to solve some computer problems

Let us know if there's something we can help you with. I suppose you already tried reinstalling TileMill?

@kocio-pl
Copy link
Collaborator Author

Or maybe try it on a fresh user account (to avoid possible configuration problems)?

@matkoniecz
Copy link
Contributor

I suppose you already tried reinstalling TileMill?

Yes, along related packages like nodejs.

Or maybe try it on a fresh user account (to avoid possible configuration problems)?

One of reasons for reinstallation is that I am unable to create a new working user account (I am not doing this solely to keep tilemill happy - migrating to 64 bit will not only allow me to use Kosmtik but also other software that is either officially or de facto not working on 32 bit).

@matthijsmelissen
Copy link
Collaborator

Would we also need to change the colour of the labels?

@pnorman
Copy link
Collaborator

pnorman commented Mar 28, 2016

Would we also need to change the colour of the labels?

Yes - which is one reason for the need for the changes to be in the script, since that's what generates the values which go into the generate shield script

@matkoniecz
Copy link
Contributor

Sorry for a long waiting time!

My thoughts after testing:

(1) it would be desirable to reduce width of tertiary roads, current width was used to separate them from other white roads. With this change it is necessary to separate them also from both from white roads and yellow secondaries

(2) tertiary shield colour should be changed

(3) personally I am not a big fan of this change, I tested something quite similar and thought that white tertiary is superior

(4) but there are no big problems, it decently works next to parkings and on farmland. It is worse next to educational areas, but it should be acceptable

http://www.openstreetmap.org/#map=16/50.0777/19.9370
selection_001

(5) I know that ?/some/many/? feel that white tertiary are significant problem, so overall I am not supporting this idea but I am also not opposing it

@dieterdreist
Copy link

sent from a phone

Am 08.04.2016 um 17:16 schrieb Mateusz Konieczny notifications@github.com:

(1) it would be desirable to reduce width of tertiary roads, current width was used to separate them from other white roads. With this change it is necessary to separate them also from both from white roads and yellow secondaries

I think they have to remain significantly wider. than unclassified and residential
The faint yellow is not sufficient to distinguish them downwards

@jojo4u
Copy link

jojo4u commented Apr 10, 2016

Making tertiary yellow again would leave room for widthening of unclassified once the hiearchy issuses with resiential/unclassified have been sorted out. On the other hand the proposed yellow is very pale, I wonder if this is suited for low-quality screens and mobile.

@mboeringa
Copy link

Anyone still working on this?

@kocio-pl
Copy link
Collaborator Author

I wait for documenting the script to make any additional changes. I guess @matkoniecz - as the author - would make it best, but I don't know if/when he plans to do this.

@matthijsmelissen
Copy link
Collaborator

I'm afraid I agree the purpose of the script is unclear. It's also not clear to me how it should be used, and how it can be parametrized. Some documentation is really needed.

@matthijsmelissen
Copy link
Collaborator

I wait for documenting the script to make any additional changes. I guess @matkoniecz - as the author - would make it best, but I don't know if/when he plans to do this.

@matkoniecz Any news on this topic?

@jdhoek
Copy link
Contributor

jdhoek commented Jul 10, 2016

@math1985 I think I can help with the script. It is not complex.

What the script does is create a palette of n colours where the hue, lightness, and saturation of each consecutive colour is increased/decreased by equal steps. It uses a colour space called CIE lch. This is good, because this colour space is better suited for this kind of incremental colour maths. It is possible to use HSL instead (which most developers here will know from CSS), but to the human eye those colours won't seem to increase in equal steps, even if you increase/decrease the hue, saturation, and lightness of those colours by equals increments/decrements.

Luckily, this scripts hides the complexity of working with CIE hcl and its conversion to RGB (which we need in the stylesheet) by using a Python library called colormath.

The current script does this for four colours. We want five (tertiary joins the group). That is easy to add; the script has a list of road types on L.27.

Next we need to tweak the end values a bit to get to the suggested colour of @kocio-pl. These can be configured on L.51 and beyond.

This is what I get by tweaking the script:

road-colours

Current

What we have now.

Suggested

No change in script, only add tertiary with suggested fill.

Script (column 3)

Only add tertiary to the script and use the output.

Script (column 4)

Add tertiary to the script, and tweak the ending values to match the suggested colour. This aligns all five colours again. I have tweaked the stroke colour here as well to work towards the white-and-grey of unclassified/residential.

@jdhoek
Copy link
Contributor

jdhoek commented Jul 10, 2016

The script also generates palettes for low zoom and road shields; I have omitted these from the sample above.

@jdhoek
Copy link
Contributor

jdhoek commented Jul 10, 2016

This is how it renders for me in Kosmtik. I agree with @matkoniecz that with the colour added, the width could well be reduced a little. I have adjusted the width of the tertiary road to sit exactly between residential and secondary in the sample.

2-and-3

@kocio-pl
Copy link
Collaborator Author

Thanks for explanations, could you also create a PR with all your changes?

@jdhoek
Copy link
Contributor

jdhoek commented Jul 10, 2016

@kocio-pl Coming right up.

@jdhoek
Copy link
Contributor

jdhoek commented Jul 10, 2016

I have split up the PR in two parts. The prerequisite refactor is pending approval.

The coloured tertiary road can be tested in my branch. I will offer it as PR when the refactor lands.

@pnorman
Copy link
Collaborator

pnorman commented Jul 10, 2016

Closing because this will need rewriting after #2220, but should be much easier.

@pnorman pnorman closed this Jul 10, 2016
@kocio-pl kocio-pl deleted the tertiary-yellow branch July 10, 2016 23:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants