Opened 13 years ago

Closed 12 years ago

Last modified 12 years ago

#3784 closed defect (fixed)

Badly placed labels with the characters "gjpqy"

Reported by: aboudreault Owned by: aboudreault
Priority: normal Milestone: 6.0 release
Component: MapServer C Library Version: svn-trunk (development)
Severity: normal Keywords:
Cc: sdlime, tbonfort, dmorissette

Description

It seems that a label that contains a letters within "gjpqy" (letters with tail) are not placed correctly. They are moved too high. It's probably due to the way we calculate the label position.

I will attach images to demonstrate the bug. I created a mapfile with 2 layers with a simple WKT feature. Both feature are points and they have the same latitude value but a different longitude value.

Attachments (2)

text_bug.gif (1.3 KB ) - added by aboudreault 13 years ago.
text_test.map (1.8 KB ) - added by aboudreault 13 years ago.

Download all attachments as: .zip

Change History (15)

by aboudreault, 13 years ago

Attachment: text_bug.gif added

by aboudreault, 13 years ago

Attachment: text_test.map added

comment:1 by aboudreault, 13 years ago

Steve, Thomas, do you have any objection if I investigate to fix this bug for 6.0 release? I would like to be sure it wont be any bad behaviors or something I didn't think about...

comment:2 by sdlime, 13 years ago

Ah, baseline issues. I suppose it's ok to investigate if you have time. I think the label placement is renderer independent isn't it Thomas?

What does 5.6 produce? If similar than I wouldn't rush to get this into 6.0..

Steve

comment:3 by tbonfort, 13 years ago

+1 for waiting till after 6.0 if this isn't a regression

comment:4 by tbonfort, 13 years ago

see also #1449, where I still don't understand what we're trying to do with lines around line 2446 of mapdraw.c: http://trac.osgeo.org/mapserver/browser/trunk/mapserver/mapdraw.c?rev=11482#L2446

from some testing I've done, msGetLabelSize will return a rectObj with maxy>0 if the given text has some characters where parts will end up below the baseline. I'm guessing we should take that into account when calculating the labelpoint rather than relying on the current hack of setting labelPtr->offsety.

Steve any comments, as you seem to be the one who knows about baseline adjustments?

comment:5 by aboudreault, 13 years ago

We did some investigation into that issue. The main problem is that agg/gd (that uses freetype) compute a label bbox based on the letter that the string contains. So, obviously, a string like "aaa", wont be the same than "apa". It is not especially a bug in MapServer since we return the minimal bbox of a string, which is ok. However, for our client, we did a patch in AGG, to always use the ascender/descender of a freetype font in the text bbox. That way, all bbox returned have always the same y1,y2 values. This is not a solution for our mapserver users. Perhaps in the future we could add a mapserver option to do something similar.

comment:6 by sdlime, 13 years ago

We really need the full bbox for collision avoidance, but could alter label point computation using ascender, descender and perhaps even caps height depending on label position. I would call it a MapServer bug.

For UL/UR/UC you'd want to add the descender height pulling the labels down (in graphics coords). For the CL/CR/CC you'd want to center on the full height. For LL/LR/LC you'd want to subtract the ascender height pulling the labels up a bit.

Alan, where'd you do your hacking? Seems like this would be in the get_metrics function(s) in maplabel.c.

Steve

comment:7 by aboudreault, 13 years ago

In the get_metrics function, we already have the string bbox computed, so we cannot know the ascender/descender values of the font. It has been obtained via agg/gd. I did my change in renderers/agg/src/agg_font_freetype.cpp, in the prepare_glyph function.

comment:8 by tbonfort, 13 years ago

Alan,

isn't rect.maxy>0 an indication of how low you are below the baseline ? (ok, it probably won't work for multiline labels)

comment:9 by aboudreault, 13 years ago

Yes Thomas, this is an indication. IIRC, the get_metrics* functions are called for each annotation to be drawn (sequentially). So if the text is "aaa", we won't know anything about what the ascender/descender are. So we can't adjust the values. However, we can know the descender If we have a text "apa", since the p will have a bbox miny<0. I may be wrong on that... you both probably know more than me about text rendering. But since we have the bbox computed in those functions, I don't see how we could adjust things properly considering *all* strings and drawing features sequentially. Of course, we could modify the API to be able to get the ascender/descender of the font easily to adjust our text placement. Though I don't think this is a 6.0 task.

comment:10 by tbonfort, 12 years ago

Resolution: fixed
Status: newclosed

in r13245 and r13250 I have removed the hack to adjust the baseline, the renderers now return a bounding box where the ink under the baseline of the last line is not accounted for in the bounding box. This makes for nicer output when labeling markers, as the label text gets put closer to the marker. The only case with latin characters that might cause a tiny collision is if the last character of the label is a q *and* the last line is the longest one (which is always the case on single line labels). I feel the benefits largely outweigh the inconvenience. With the applied patch, this test-case passes without the need for patching AGG.

comment:11 by tbonfort, 12 years ago

msautotest added in 13251

comment:12 by tbonfort, 12 years ago

msautotest added in r13251

comment:13 by dmorissette, 12 years ago

aboudreault, can you please confirm that this change does not "break" anything with MapInfo labels coming as STYLEITEM AUTO which was the original problem that was reported to us and led to the current ticket?

Note: See TracTickets for help on using tickets.