Dummy geometry for scrub pattern compatibility

[sommerluk]

The scrub pattern crosses the border horizontally, but not vertically. This PR passes the existing scrub pattern through the newest version of the sanitizer of jsdotpattern, so it gets a dummy geometry added (just to make sure that we work around Mapnik limitations)…

[sommerluk]

Wrong screenshot, sorry. Substituted with the good one.

[kocio-pl]

I believe we should go with 256 px for all the patterns now (instead of 512 px).

[sommerluk]

Re-generated for 256 px.

Before:

After:

[kocio-pl]

It has a bit more space but I don't feel this is a problem.

[matthijsmelissen]

Looks good to me. @imagico are there technical reasons why we should hold this off?

[imagico]

Not sure what i can add here.

• this obviously fixes the periodicity problem.
• this also switches from a relaxed random pattern to a regular snub square grid - a choice which has not been discussed so far.
• it obviously does not solve SVG patterns in landcover-area-symbols render inconsistently in different metatiles #2750.

[kocio-pl]

What do you suggest to do with it? Basic options would be to:

1. Leave the current SVG pattern as it is.
2. Revert to PNG version.
3. Merge this SVG version
   a) as it is
   b) tune it more (how?)

Probably the same decision list applies to #2760 and other SVG patterns as well.

[sommerluk]

The description at scrub.md was about a snub square pattern yet before this PR. During developping this PR, I’ve tried to generate with the instructions at scrub.md the very same pattern again, but this wasn’t possible. The result of jsdotpattern (using the instrunctions from scrub.md) was quite different from the previous pattern. Maybe the original description at scrub.md was not up to date? Anyway, I’ve played around with the other settings then – until I got something more or less similar to the previeous pattern. 2017-08-23 14:48 GMT, kocio-pl <notifications@github.com>:

…

What do you suggest to do with it? Basic options would be to: 1. Leave the current SVG pattern as it is. 2. Revert to PNG version. 3. Merge this SVG version a) as it is b) tune it more (how?) Probably the same decision list applies to #2760 and other SVG patterns as well. -- You are receiving this because you authored the thread. Reply to this email directly or view it on GitHub: #2748 (comment)

-- Lukas Sommer

[imagico]

I would normally suggest to discuss and decide on the matter of relaxed random patterns and regular patterns in general before making changes along these lines - but given the experience with discussing general design principles and guidelines in #2462 i am not sure if this is possible. In any case someone should make a conscious decision about this and i am not sure if @sommerluk did.

And yes, until #2750 is solved i would suggest to stick to PNG patterns for anything with transparency or partial pixel coverage.

[sommerluk]

Well, as I said, scrub.md had documented this as snub pattern yet before this PR, so this was not a conscious decision.

If desired, I could try to create another pattern that is randomized.

[imagico]

I have reconstructed more or less the original command sequence based on the description by @meased (see #2395):

http://www.imagico.de/map/jsdotpattern.php#x,512,jdp97762;g5,45,64,64;rx,250,2,64,64;rx,250,2,64,64;rx,250,2,64,64;rd,1,0,0,scrub2,1,5,12,0,jdp3379,8ece8f,b5e3b5;

This cannot be directly translated to 256 pixel size because the relaxation dynamics are different then. You can still generate a relaxed random pattern at this size and density but it would not be very good - see also #937 (comment).

[kocio-pl]

I would normally suggest to discuss and decide on the matter of relaxed random patterns and regular patterns in general before making changes along these lines - but given the experience with discussing general design principles and guidelines in #2462 i am not sure if this is possible.

I'm more optimistic with that, because the scope would be much smaller.

[kocio-pl]

@sommerluk The problem with patterns looks quite complex (involving subtle Mapnik bugs and metatile issues), so the question is what do you plan to do with this code? For me the pattern is quite irregular (as needed for natural features) and works with 256px. Converting it to PNG would be safer, but I'm not sure if such precautions are really needed in this case.

[sommerluk]

I would propose doing things in this order:

1. Decide if all patterns should 256 px maximum
2. If it should be 256 px, I could try something based on the reconstructed pattern instructions to see it it works, otherwise I could take the reconstructed pattern instructions as is. (If desired I could also take the current pattern, but as it’s not randomized, not sure…?)
3. Convert the pattern to PNG to avoid rendering problems
4. Documentation: Update scrub.md so that it documents the original pattern generating instructions. Have also the origininal SVG file, from which the PNG was rendered, available.
5. Merge 😉

I could take a look at steps 2, 3 and 4, but I would like to wait until there is a decision about step 1.

[kocio-pl]

It doesn't look like we have any pattern related decisions coming any time soon and I don't have an opinion what to do with them, so I will leave it to you.

[sommerluk]

The easiest solution for now: The pattern as reconstructed by @imagico just converted to PNG, with documentation. (And also a minor documentation fix for leaftype.) There are currently more patterns > 256 px anyway. If later we decide to switch to patterns ≤ 256 px, we have documentation available about the pattern generation for scrub.

[kocio-pl]

I think that leaftype documentation fix doesn't belong here, so please open another PR for it.

[sommerluk]

Now without documentation fix.

[kocio-pl]

If you feel it's OK, you can merge it yourself now in my opinion, since enough time has passed and nobody else is ready to do that (including me).

[sommerluk]

There are three merge options (Create…, Squash…, Rebase…). Which one should I use preferred?

[kocio-pl]

You don't have to even use pull down menu, just click "Merge pull request" button in standard cases.