Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Incorrect ShieldSymbolizer vertical alignment #1078

Closed
ramunasd opened this Issue · 24 comments

7 participants

@ramunasd

There is additional vertical offset for shield symbolizer image. To compensate this, i need to use dy="-2"

@mirecta

Yes trouble for me to

@Andrey-VI

Same problem

@herm herm was assigned
@herm
Collaborator

I'll investigate this. It almost certainly has to do something with the new text placement code.

@springmeyer
Owner

see also MapQuest/MapQuest-Mapnik-Style#8 which might be related.

@yhahn

I noticed while testing 009a1e4 that the text position now matches between master and an old mapnik revision prior to the new text placement code, but the shield image is now offset by several pixels vertically.

@ramunasd

seems there is problem and with simple TextSymbolizer placement="line", same offset of 2 pixels

@herm herm referenced this issue from a commit
@herm herm Revert "+ fix shield/text positioning - we still need to floor both t…
…ext and shield"

floor is removed from position calculation now, resulting in better marker and text matching.
Refs #1078.

This reverts commit 009a1e4.
7b22d69
@springmeyer
Owner

so, @herm - do these changes fix the issue, or no?

@Andrey-VI

@springmeyer
No, it's not.

@FewKinG

I can say nothing about the vertical positioning as the rotational issue (MapQuest/MapQuest-Mapnik-Style#8) still persists for me using the trunk build from last night. Any chance these might indeed be related?

@ramunasd

After 9be9b09 i have additional offset from left and top (excluding already compensated 2 pixels):

So guys, it's going worst and worst? :D

@Andrey-VI

Rotation issue exist if using placement='line' parameter in ShieldSymbolizer. I changed this setting to placement='point' and rotation gone. But I don't know is it right or wrong.

@FewKinG

Yeah right, changed the placement for all shield texts to "point" instead of "line" which seems to make sense. The rotational issue is gone now. Don't know if mapnik or the mapquest style got it wrong though.
Now just the offset problems remain.

@ramunasd

I think we need good visual tests, otherwise we cannot check regression.

@herm
Collaborator

I've added a test:https://github.com/mapnik/mapnik/blob/master/tests/visual_tests/shieldsymbolizer-1.xml
Looks like the text is correctly placed, but the image has an offset.
(at least for point placement. I'll check line placement later)

@herm
Collaborator

The problem seems to be that my new placement finder returns different starting_y values than the old version. When I set unlock-image="true" everything works.

Update: only starting_y is problematic.

@herm herm referenced this issue from a commit
@herm herm Handle text position differently.
Should fix part of the ShieldSymbolizer problems.
Refs #1078.
6efdb39
@herm herm closed this issue from a commit
@herm herm Fix text height calculation.
Closes #1078.
2ea6ab2
@herm herm closed this in 2ea6ab2
@ramunasd

still exists strange offset from left and top, strange because it is not on all shields

@herm
Collaborator

@ramunasd: Could you please post an example?

@FewKinG

This is what it looks like for me now. Still some misplacement there I guess:
http://img339.imageshack.us/img339/6756/shieldtext.png

Shields are positioned using placement="point". I am NOT using unlock-image="true".

@herm herm reopened this
@herm
Collaborator

Looks like my testcase was not good enough. The offset seems to be only one pixel. I guess the style you are using is MapQuest's and therefore SVG shields. There should be no rounding issues with SVGs. I'll try to find this offset later today.

@ramunasd

I'll try to add more visual tests to show current problem

@FewKinG

Yes I am using the mapquest style.

@herm herm referenced this issue from a commit
@herm herm Render ShieldSymbolizer tests with varying width. Triggers the proble…
…m in #1078.

Update script to handle case when no reference image is found.
73339b3
@herm herm closed this in 46f80cc
@ramunasd

Yeah, this case is fixed, but we still need more visual tests to prove good rendering. I see some tests fails after latest fix?

@herm
Collaborator

Failing tests were unrelated, but are fixed now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.