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

MarkersSymbolizer width & height values get doubled #1134

Closed
ajashton opened this Issue Mar 17, 2012 · 12 comments

Comments

Projects
None yet
4 participants
@ajashton
Copy link
Member

ajashton commented Mar 17, 2012

This XML snippet will render an ellipse that is 20 pixels wide and 40 pixels tall:

<MarkersSymbolizer width="10" height="20" stroke-width="0" />

Are the width and height units meant to be pixels? The values for stroke-width match up with pixels as expected, but the width and height parameters do not.

(AGG renderer, Mapnik master 385ca5b ).

@novldp

This comment has been minimized.

Copy link
Contributor

novldp commented Mar 18, 2012

Ah, I noticed this some time ago, but seem to have forgotten to open an issue on this.

You almost think it's "radius" in x/y direction instead of width/height, but then the parameter name is just plainly confusing.

@novldp

This comment has been minimized.

Copy link
Contributor

novldp commented Mar 18, 2012

agg::ellipse c(x, y, w, h);

Yup, width/height are actually radii for ellipses. I suggest for clarity's sake to internally divide by two the given width/height params for MarkersSymbolizer. I see no real sense to introduce radius parameters just for the ellipse case.

@strk

This comment has been minimized.

Copy link
Contributor

strk commented Jun 15, 2012

Did it really make sense to backport this ? Sounds like something that will change the aspect of any map using markers.

@springmeyer

This comment has been minimized.

Copy link
Member

springmeyer commented Jun 15, 2012

@strk - I think it did, but I can understand your concern. In the case of dynamically sized markers they are new in Mapnik 2.0.0 as a feature, so I deemed this fix worthy of fixing in all maintained series.

@strk

This comment has been minimized.

Copy link
Contributor

strk commented Jun 15, 2012

Why do you mention dynamically sized markers ? Isn't this about statically sized ones ?

@springmeyer

This comment has been minimized.

Copy link
Member

springmeyer commented Jun 15, 2012

Previously markers could only be loaded from a file, and not drawn at arbitrary (e.g. dynamic) widths/heights - that's what I mean. And soon width/height will be expressions - so more dynamic: #1102

@springmeyer

This comment has been minimized.

Copy link
Member

springmeyer commented Jun 17, 2012

I am totally open to removing the backport I made to 2.0.x, just let me know what others think. I however think the fix should stay in mainline master (aka 2.1.x-pre).

@strk

This comment has been minimized.

Copy link
Contributor

strk commented Jun 18, 2012

Looking at the CHANGELOG this one seems to be the only interface breaking change. Unfortunately a release with this change exists already, so any reversion would go in 2.0.2 which would represent even more troubles for those who based their maps on the "correct" semantic in 2.0.1. When was 2.0.1 released ?

@strk

This comment has been minimized.

Copy link
Contributor

strk commented Jun 18, 2012

Looking at the CHANGELOG this one seems to be the only interface breaking change. Unfortunately a release with this change exists already, so any reversion would represent troubles for those who based their maps on the "correct" semantic in 2.0.1. When was 2.0.1 released ?

@springmeyer

This comment has been minimized.

Copy link
Member

springmeyer commented Jun 18, 2012

2.0.1 was released April 10th, 2012 (just added this): https://github.com/mapnik/mapnik/blob/master/CHANGELOG.md#mapnik-201

@strk

This comment has been minimized.

Copy link
Contributor

strk commented Jun 18, 2012

On Mon, Jun 18, 2012 at 08:58:56AM -0700, Dane Springmeyer wrote:

2.0.1 was released April 10th, 2012 (just added this): https://github.com/mapnik/mapnik/blob/master/CHANGELOG.md#mapnik-201

In that case it seems young enough not to represent a big problem if
it gets replaced very soon by 2.0.2, with that commit reverted.

2.0.1 has been around for 2 months, 2.0.0 for 8.

--strk;

Sent from our free software
http://www.gnu.org/philosophy/free-sw.html

@springmeyer

This comment has been minimized.

Copy link
Member

springmeyer commented Jun 18, 2012

another +1 vote in favor of seeing 2.0.1 as "young" is that @dpaleino who manages debian packaging is waiting for 2.0.2 (since he saw it would come very soon): #1221 (comment)

springmeyer pushed a commit that referenced this issue Jul 4, 2012

Dane Springmeyer
partially rollback b93c760 - meaning that radii will continue (as in …
…Mapnik 2.0.0) to be assumed for marker width/height in the Mapnik 2.0.x series (>= 2.0.2) - closes #1163, refs #1134
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment