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

Cemeteries rework in SVG #2714

Merged
merged 1 commit into from
Aug 11, 2017
Merged

Conversation

kocio-pl
Copy link
Collaborator

@kocio-pl kocio-pl commented Jul 31, 2017

Resolves #2638.

Warsaw, z19
Before
uf1lzgaa
After
blzubrge

@kocio-pl kocio-pl force-pushed the cemetery-muslim branch 2 times, most recently from 46f361d to 5eb9801 Compare July 31, 2017 12:43
@kocio-pl
Copy link
Collaborator Author

This is how it would look with all cemeteries as SVG:
Before
5ywibasp
After
ruujmuxx

@pnorman
Copy link
Collaborator

pnorman commented Jul 31, 2017

The icon density looks too high. I'd go for spacing them out more, and possibly adjusting the weight of them,

Can we deal with all the cemeteries in one PR? The above comment applies to all of the patterns.

@kocio-pl
Copy link
Collaborator Author

kocio-pl commented Jul 31, 2017

The weight seems to be because of less blur with vector icons, so I don't know what should we change. With 16x16 px matrix and 10x9 px symbols I'm also not sure how to tune it. 32 px matrix gives much higher spacing, smaller symbols might be unreadable. Maybe 18 px matrix was OK - why do you claim it should be power of 2? Or what other options do you propose?

Yes, if we'll find what we want to do, I can make one PR for all the cemeteries.

@pnorman
Copy link
Collaborator

pnorman commented Jul 31, 2017

why do you claim it should be power of 2?

The tile edges need to align with the pattern edges. This means the pattern size needs to be made from the factors of the tile size, which are all 2.

@kocio-pl
Copy link
Collaborator Author

I thought so. But with 18 px elements it should be something wrong every (256/18=14.2...) 14 rows and columns, while I see just one strangely aligned column here (instead of expected about 5) and no strange rows (instead of expected about 2), which makes me think there's some other problem:

http://www.openstreetmap.org/#map=19/52.24621/20.97196

What do you think about it?

@pnorman
Copy link
Collaborator

pnorman commented Aug 1, 2017

I thought so. But with 18 px elements it should be something wrong every (256/18=14.2...) 14 rows and columns, while I see just one strangely aligned column here (instead of expected about 5) and no strange rows (instead of expected about 2), which makes me think there's some other problem:

There are rows with problems too. Varying metatile sizes mean that you won't see exactly the same patterns, but a pattern that goes evenly into 256 is always safe.

@kocio-pl
Copy link
Collaborator Author

kocio-pl commented Aug 1, 2017

So what path you're inclined to follow - 16 px everywhere to avoid even slight aligning problems, but more dense for bigger symbols (jewish, muslim), 18 px with exactly opposite properties or maybe something else? Also what about weight - maybe we should just use lighter shade of green?

@pnorman
Copy link
Collaborator

pnorman commented Aug 1, 2017

I'm inclined towards a 32px interval and keeping the visual weight of a single symbol the same. I've long felt the pattern is too dense, and that's feedback I often get from others.

If we wanted something closer to what we have right now, we could go with 7 symbols in a 128x128 grid, and a spacing of 18px between 6 of them. The other and where it loops around would then be 17px.

|  x 18 x 18 x 18 x 17 x 18 x 18 x |  17 x ...

is kinda what it would look like, with | being the 128px bounds, and the numbers being the distance between symbols, which are x.

But 32px is much easier.

@kocio-pl
Copy link
Collaborator Author

kocio-pl commented Aug 1, 2017

keeping the visual weight of a single symbol the same

If I understand you, it means probably that we have to use different shades of green, because christian symbol is pretty much the same, but jewish one is darker. I would rather use lighter shade for all of them.

@pnorman
Copy link
Collaborator

pnorman commented Aug 1, 2017

If I understand you, it means probably that we have to use different shades of green, because christian symbol is pretty much the same, but jewish one is darker. I would rather use lighter shade for all of them.

Keep the visual weight of the symbol the same as the existing symbols.

@kocio-pl
Copy link
Collaborator Author

kocio-pl commented Aug 1, 2017

I would go with 32px version for all the cemeteries. If there are people who think it's already too dense, that's the best solution and we don't have to cheat with spacing, which makes the pattern simple. It also gives us more room to balance the size of the symbols.

Visual weight is related to the color. shape, size and bluriness, so I would start with size (shape is beyond our control, bluriness also) and we might change the shade of green as the last step to make them less obtrusive. So I've made a cross bigger to match the size of more complicated symbols more or less (12x9 vs 10x9). They could be also make larger a bit if needed (12x11). What do you think now?

Before
bui1whb3
After (not touching symbols)
ob_fsdzw
After (making cross bigger)
rxbt0agn
After (making all symbols bigger, with h/w=12 px)
e8 j250z

@kocio-pl kocio-pl changed the title Adding rendering for muslim cemeteries Cemeteries rework in SVG Aug 1, 2017
@kocio-pl
Copy link
Collaborator Author

kocio-pl commented Aug 1, 2017

I like 32 px matrix with bigger symbols (the last rendering). For the completeness - generic symbols rendering:

Before
mb5thlmp
After (making all symbols bigger, with h/w=12 px)
cjhdssp3

@pnorman
Copy link
Collaborator

pnorman commented Aug 2, 2017

So I've made a cross bigger to match the size of more complicated symbols more or less (12x9 vs 10x9). They could be also make larger a bit if needed (12x11). What do you think now?

I've had success with the existing size at 32px spacing, using https://github.com/kartotherian/brighmed/blob/master/symbols/patterns/cemetery.svg to get

image

That's a different colour scheme, of course, but I like the overall balance of it.

@kocio-pl
Copy link
Collaborator Author

kocio-pl commented Aug 3, 2017

But what about different symbols? Do we accept the fact that christian/generic symbols would be small and jewish/muslim would be big or maybe also small but harder to recognize?

@pnorman
Copy link
Collaborator

pnorman commented Aug 3, 2017

I'd go for not changing the symbols for now. Right now the symbols are changed

image

@kocio-pl
Copy link
Collaborator Author

kocio-pl commented Aug 3, 2017

I have no idea how these PNGs were made and rasterization would be made by Mapnik now, so of course exact recreation is not possible.

Just to make sure - you'd like a version from this comment?:

After (not touching symbols)
ob_fsdzw

@kocio-pl
Copy link
Collaborator Author

kocio-pl commented Aug 7, 2017

It looks like jewish symbol for cemeteries was simply imported from previous (XML-based) style implementation as cemetery_jewish.18.png in 2012 and later edited into grave_yard_jewish.png by removing the background in 2014.

Generic jewish symbol was recreated in SVG by @nebulon42 as jewish.svg and while it looks similar to jewish3.p.16.png on the GitHub, its rendering done by Mapnik looks quite different (it has less details and is bolder). It's the same problem as you see with my try to use this vector shape for cemeteries.

So the problem exists probably for both jewish symbols files (generic and cemetery), but we just haven't noticed the change for a generic jewish symbol before. We have no control over Mapnik rasterization process and it doesn't look like the problem with our SVGs (see also #2727 (comment)).

I think the most important thing is that cemetery type should be clear, not recreating the same image.

@pnorman
Copy link
Collaborator

pnorman commented Aug 8, 2017

I think the most important thing is that cemetery type should be clear, not recreating the same image.

I find the previous images clearer. Given that the previous images work better, I'd rather stick with them.

@kocio-pl
Copy link
Collaborator Author

kocio-pl commented Aug 8, 2017

OK, I can leave jewish symbol for cemeteries as PNG (and just make it fit 32 px matrix). With other symbols (generic, muslim and christian) I see no such problem, so I will recreate them in SVG.

@kocio-pl
Copy link
Collaborator Author

kocio-pl commented Aug 9, 2017

32 px matrix for all, jewish cemetery symbol left as PNG.

zth1gzy6
j1ob cex

@kocio-pl kocio-pl merged commit d4408e6 into gravitystorm:master Aug 11, 2017
@kocio-pl kocio-pl deleted the cemetery-muslim branch August 11, 2017 10:49
@Vort
Copy link

Vort commented Aug 12, 2017

Why default symbol is not symmetric anymore?
tomb

@kocio-pl
Copy link
Collaborator Author

I don't know, SVG is symmetric:
zrzut ekranu z 2017-08-12 15-35-38

@Vort
Copy link

Vort commented Aug 12, 2017

tomb2

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.

3 participants