Opened 12 years ago

Closed 12 years ago

#2142 closed defect (fixed)

mappdf.c build warning - and why lookup fontlist?

Reported by: dmorissette Owned by: zjames
Priority: normal Milestone: 5.0 release
Component: Output-PDF Version: 5.0
Severity: normal Keywords:
Cc: zjames@…

Description

mappdf.c produces the following warning:

mappdf.c: In function ‘msDrawTextPDF’:
mappdf.c:1276: warning: assignment makes integer from pointer without a cast

This is happening on a line doing a lookup of the font name in the fontset:

    font = msLookupHashTable(&(fontset->fonts), label->font);

    if(!font)
    {
        msSetError(MS_TTFERR, "Requested font (%s) not found.", "msDrawTextPDF()",
                   label->font);
        return(-1);
    }

And then the 'font' value is not reused anywhere in the function. Actually font is set to the return value from a call to PDF_findfont() later on.

What is the correct fix? Why lookup the font name if the fontset if we don't need the value? Is this a sign that the original code logic may have been broken by some past changes?

Attachments (1)

mappdf-ticket-2124.diff (1.4 KB) - added by zjames 12 years ago.

Download all attachments as: .zip

Change History (5)

comment:1 Changed 12 years ago by zjames

I think it's quite likely that problems have crept in since nobody is actively maintaining this.

PDF_findfont() uses the system mechanisms to look up a font and has some side effects like loading font metrics. However, the hash is needed to discover an actual font name via the mapserver fonts file. The proper logic is that the needed font should be resolved to a font file name (possibly with an absolute path) and then PDF_findfont should be used to get a reference to the same font so it can be loaded. It may also be necessary to set PDFLib's font search path to the proper location.

As things are, the pdf output is using system fonts and fontnames and could easily fail depending on how mapserver's fonts are configured.

I'm prepared to work on a patch for this if the fix isn't simple. The other alternative is to drop pdf support as I don't believe there are many users.

comment:2 Changed 12 years ago by dmorissette

Owner: changed from mapserverbugs to zjames

Dropping PDF support without notice may be a bit rude for users who may rely on it, but we could perhaps consider that for 5.2.

In the meantime, would you (Zak) be able to fix the lookup of fonts which is the main problem in this ticket?

I'll reassign to zjames, if you can't do it for beta5 then please push target milestone to 5.2 and reassign to me.

Changed 12 years ago by zjames

Attachment: mappdf-ticket-2124.diff added

comment:3 Changed 12 years ago by zjames

I'm attaching a patch that corrects this problem by using the path resolved by mapserver as the source of the font outline. Previously, the font to load was taken from label->font directly and it's likely that a host (system) font was used rather than the requested one. I also found that PDF_findfont() was deprecated so I replaced the call with PDF_load_font. The patch also corrects the compiler warning.

comment:4 Changed 12 years ago by dmorissette

Resolution: fixed
Status: newclosed

Thanks Zak. Your patch has been committed in r6693.

Note: See TracTickets for help on using tickets.