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

Uniformize swamp rendering with forest/wood #3051

Merged
merged 5 commits into from
Mar 6, 2019

Conversation

Penegal
Copy link
Contributor

@Penegal Penegal commented Feb 1, 2018

This is my first PR, which I hope is good enough; it fixes #1920 using @imagico's pattern for wetland=swamp, which uniformizes the tree icons with the ones of natural=wood and landuse=forest.

Before:
image

After:
image

P.-S.: I closed a first PR for that, it had an authoring problem; it can be safely deleted, if relevant.

@kocio-pl
Copy link
Collaborator

kocio-pl commented Feb 1, 2018

Great, I hope I will test it soon!

Two general questions:

  1. Do we want to show different leaf types? I haven't found such tags in combination with swamp, so we may not bother and add it later if it ever start to be popular enough.

  2. How did you made the images? Please document them if needed (I guess this was made with a general instruction, but I want to make sure) and remove the old image if not needed elsewhere.

You don't have to squeeze the commits, because maintainers can do it easily now while merging.

@Penegal
Copy link
Contributor Author

Penegal commented Feb 1, 2018

Well, I merely added the images @imagico made, so I'm unsure about your questions, but, naively, I would say:

  1. let's not bother about that if there is no data, and
  2. @imagico did them, most probably using his jsdotpattern; what the parameters were, however, I can't say.

@kocio-pl
Copy link
Collaborator

kocio-pl commented Feb 1, 2018

I meant how did you combine them if this is not already documented.

OK, so let just remove the old image if not needed (I guess it was used only for producing swamp image).

@imagico
Copy link
Collaborator

imagico commented Feb 1, 2018

Assembling the pattern image is explained on

https://github.com/gravitystorm/openstreetmap-carto/blob/master/symbols/generating_patterns/wetland.md

The change in symbol can be done on the SVG level, no need to re-generate the base pattern.

@kocio-pl
Copy link
Collaborator

kocio-pl commented Feb 1, 2018

OK, that's what I suspected. Any other comments or is it OK for you?

@kocio-pl
Copy link
Collaborator

kocio-pl commented Feb 1, 2018

The change in symbol can be done on the SVG level, no need to re-generate the base pattern.

Do you mean producing SVG image (or what else)?

@imagico
Copy link
Collaborator

imagico commented Feb 1, 2018

The change in symbol can be done on the SVG level, no need to re-generate the base pattern.

Do you mean producing SVG image (or what else)?

I meant replacing the symbol in the SVG code in

https://raw.githubusercontent.com/gravitystorm/openstreetmap-carto/master/symbols/generating_patterns/swamp.svg

with a different symbol.

@kocio-pl
Copy link
Collaborator

kocio-pl commented Feb 6, 2018

@Penegal Do you plan to fix this?

@Penegal
Copy link
Contributor Author

Penegal commented Feb 8, 2018

Don't read this with an ironic tone, it is not my intent: as you 2 were talking on your own, I thought you were no longer waiting my contribution. I'll try, then, once I will have found the necessary symbols and the parameters of your pattern generation code.

@Penegal
Copy link
Contributor Author

Penegal commented Feb 8, 2018

Upon reflection, I don't understand how I can use the pattern generation code with 2 symbols; the .md file for wetland doesn't help, as it describes the generation of a bitmap using ImageMagick, not a SVG. I could create the SVG myself, but there must be some semi-automated way to do it, isn't it?

@imagico
Copy link
Collaborator

imagico commented Feb 9, 2018

The description in

https://github.com/gravitystorm/openstreetmap-carto/blob/master/symbols/generating_patterns/wetland.md

describes in a generic way how to produce the composite wetland patterns from the generic wetland svg

https://raw.githubusercontent.com/gravitystorm/openstreetmap-carto/master/symbols/generating_patterns/wetland.svg

and the specific pattern svg (named pattern.svg in the document), in this case

https://raw.githubusercontent.com/gravitystorm/openstreetmap-carto/master/symbols/generating_patterns/swamp.svg

and the two colors $SYMBOL and $WETLAND.

This will allow you to re-create the current swamp png from the svg files.

To generate a new pattern with different symbols you do the same - but with a different pattern.svg where you replace the symbol SVG code (which you can find in the defs section on top) with a new symbol code. This requires some knowledge of the svg format of course.

@kocio-pl
Copy link
Collaborator

kocio-pl commented Feb 10, 2018

Don't read this with an ironic tone, it is not my intent: as you 2 were talking on your own, I thought you were no longer waiting my contribution.

We have the open, public discussions here and this is very typical to nitpick all the details from all the possible points of view before merging. Not only your contribution is welcome, but also it looks like quite close to being accepted.

@Penegal
Copy link
Contributor Author

Penegal commented Feb 12, 2018

@imagico: I followed the instructions, retrieved the colors from already existing patterns and used symbols/leaftype_unknown.svg as pattern.svg, but it still generated a PNG pattern, not a SVG one.

There must be something I'm missing here: AFAIK, convert, as all ImageMagick commands, can only generate bitmap images. How could it be that I should use it to generate an SVG pattern? What am I missing here?

Now, I could ask someone else to do it, but, now I'm on it, I would like to be able to understand and complete the task, as asking somebody else won't learn me anything.

@kocio-pl: OK, thanks for the note.

@imagico
Copy link
Collaborator

imagico commented Feb 12, 2018

The wetland patterns are generated as PNG. You could try implementing a similar process on SVG level but given the limited scripting abilities of Inkscape and the lack of any other tools that allow similar processing in an automated fashion makes this difficult.

And considering Mapnik's SVG pattern rendering is broken the practical usefulness of this would be rather limited.

@Penegal
Copy link
Contributor Author

Penegal commented Feb 13, 2018

Oh, I think I see my error: I thought I had to generate the whole swamp pattern in SVG, but only had to regenerate symbols/generating_patterns/swamp.svg and create my PNG from it, right? Instead, I followed the instructions for wetland pattern generation, using symbols/leaftype_unknown.svg.

Could it be OK to simly use this 256256 px pattern 4 times to generate the 512512 px symbols/generating_patterns/swamp.svg?

@imagico
Copy link
Collaborator

imagico commented Feb 14, 2018

The 512x512 pixel pattern size for wetlands was chosen because of the difficulty of creating a proper random relaxed pattern of this density at a smaller size. Apart from that there is no reason why you cannot use a pattern of 256x256 pixel.

@Penegal
Copy link
Contributor Author

Penegal commented Feb 14, 2018

Then I'll try to create the 512*512 px manually.

@Penegal
Copy link
Contributor Author

Penegal commented Feb 17, 2018

And here you are! Is it OK now? BTW, FWIW
image

… I used jsdotpattern this way:

  • selected tree pair icon;
  • selected 512*512 image;
  • typed a distance of 25, then clicked generate;
  • clicked relax ×10.

@kocio-pl
Copy link
Collaborator

Please have a look at the files, looks like something is wrong with both images - right and bottom parts have unexpected margin space with no tree pattern.

@Penegal
Copy link
Contributor Author

Penegal commented Feb 21, 2018

Wow, I missed that! Strange… I'll check it; thanks for noticing.

@kocio-pl
Copy link
Collaborator

It also looks like wetland_swamp@2x.png is darker than wetland_swamp.png and I guess they shouldn't be different visually.

@Penegal
Copy link
Contributor Author

Penegal commented Feb 21, 2018

That must be from the interpolation used when doubling the resolution, because I did nothing else.

@kocio-pl
Copy link
Collaborator

Could you try the same procedure with current versions to see if this problem will appear?

@kocio-pl
Copy link
Collaborator

kocio-pl commented Mar 1, 2018

@Penegal What is current state of this code, do you think you can fix it?

@Penegal
Copy link
Contributor Author

Penegal commented Mar 5, 2018

Kept that in mind but had no time to correct; I'll try to fix it in the next days.

@Penegal
Copy link
Contributor Author

Penegal commented Mar 5, 2018

Well, I don't understand why, but the pattern_casing.png file is truncated to 481×481 px, whereas the swamp.svg file is 512×512 px with no empty borders; here is pattern_casing.png:
pattern_casing

My knowledge about ImageMagick tools is near zero; do you see where the problem could lie?

@Penegal
Copy link
Contributor Author

Penegal commented Mar 9, 2018

Is there someone with ImageMagick knowledge who could say why the first convert call shrinked the image, or, if normal, why it affects subsequent images when it shouldn't? I assume the generated pattern_casing.png mask should not be shrinked to 481×481 px.

Edit: I can get round it by manually resizing pattern_casing.png to 512×512 px, but, later, that doesn't prevent pattern_col.png to be 512×512 px, but show the pattern only on 484×484 px:
image

Because the same problem arises at two different points of the process, and with two different pixel counts, there seems to be two problems in the command sequence in https://github.com/gravitystorm/openstreetmap-carto/blob/master/symbols/generating_patterns/wetland.md As I use the ImageMagick version supplied with Debian 9 stable, it is likely that I'm not the only one affected by this bug, so it could be very useful that someone with good knowledge of ImageMagick take a look at this problem to help me solve it.

@Penegal
Copy link
Contributor Author

Penegal commented Feb 19, 2019

@imagico: if I understand correctly, your Inkscape calls only replace the convert -depth 8 -density 90 wetland.svg wetland_tile.png call with inkscape -z --export-png=wetland_tile.png --export-dpi=90 --export-background=white wetland.svg — this is the command that I understood, from your Python code, I should use —; if so, I still have the truncated wetland_tile.png pattern I got the last times. Besides, that doesn't prevent the erroneous final wetland_pattern.png to be generated once more.

If your configuration allows you to correctly generate the patterns, would you simply generate them and then you or I would include them in the PR and that's all? We wouldn't get the cause of these bugs, but they would have been circumvented and the new patterns could be put in production. This PR has been opened months ago, and you seem the only one to be able to generate the patterns correctly. Would you generate them?

@imagico
Copy link
Collaborator

imagico commented Feb 20, 2019

Of course i can generate a pattern image for this but for the goal of adaptability and maintainability there should be a working process to create those to allow people adjusting the pattern - like for using different symbols or a different color.

I would gladly help you tracking down the problem you have but since i can't reproduce it i cannot really do much about it actively.

A useful starting point could be to run the pattern image generation on the alternative-colors style and see what the results are. Do they differ between using Imagemagick or Inkscape? Are all patterns produced incorrectly or just some?

Another starting point would be to open the pattern SVG in Inkscape interactively and then try to export it as PNG. Does it correctly export the image then? With the right extent and the right size? The inkscape command should generate the same as you get when you interactively export the image from inkscape.

@Penegal
Copy link
Contributor Author

Penegal commented Feb 20, 2019

Well, I think I found something explaining the downsizing of wetland_tile.png: wetland.svg has a density of 96 ppp; in Inkscape, when I try to change the density to 90 ppp, the size changes from 256256 px to 240240 px. I don't know how the SVG gives 96 ppp for density, but it seems that this difference explains the problem.

Regarding the swamp.svg downsizing, I tried to regenerate it using your online version of jsdotpattern, and, even if it is generated with a 512*512 px size specified in the SVG code, it get resized, but, if I try to export it using a 96 ppp density, the exported size miraculously increases to 512 px. Definitely a density-related bug here, but why? I'll try to get a more recent version of imagemagick, if I can install it without breaking my distro.

Edit: what's the matter with density anyway? It seems that the sequence of commands only take care of pixel size, not physical size; if so, we could just let imagemagick take care of density and see what happens. I'll also try that.

@Penegal
Copy link
Contributor Author

Penegal commented Feb 20, 2019

Gah! Tried to remove density-related arguments for patter_casing.png generation:
pattern_casing

That would need messing with other arguments, which is way too much for my current imagemagick knowledge.

@Penegal
Copy link
Contributor Author

Penegal commented Feb 20, 2019

Ok, I tried with the testing version of ImageMagick (6.9.10-23), and here is what I got: pattern_casing.png is still 481*481 px, which breaks subsequent operations:
image

If you look closely, you will see that room was made in the wetland pattern for the tree pairs, but the holes are slightly too little and too close of each others for the tree pairs to fit in. This version of ImageMagick correctly interprets the second-to-last convert command, but still lacks a correct understanding of the first convert, hence the 481*481 px pattern_casing.png.

@Penegal
Copy link
Contributor Author

Penegal commented Feb 20, 2019

I also tried to upgrade rsvg-convert to 2.44.10-1, as it is called for SVG operations by ImageMagick, but that did not changed the problem.

@imagico
Copy link
Collaborator

imagico commented Feb 20, 2019

As i have written above in #3051 (comment) for the wetland pattern process you need both the native resolution version and one with eight times the linear resolution for the casing processing.

If rasterization for you without a specified resolution leads to an incorrect output resolution that is a bug in the program in the version used (presumably resulting in use of a different assumed resolution for converting SVG dimensionless units into real world units and converting back into pixel). But since both imagemagick and inkscape allow specifying an output resolution you can compensate for that.

So you'd need to rasterize once at a resolution that matches the assumed resolution for converting between pixel and real world units and another time at eight times that resolution (which would be 768 ppi if the assumed native resolution is 96 ppi as you indicate) for the casing which is after processing scaled back to 12.5% which is the same size again.

@Penegal
Copy link
Contributor Author

Penegal commented Feb 20, 2019

To explain my difficulty to understand you: I'm a n00b regarding ImageMagick and such advanced operations, as I am with bitmap manipulation, density and so on. Furthermore, I'm returning on this issue after months, so I totally forgot what was going on for this PR. That is to say: I'm trying to follow your pace, really, but I'm feeling totally lost with these densities, zooms and resolutions, trying to correct problems by trial-and-error on operations which exceed by far my level of knowledge, basically tinkering, so please forgive me not understanding your points: that only means that I'm lost and do not see what you're trying to show me, not that I'm stubbornly refusing to understand nor that you do not correctly explain. Please, drink up a full bowl of patience, I think we'll both need it if I am to solve this problem.

Here is the sequence of commands I used this time:

  • convert -density 768 swamp.svg -morphology Erode Disk:5.3 \( +clone -fill black -draw 'color 0,0 floodfill' -negate \) +swap -morphology Erode Disk:10.3 -compose Darken -composite -scale 12.5% -depth 8 pattern_casing.png
  • convert -depth 8 -density 90 wetland.svg wetland_tile.png
  • montage wetland_tile.png wetland_tile.png wetland_tile.png wetland_tile.png -geometry 256x256+0+0 wetland_512.png
  • convert wetland_512.png \( pattern_casing.png -negate \) -compose Lighten -composite -threshold 50% \( +clone -negate -morphology hitandmiss peaks:1.9 \) -compose Lighten -composite +level 20%,100% wetland_pattern_bkg.png
  • convert -depth 8 -size 512x512 xc:"#93b685" \( swamp.svg -negate \) -set colorspace RGB -alpha Off -compose CopyOpacity -composite pattern_col.png
  • convert -depth 8 -size 512x512 xc:"#4aa5fa" \( wetland_pattern_bkg.png -negate \) -set colorspace RGB -alpha Off -compose CopyOpacity -composite +compose pattern_col.png -compose Over -composite wetland_pattern.png

Here is what I got
image

As you can see, the holes in the wetland pattern and the tree pairs are still misaligned, but the offset is now very small on the whole extent of the bitmap. If I display the generated pattern on top of pattern_casing.png, I get this:
image
image

As you can see, there is still a density offset, but very small, something like 1%. That's far of the 6% of density mismatch, so I don't really understand where that does come from.

@imagico
Copy link
Collaborator

imagico commented Feb 20, 2019

I understand. But please keep in mind that i cannot verify if what you do is working or not and if not why because the same commands for me produce different results.

If i understand your description correctly every command that processes an SVG for you apparently will pecified to prneed the density soduce correct results. That is the first, second and fifth in your sequence. Presumably that is -density 96 for the second and fifth and -density 768 for the first.

If that does not work you should check if all the intermediate images are the correct size (that is 512x512 pixel for all except wetland_tile.png which is 256x256 pixel).

If that is not the case that would mean imagemagick does not show consistent results for you at all and you could try using inkscape for rasterization using something like

  • inkscape -z --export-png=wetland.png --export-dpi=96 --export-background=white wetland.svg
  • inkscape -z --export-png=swamp.png --export-dpi=96 --export-background=white swamp.svg
  • inkscape -z --export-png=swamp_hr.png --export-dpi=768 --export-background=white swamp.svg

And then use wetland.png/swamp.png/swamp_hr.png in the process instead of using the SVGs. You might need to use a different resolution value with inkscape than with imagemagick because the mismatch in assumed default resolutions might not be the same. The sizes of the resulting images should be 256x256 pixel for wetland.png, 512x512 for swamp.png and 4096x4096 for swamp_hr.png.

@Penegal
Copy link
Contributor Author

Penegal commented Feb 21, 2019

If i understand your description correctly every command that processes an SVG for you apparently will pecified to prneed the density soduce correct results. That is the first, second and fifth in your sequence. Presumably that is -density 96 for the second and fifth and -density 768 for the first.

Using -density 96 instead of -size 512x512 for the fifth command generates a 1*1 px pattern_col.png; using -density 96 -size 512x512 doesn't change the final result.

If that does not work you should check if all the intermediate images are the correct size (that is 512x512 pixel for all except wetland_tile.png which is 256x256 pixel).

With convert -depth 8 -density 96 wetland.svg wetland_tile.png, wetland_tile.png is 259x259 px, but I can get it with 256*256 px with inkscape -z --export-png=wetland_tile.png --export-dpi=96 --export-background=white wetland.svg. Then, the only discrepancy with file sizes is that patter_casing.png is 513x513 px.

Comparing the different intermediate images, it seems that the main problem left is the first command, generating, pattern_casing.png, as it doesn't match swamp.svg and comparing them give the same offset that bothers me on the final wetland_pattern.png.

Now, here are the good news: it seems I finally got a good result by starting to generate a rasterized version of swamp.svg, then using it instead of the SVG. That gives the following chain of commands:

inkscape -z --export-png=swamp.png --export-dpi=96 --export-background=white swamp.svg
convert -scale 800% swamp.png -morphology Erode Disk:5.3 \( +clone -fill black -draw 'color 0,0 floodfill' -negate \) +swap -morphology Erode Disk:10.3 -compose Darken -composite -scale 12.5% -depth 8 pattern_casing.png
inkscape -z --export-png=wetland_tile.png --export-dpi=96 --export-background=white wetland.svg
montage wetland_tile.png wetland_tile.png wetland_tile.png wetland_tile.png -geometry 256x256+0+0 wetland_512.png
convert wetland_512.png \( pattern_casing.png -negate \) -compose Lighten -composite -threshold 50% \( +clone -negate -morphology hitandmiss peaks:1.9 \) -compose Lighten -composite +level 20%,100% wetland_pattern_bkg.png
convert -depth 8 -size 512x512 xc:"#93b685" \( swamp.png -negate \) -set colorspace RGB -alpha Off -compose CopyOpacity -composite pattern_col.png
convert -depth 8 -size 512x512 xc:"#4aa5fa" \( wetland_pattern_bkg.png -negate \) -set colorspace RGB -alpha Off -compose CopyOpacity -composite +compose pattern_col.png -compose Over -composite wetland_pattern.png

This sequence gives me the following result:
image

It seems that there is a bit of transparency on the pattern elements, but perhaps this is normal. Is that pattern OK?

Edit: comparing with some current wetland patterns, for instance reedbed, it also have this transparency, so I assume it is OK.

@imagico
Copy link
Collaborator

imagico commented Feb 21, 2019

convert -scale 800% swamp.png -morphology Erode Disk:5.3 \( +clone -fill black -draw 'color 0,0 floodfill' -negate \) +swap -morphology Erode Disk:10.3 -compose Darken -composite -scale 12.5% -depth 8 pattern_casing.png

That will work in some way but blowing up the PNG image to 800% is pretty pointless. As said to use the original process you'd need to rasterize the SVG twice - at native resolution and at eight times the native resolution. So that would probably mean

  • inkscape -z --export-png=swamp_hr.png --export-dpi=768 --export-background=white swamp.svg
  • convert swamp_hr.png -morphology Erode Disk:5.3 \( +clone -fill black -draw 'color 0,0 floodfill' -negate \) +swap -morphology Erode Disk:10.3 -compose Darken -composite -scale 12.5% -depth 8 pattern_casing.png

It seems that there is a bit of transparency on the pattern elements, but perhaps this is normal. Is that pattern OK?

I don't know. That depends on how you want the pattern to look like in the map. If you unintentially get semi-transparent symbols that is probably because the SVG you use is not black and white. The one in your PR is but maybe you are working with a different one now.

@Penegal
Copy link
Contributor Author

Penegal commented Feb 21, 2019

That will work in some way but blowing up the PNG image to 800% is pretty pointless. As said to use the original process you'd need to rasterize the SVG twice - at native resolution and at eight times the native resolution. So that would probably mean

* `inkscape -z --export-png=swamp_hr.png --export-dpi=768 --export-background=white swamp.svg`

* `convert swamp_hr.png -morphology Erode Disk:5.3 \( +clone -fill black -draw 'color 0,0 floodfill' -negate \) +swap -morphology Erode Disk:10.3 -compose Darken -composite -scale 12.5% -depth 8 pattern_casing.png`

I included that in the commands sequence, and that gives me this sequence:

inkscape -z --export-png=swamp.png --export-dpi=96 --export-background=white swamp.svg
inkscape -z --export-png=swamp_hr.png --export-dpi=768 --export-background=white swamp.svg
convert swamp_hr.png -morphology Erode Disk:5.3 \( +clone -fill black -draw 'color 0,0 floodfill' -negate \) +swap -morphology Erode Disk:10.3 -compose Darken -composite -scale 12.5% -depth 8 pattern_casing.png
inkscape -z --export-png=wetland_tile.png --export-dpi=96 --export-background=white wetland.svg
montage wetland_tile.png wetland_tile.png wetland_tile.png wetland_tile.png -geometry 256x256+0+0 wetland_512.png
convert wetland_512.png \( pattern_casing.png -negate \) -compose Lighten -composite -threshold 50% \( +clone -negate -morphology hitandmiss peaks:1.9 \) -compose Lighten -composite +level 20%,100% wetland_pattern_bkg.png
convert -depth 8 -size 512x512 xc:"#93b685" \( swamp.png -negate \) -set colorspace RGB -alpha Off -compose CopyOpacity -composite pattern_col.png
convert -depth 8 -size 512x512 xc:"#4aa5fa" \( wetland_pattern_bkg.png -negate \) -set colorspace RGB -alpha Off -compose CopyOpacity -composite +compose pattern_col.png -compose Over -composite wetland_pattern.png

This sequence works for me; should I include it in wetland.md, as a replacement in the case the reader encounters the same problem as me, and then carry on to finishing this PR?

I don't know. That depends on how you want the pattern to look like in the map. If you unintentially get semi-transparent symbols that is probably because the SVG you use is not black and white. The one in your PR is but maybe you are working with a different one now.

In fact, the current online version of the wetland patterns already are semi-transparent; that is noticeable, for example, by examining the blue dashes against the transparent grid background of Gimp. I assume this means the semi-transparency is normal and that I can carry on.

@imagico
Copy link
Collaborator

imagico commented Feb 21, 2019

This sequence works for me; should I include it in wetland.md, as a replacement in the case the reader encounters the same problem as me, and then carry on to finishing this PR?

You can but you should indicate that this won't work universally - for me the first command for example results in a 546x546 pixel image.

In fact, the current online version of the wetland patterns already are semi-transparent; that is noticeable, for example, by examining the blue dashes against the transparent grid background of Gimp. I assume this means the semi-transparency is normal and that I can carry on.

The blue dash pattern transparency comes from a different source, that is done with the +level 20%,100%.

The process gives you full control over color and opacity of the symbols. swamp.png and wetland_pattern_bkg.png essentially serve as alpha masks for the final pattern image compositioning.

@Penegal
Copy link
Contributor Author

Penegal commented Feb 21, 2019

Wow. I used the new pattern in Kosmtik to test it, but ran into a strange issue:
image

This is an altered screenshot: the upper left corner uses the new pattern as rendered by Kosmtik: as you can see, it seems somewhat faded. On the lower right, I altered the screenshot to directly superimpose the new pattern; as you can see, it renders as one should expect, without fading. I'm rather puzzled here, as the style doesn't seem to add transparency to the pattern. What could have happened?

Edit: tried after merging from upstream/master, but that did not change anything.

@imagico
Copy link
Collaborator

imagico commented Feb 21, 2019

You need an additional +gamma - -strip in the last command to get rid of wrong metadata.

But more importantly your pattern is not pixel aligned - if you produced it with jsdotpattern you need to use rd,1 instead of rd,0. But as indicated above it would be better to use one of the other symmetric wetland patterns (like reed or marsh) and just replace the symbol. This way the pattern arrangement would be identical.

@Penegal
Copy link
Contributor Author

Penegal commented Feb 22, 2019

Wow! You magician 😍
image

Seems good to merge, now.

@Penegal Penegal changed the title [WIP] Uniformize swamp rendering with forest/wood Uniformize swamp rendering with forest/wood Feb 27, 2019
@Penegal
Copy link
Contributor Author

Penegal commented Mar 4, 2019

Is there a problem preventing this to be merged?

@imagico
Copy link
Collaborator

imagico commented Mar 4, 2019

My comments on the change as it is now:

  • since the change is meant to Uniformize swamp rendering with forest/wood i am wondering about the symbol color. It is different from the previous one and different from the forest symbol color.
  • the tree pair symbol is for forest/wood used to indicate areas lacking a leaf_type tag. But for swamp the same symbol is used universally. I am not sure if this would work out nicely. A mapper just tagging natural=wood and wetland=swamp will get consistent symbols but a mapper tagging more specifically with leaf_type will get different rendering for wood and swamp.

I am not opposed to the change as is and not using a different symbol for swamp but i am wondering about the overall strategy.

@Penegal
Copy link
Contributor Author

Penegal commented Mar 5, 2019

* since the change is meant to _Uniformize swamp rendering with forest/wood_ i am wondering about the symbol color.  It is different from the previous one and different from the forest symbol color.

Different? I compared the final rendered colors of the tree icons in the new swamp symbol and the current forest symbols, and they are rendered the same: 93b685. You can see, at a limit between the two symbols, that they have the same color:
image

* the tree pair symbol is for forest/wood used to indicate areas lacking a leaf_type tag.  But for swamp the same symbol is used universally.  I am not sure if this would work out nicely.  A mapper just tagging `natural=wood` and `wetland=swamp` will get consistent symbols but a mapper tagging more specifically with leaf_type will get different rendering for wood and swamp.

When I asked about updating this symbol, SK53 warned about boreal swamps which are mainly coniferous, hence keeping the two symbols. Or maybe you mean that the new swamp symbol should take leaf_type into account? Combining leaf_type with wetland=swamp is too rare to be accounted by Taginfo; if present, this combination has less than a thousand occurrences. I don't think it would be relevant to take such a rare combination into account.

@imagico
Copy link
Collaborator

imagico commented Mar 6, 2019

Regarding color - i am sorry about that, it looked from your example like the color is different but that might just be because the wetland dashing around creates that impression.

Regarding the symbol - yes, the logical step if your goal is to uniformize swamp rendering with forest/wood would be to show leaf_type in the same fashion. If the aim is consistency to normal woodland low use numbers are not a significant factor since despite low use volume this is an undisputed tagging. But you don't have to do that in this PR. I was just asking for the overall strategy here.

@imagico imagico merged commit 57d2fe3 into gravitystorm:master Mar 6, 2019
@imagico
Copy link
Collaborator

imagico commented Mar 6, 2019

Looks good, thanks.

@Penegal
Copy link
Contributor Author

Penegal commented Jun 20, 2019

More than 3 months since the merge, and it's still not online ? I thought the server problems were solved.

@matkoniecz
Copy link
Contributor

See openstreetmap/chef#235

Reviewing whatever v4.21.1 still has regression bugs would allow recommending it for release.

@Penegal
Copy link
Contributor Author

Penegal commented Jun 21, 2019

@matkoniecz: Ok, thanks for the link!

@jeisenbe
Copy link
Collaborator

Reviewing whatever v4.21.1 still has regression bugs would allow recommending it for release.

It seems the bugs have all been fixed. Should we consider releasing v4.21.1 or v4.22 soon?

@pitdicker pitdicker mentioned this pull request Sep 9, 2021
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.

Standardize pictos of wood/forest and swamp/mangrove
8 participants