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

adjust buildings hue, darken outlines #1208

Merged
merged 1 commit into from
Jan 24, 2015

Conversation

nebulon42
Copy link
Contributor

Starting from #1202 (comment) I'd like to propose a change were the hue of buildings is moved from the more yellowish HSV(39°, .07, .84) to the more reddish HSV(25°, .07, .84). At the same time the outline is darkened by 10% to now 20% from the fill for more contrast.

Previews (before on the left, after on the right):
These previews are outdated.

no landuse
building_no_landuse_z18

industrial/retail/commerical
building_industrial_etc_z16

residential/place of worship:
building_residential_pow_z18

garden/grass:
building_green_z18

farmland/farmyard:
building_farmland_z16

@matthijsmelissen
Copy link
Collaborator

Personally I think I prefer the old rendering (on a LCD laptop screen).

Would darkening the outline to 15% or 20% while keeping the fill the same be an option?

@nebulon42
Copy link
Contributor Author

That would be an option, but more contrast would be better.

These are outlines darkened by 15%.
farm_after_2

industrial_2

Apart from outlines I strongly suggest to change the hue. The improvement is not that visible on the images, so I suggest to try it live.

Ok, maybe I understood you wrong. You were talking about keeping the hue?
That is the first image in #1202 (comment). That doesn't look particularly good IMO. It looks great on the canvas, but not on landuses.

@matthijsmelissen
Copy link
Collaborator

@mkoniecz @pnorman What do you think?

@pnorman
Copy link
Collaborator

pnorman commented Jan 15, 2015

I'm okay with either, but the building outlines might need to have their weight adjusted.

@kocio-pl
Copy link
Collaborator

I like those slightly darker buildings more, but outline fix would be also good. The recent changes in buildings rendering were OK for me, but went just a bit too far.

@nebulon42
Copy link
Contributor Author

I like those slightly darker buildings more

Just for clarifying: the buildings are not darker, it's just the more reddish hue that makes them look darker (and of course the darker outlines).

On another monitor I tested the yellow hue issue wasn't that amplified as on my monitor, still I would like to shift the hue more towards red. I think this is not a big change. I'm open to all suggestions how dark the outlines should be.

@pnorman
Copy link
Collaborator

pnorman commented Jan 16, 2015

Just for clarifying: the buildings are not darker, it's just the more reddish hue that makes them look darker (and of course the darker outlines).

HSV(39°, .07, .84) has a lightness of 70 while HSV(25°, .07, .84) has a lightness of 61. If you want to say something the same lightness, chroma or hue you can't work in HSV but need to use a perceptual colour space.

@vholten
Copy link
Contributor

vholten commented Jan 16, 2015

HSV(39°, .07, .84) has a lightness of 70 while HSV(25°, .07, .84) has a lightness of 61.

However, what is actually used in the code and on screen are the RGB values, and in RGB the two colors are (214, 209, 200) and (214, 207, 200), so only the G channel differs by two units. This corresponds to a very small change in lightness.

@pnorman
Copy link
Collaborator

pnorman commented Jan 16, 2015

However, what is actually used in the code and on screen are the RGB values, and in RGB the two colors are (214, 209, 200) and (214, 207, 200), so only the G channel differs by two units. This corresponds to a very small change in lightness.

Ah - typo in the conversion.

@nebulon42
Copy link
Contributor Author

I see I stand corrected. :)

@nebulon42
Copy link
Contributor Author

I think more important than adjusting the hue would be darkening the building outlines. I have made another attempt in changing the hue a little bit to red, from Lch(84, 5, 88) to Lch(84, 5, 80). But this can also be left out if there is no support for it. Outlines are still darkened by 20%.

no landuse
no_landuse

industrial/retail/commerical
industrial

residential/place of worship
residential

garden/grass
green

farmyard/farmland
farmland_farmyard

(note that the changed forest symbology is apparently a TillMill caching issue)

@nebulon42 nebulon42 changed the title adjust buildings hue adjust buildings hue, darken outlines Jan 19, 2015
@althio
Copy link

althio commented Jan 19, 2015

+1 for outlines by 20%.
The hue modification is maybe a step on the right direction, but too subtle to make any real difference to my eyes and screen (only +1 on red channel RGB?).

@pnorman
Copy link
Collaborator

pnorman commented Jan 19, 2015

(note that the changed forest symbology is apparently a TillMill caching issue)

After any PNG or SVG changes, you need to restart TileMill or Kosmtik. If that doesn't solve it, there's a cache folder in the TileMill directory you can clear.

@nebulon42
Copy link
Contributor Author

The hue modification is maybe a step on the right direction, but too subtle to make any real difference to my eyes and screen

Maybe it is too subtle, but I wanted to keep it low since changing back to red doesn't seem to be too popular. I would be open for more red. Maybe the maintainers could clarify which change (hue and/or building outlines) would be an option for them, if any. Thanks.

@matthijsmelissen
Copy link
Collaborator

I don't have a strong opinion.

Perhaps you could state again what goals you are trying to accomplish, or what problem this is meant to solve?

@nebulon42
Copy link
Contributor Author

After the change in building colours (which I still think was a necessary improvement) the representation of buildings on landuse colours suffers from rather low contrast. Especially on the grey of residential landuse the current buildings hue results in a dirty, yellowish light-brown.

Buildings as they are now look great on the empty canvas, but not so good in combination with landuse=residential and other landuses such as industrial/commerical/retail/farmyard:

buildings_canvas
buildings_residential

To solve the low contrast issue I suggest to darken the building outlines by 10% more:

buildings_outlines

This improves the situation, but there is still a slightly dirty, yellow-brownish look. Therefore, I would suggest to shift the hue (and only the hue) of the buildings colour slightly towards red:
buildings_red

The colour shown here is now Lch(84, 5, 75). This is a bit more towards red as the Lch(84, 5, 80) depicted in #1208 (comment).

However, the main improvement IMO are the stronger building outlines.

@matthijsmelissen
Copy link
Collaborator

Thanks for the clear write-up.

I think the new outlines are too strong compared to other features, in particular roads:
screenshot from 2015-01-23 13 48 19
screenshot from 2015-01-23 13 49 09
Maybe 15% (increase of 5%) would be a good compromise?

@nebulon42
Copy link
Contributor Author

Yes, 15% would still be ok for me. What do you think about the hue change?

@matthijsmelissen
Copy link
Collaborator

I support the hue change, now you break it up in steps I see what you mean.

@nebulon42
Copy link
Contributor Author

Based on the discussion so far the new proposal would then be a building colour of Lch(84, 5, 70) in contrast to the currently used Lch(84, 5, 88) and darkened outlines by 15% in contrast to the currently 10% darker outlines.

no landuse
no_landuse

industrial/retail/commerical
industrial

residential/place of worship
residential

garden/grass
grass

farmyard/farmland
farmland

@matthijsmelissen
Copy link
Collaborator

Looks good to me. Can you update the PR?

@nebulon42
Copy link
Contributor Author

Looks good to me. Can you update the PR?

Ah, I did that already (before I commented). :-)

@matthijsmelissen
Copy link
Collaborator

Ah, I did that already (before I commented). :-)

You're right, I was confused by the fact that Github says that the last commit was authored 15 days ago (but it is in fact the latest change). Some aspects of Git(hub) remain a mystery to me.

@nebulon42
Copy link
Contributor Author

Since I always squash updates only the initial commit remains and that might be 15 days ago. So technically Github seems to be right, but it could be confusing.

On the other hand, another process related question: Do you prefer squashed updates all the time or to see the update history with squashing occuring just before the commit?

@matthijsmelissen
Copy link
Collaborator

On the other hand, another process related question: Do you prefer squashed updates all the time or to see the update history with squashing occuring just before the commit?

To me either is fine, I'm not sure what the others think.

@matkoniecz
Copy link
Contributor

To me either is fine, but it certainly should be done before merging (to avoid multiple "fix/change stuff" commits on master, like it sometimes unfortunately happens).

@matthijsmelissen matthijsmelissen merged commit 867fc92 into gravitystorm:master Jan 24, 2015
@nebulon42
Copy link
Contributor Author

Thanks.

@nebulon42 nebulon42 deleted the building-hue branch January 24, 2015 10:04
@daganzdaanda
Copy link

Cool, I think this is some good finetuning!

@nebulon42 nebulon42 mentioned this pull request Feb 1, 2015
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.

8 participants