Switch vineyard to SVG

[sommerluk]

Switch vineyard to SVG.

Before:

After:

[imagico]

I would suggest you increase the vertical gap between the symbol from 1/3 to 2/3 pixel - this way you would avoid a visual overlap between the lines in vertical direction.

However you would also more prominently see the problem from #2727:

left is with polygon-gamma: 1; as per point 4 in #2727 (comment), right as is with your code (which is equivalent to point 3).

[sommerluk]

I would suggest you increase the vertical gap between the symbol from 1/3 to 2/3 pixel

That implies to reduce the height of the lines from 5 to 4⅔. If that’s okay, I can increase the gap…

Might it make sense to use round line caps?

[imagico]

That implies to reduce the height of the lines from 5 to 4⅔.

That is correct.

Note the length perception of for example a 5px line differs significantly depending on if it is pixel aligned or not.

Ultimately any such changes are small compared to those caused by the SVG rendering issues (#2750).

[kocio-pl]

Is this code ready to be merged? How does this pattern look like now?

[sommerluk]

Rebased.

Implemented the suggestion of @imagico

[kocio-pl]

For me it's fine, you can join the commits into one.

[matthijsmelissen]

I'd prefer if the person merging the PR does that.

[kocio-pl]

What would that help us with?

[sommerluk]

Oops, too late, the commits are yet joined (and I likely don’t know how to un-do that).

[sommerluk]

What would that help us with?

Would mean less work for me 😉

[matthijsmelissen]

The PR proposer normally doesnt know in advance when review of his PR is finished, so it's hard to know what's the right moment for squashing. In this case @kocio-pl explicitly asked for it, which requires an additional action from the proposer (and not all proposers respond as faat). My comment was more to point out general best practice, but in this particular case it really does not matter.

[kocio-pl]

I understand now, but I guess this would change the commit attribution (from the author to the merger)?

[matthijsmelissen]

Good point, I'm not quite sure.

[HolgerJeromin]

I understand now, but I guess this would change the commit attribution (from the author to the merger)?

I am pretty sure it will not. You can fake whatever you want with git. I have done nothing codewise for #1694 Mateusz has done everything by himself.

[matkoniecz]

In #1694 I probably manually added authorship information.

Git rebase is not changing authorship information (for merging commits by the same author), what github calls rebase is changing authorship metadata - see https://help.github.com/articles/about-pull-request-merges/#rebase-and-merge-your-pull-request-commits

[kocio-pl]

As far as I understand we should not move this pattern to SVG, since it uses gamma (#2750).

[kocio-pl]

Thanks for trying! I hope we will eventually switch raster patterns to SVG one day.