Steve, Shawn,<br><br>I think this makes a lot of sense (conceptually, I haven&#39;t looked at the code).<br><br>The current approach of Mapserver is far from perfect, and I&#39;ve asked previously on the wms-dev list if it is allowed to leave out width and height in the LegendURL or use values of -1, but as you noticed WMS 
1.3 has this possibility where 1.1 hasn&#39;t. <br><br>But if the server knows the legend image size correctly, it will always be the preferable way to include it in the GetCapabilities output.<br><br>So I&#39;m +1.<br><br>
Best regards,<br>Bart<br><br><div class="gmail_quote">On Dec 11, 2007 11:40 PM, Shawn Gervais &lt;<a href="mailto:project10@project10.net">project10@project10.net</a>&gt; wrote:<br><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
Hi devs,<br><br>I have attached a patch which:<br> &nbsp; &nbsp; &nbsp; &nbsp;- Refactors legend size calculation out of msDrawLegend and into its<br>own function<br> &nbsp; &nbsp; &nbsp; &nbsp;- Uses the legend size calculating function to better estimate the<br>
height/width of LegendURLs advertised in GetCapabilities responses<br><br><br>There seems (to me) to be some ambiguity in the WMS specs regarding<br>LegendURL. WMS 1.3.0 clears this up a lot, and states (03-109r1, 7.2.4.6.5
):<br><br>&quot;Servers should provide the width and height attributes if known at the<br>time of processing the GetCapabilities request.&quot;<br><br>And WMS 1.3.0 makes those attributes optional. However, under WMS 1.1.1
<br>MapServer advertises a GetLegendGraphic-using OnlineResource for the<br>LegendURL, but provides dimensions that are only sufficient for drawing<br>the legend &#39;key&#39;, but not the label.<br><br>Of course, the dimensions will (potentially) be incorrect if the client
<br>issues RULE or SCALE parameters, but they should be correct for the URL<br>that is advertised.<br><br>Any comments? Flames?<br><font color="#888888"><br>-Shawn<br></font><br>Index: maplegend.c<br>===================================================================
<br>--- maplegend.c (revision 7163)<br>+++ maplegend.c (working copy)<br>@@ -110,6 +110,104 @@<br>&nbsp;}<br><br>&nbsp;/*<br>+ * Calculates the optimal size for the legend<br>+ *<br>+ * Returns one of:<br>+ * &nbsp; MS_SUCCESS<br>+ * &nbsp; MS_FAILURE
<br>+ */<br>+int msLegendCalcSize(mapObj *map, int scale_independent, int *size_x,<br>+ &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; int *size_y) {<br>+ &nbsp; &nbsp;int i, j;<br>+ &nbsp; &nbsp;int status, maxwidth=0, nLegendItems=0;<br>+ &nbsp; &nbsp;char *transformedText; // Label text after applying wrapping,
<br>+ &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; // encoding if necessary<br>+ &nbsp; &nbsp;layerObj *lp;<br>+ &nbsp; &nbsp;rectObj rect;<br>+<br>+ &nbsp; &nbsp;/* Reset sizes */<br>+ &nbsp; &nbsp;*size_x = 0;<br>+ &nbsp; &nbsp;*size_y = 0;<br>+<br>+ &nbsp; &nbsp;/* Enable scale-dependent calculations */
<br>+ &nbsp; &nbsp;if (!scale_independent) {<br>+ &nbsp; &nbsp; &nbsp; &nbsp;map-&gt;cellsize = msAdjustExtent(&amp;(map-&gt;extent), map-&gt;width, map-&gt;height);<br>+ &nbsp; &nbsp; &nbsp; &nbsp;status = msCalculateScale(map-&gt;extent, map-&gt;units, map-&gt;width,<br>
+ &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;map-&gt;height, map-&gt;resolution, &amp;map-&gt;scaledenom);<br>+ &nbsp; &nbsp; &nbsp; &nbsp;if (status != MS_SUCCESS) return MS_FAILURE;<br>+ &nbsp; &nbsp;}<br>+<br>+ &nbsp; &nbsp;/*<br>+ &nbsp; &nbsp; * step through all map classes, and for each one that will be displayed
<br>+ &nbsp; &nbsp; * calculate the label size<br>+ &nbsp; &nbsp; */<br>+ &nbsp; &nbsp;for (i=0; i&lt;map-&gt;numlayers; i++) {<br>+ &nbsp; &nbsp; &nbsp; &nbsp;lp = (GET_LAYER(map, map-&gt;layerorder[i]));<br>+<br>+ &nbsp; &nbsp; &nbsp; &nbsp;if ((lp-&gt;status == MS_OFF) || (lp-&gt;type == MS_LAYER_QUERY)) /* skip it */
<br>+ &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;continue;<br>+<br>+ &nbsp; &nbsp; &nbsp; &nbsp;if (!scale_independent &amp;&amp; map-&gt;scaledenom &gt; 0) {<br>+ &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;if ((lp-&gt;maxscaledenom &gt; 0) &amp;&amp; (map-&gt;scaledenom &gt; lp-&gt;maxscaledenom))<br>+ &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;continue;
<br>+ &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;if ((lp-&gt;minscaledenom &gt; 0) &amp;&amp; (map-&gt;scaledenom &lt;= lp-&gt;minscaledenom))<br>+ &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;continue;<br>+ &nbsp; &nbsp; &nbsp; &nbsp;}<br>+<br>+ &nbsp; &nbsp; &nbsp; &nbsp;for (j=lp-&gt;numclasses-1; j&gt;=0; j--) {<br>+ &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;if (!lp-&gt;class[j]-&gt;name) continue; /* skip it */
<br>+<br>+ &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;/* Verify class scale */<br>+ &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;if (!scale_independent &amp;&amp; map-&gt;scaledenom &gt; 0) {<br>+ &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;if ( &nbsp; (lp-&gt;class[j]-&gt;maxscaledenom &gt; 0)<br>+ &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;&amp;&amp; (map-&gt;scaledenom &gt; lp-&gt;class[j]-&gt;maxscaledenom))
<br>+ &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;continue;<br>+<br>+ &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;if ( &nbsp; (lp-&gt;class[j]-&gt;minscaledenom &gt; 0)<br>+ &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;&amp;&amp; (map-&gt;scaledenom &lt;= lp-&gt;class[j]-&gt;minscaledenom))<br>+ &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;continue;
<br>+ &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;}<br>+<br>+ &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;/*<br>+ &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; * apply encoding and line wrapping to the legend label if requested<br>+ &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; * this is done conditionnally as the text transformation function<br>+ &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; * does some memory allocations that can be avoided in most cases.
<br>+ &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; * the transformed text must be freed once finished, this must be done<br>+ &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; * conditionnally by testing if the transformed text pointer is the<br>+ &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; * same as the class name pointer
<br>+ &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; */<br>+ &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;if (map-&gt;legend.label.encoding || map-&gt;legend.label.wrap)<br>+ &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;transformedText = msTransformLabelText(&amp;map-&gt;legend.label,<br>+ &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; lp-&gt;class[j]-&gt;name);
<br>+ &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;else<br>+ &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;transformedText = lp-&gt;class[j]-&gt;name;<br>+<br>+ &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;if ( &nbsp; transformedText == NULL<br>+ &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;|| msGetLabelSize(transformedText, &amp;map-&gt;legend.label,<br>
+ &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;&amp;rect, &amp;(map-&gt;fontset), 1.0, MS_FALSE) != 0)<br>+ &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;{ /* something bad happened */<br>+ &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;if (transformedText != lp-&gt;class[j]-&gt;name)<br>+ &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;free(transformedText);
<br>+<br>+ &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;return MS_FAILURE;<br>+ &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;}<br>+<br>+ &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;maxwidth = MS_MAX(maxwidth, MS_NINT(rect.maxx - rect.minx));<br>+ &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;*size_y += MS_MAX(MS_NINT(rect.maxy - rect.miny), map-&gt;legend.keysizey
);<br>+ &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;nLegendItems++;<br>+ &nbsp; &nbsp; &nbsp; &nbsp;}<br>+ &nbsp; &nbsp;}<br>+<br>+ &nbsp; &nbsp;/* Calculate the size of the legend: */<br>+ &nbsp; &nbsp;/* &nbsp; - account for the Y keyspacing */<br>+ &nbsp; &nbsp;*size_y += (2*VMARGIN) + ((nLegendItems-1) * map-&gt;legend.keyspacingy
);<br>+ &nbsp; &nbsp;/* &nbsp; - determine the legend width */<br>+ &nbsp; &nbsp;*size_x = (2*HMARGIN) + maxwidth + map-&gt;legend.keyspacingx + map-&gt;legend.keysizex;<br>+<br>+ &nbsp; &nbsp;return MS_SUCCESS;<br>+}<br>+<br>+/*<br>&nbsp;** Creates a GD image of a legend for a specific map. msDrawLegend()
<br>&nbsp;** respects the current scale, and classes without a name are not<br>&nbsp;** added to the legend.<br>@@ -121,14 +219,11 @@<br>&nbsp;*/<br>&nbsp;imageObj *msDrawLegend(mapObj *map, int scale_independent)<br>&nbsp;{<br>- &nbsp; &nbsp;int status;<br>
-<br> &nbsp; &nbsp; gdImagePtr img; /* image data structure */<br> &nbsp; &nbsp; int i,j; /* loop counters */<br> &nbsp; &nbsp; pointObj pnt;<br> &nbsp; &nbsp; int size_x, size_y=0;<br> &nbsp; &nbsp; layerObj *lp;<br>- &nbsp; &nbsp;int maxwidth=0,nLegendItems=0;<br> &nbsp; &nbsp; rectObj rect;
<br> &nbsp; &nbsp; imageObj *image = NULL;<br> &nbsp; &nbsp; outputFormatObj *format = NULL;<br>@@ -142,14 +237,12 @@<br> &nbsp; &nbsp; typedef struct legend_struct legendlabel;<br> &nbsp; &nbsp; legendlabel *head=NULL,*cur=NULL;<br><br>- &nbsp; &nbsp;if (!scale_independent) {
<br>- &nbsp; &nbsp; &nbsp; &nbsp;map-&gt;cellsize = msAdjustExtent(&amp;(map-&gt;extent), map-&gt;width, map-&gt;height);<br>- &nbsp; &nbsp; &nbsp; &nbsp;status = msCalculateScale(map-&gt;extent, map-&gt;units, map-&gt;width, map-&gt;height, map-&gt;resolution, &amp;map-&gt;scaledenom);
<br>- &nbsp; &nbsp; &nbsp; &nbsp;if(status != MS_SUCCESS) return(NULL);<br>- &nbsp; &nbsp;}<br>+<br><br> &nbsp; &nbsp; if(msValidateContexts(map) != MS_SUCCESS) return NULL; /* make sure there are no recursive REQUIRES or LABELREQUIRES expressions */<br><br>+ &nbsp; &nbsp;if(msLegendCalcSize(map, scale_independent, &amp;size_x, &amp;size_y) != MS_SUCCESS) return NULL;
<br>+<br> &nbsp; &nbsp; /*<br> &nbsp; &nbsp; &nbsp;* step through all map classes, and for each one that will be displayed<br> &nbsp; &nbsp; &nbsp;* keep a reference to its label size and text<br>@@ -201,19 +294,10 @@<br> &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; }<br> &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; return(NULL);
<br> &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; }<br>- &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;maxwidth = MS_MAX(maxwidth, MS_NINT(rect.maxx - rect.minx));<br> &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; cur-&gt;height = MS_MAX(MS_NINT(rect.maxy - rect.miny), map-&gt;legend.keysizey);<br>- &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;size_y+=cur-&gt;height;
<br>- &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;nLegendItems++;<br> &nbsp; &nbsp; &nbsp; &nbsp; }<br> &nbsp; &nbsp; }<br><br>- &nbsp; &nbsp;/*<br>- &nbsp; &nbsp; ** Calculate the optimal image size for the legend<br>- &nbsp; &nbsp; */<br>- &nbsp; &nbsp;size_y += (2*VMARGIN) + ((nLegendItems-1)*map-&gt;legend.keyspacingy); /*initial vertical size*/
<br>- &nbsp; &nbsp;size_x = (2*HMARGIN)+(maxwidth)+(map-&gt;legend.keyspacingx)+(map-&gt;legend.keysizex);<br>-<br> &nbsp; &nbsp; /* ensure we have an image format representing the options for the legend. */<br> &nbsp; &nbsp; msApplyOutputFormat(&amp;format, map-&gt;outputformat, map-&gt;
legend.transparent, map-&gt;legend.interlace, MS_NOOVERRIDE);<br><br>Index: mapserver.h<br>===================================================================<br>--- mapserver.h (revision 7163)<br>+++ mapserver.h (working copy)
<br>@@ -1648,6 +1648,7 @@<br>&nbsp;MS_DLL_EXPORT void freeImageCache(struct imageCacheObj *ic);<br><br>&nbsp;MS_DLL_EXPORT imageObj *msDrawLegend(mapObj *map, int scale_independent); /* in maplegend.c */<br>+MS_DLL_EXPORT int msLegendCalcSize(mapObj *map, int scale_independent, int *size_x, int *size_y);
<br>&nbsp;MS_DLL_EXPORT int msEmbedLegend(mapObj *map, imageObj *img);<br>&nbsp;MS_DLL_EXPORT int msDrawLegendIcon(mapObj* map, layerObj* lp, classObj* myClass, int width, int height, imageObj *img, int dstX, int dstY);<br>&nbsp;MS_DLL_EXPORT imageObj *msCreateLegendIcon(mapObj* map, layerObj* lp, classObj* myClass, int width, int height);
<br>Index: mapwms.c<br>===================================================================<br>--- mapwms.c &nbsp; &nbsp;(revision 7163)<br>+++ mapwms.c &nbsp; &nbsp;(working copy)<br>@@ -1029,6 +1029,24 @@<br> &nbsp; }<br>&nbsp;}<br><br>+/*<br>+ * msWMSGetLegendURLSize() - Estimates the size of a GetLegendGraphic result,
<br>+ * &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; for a specific layer<br>+ */<br>+void msWMSGetLegendURLSize(mapObj *map, layerObj *lp, int *height, int *width) {<br>+ &nbsp; &nbsp;int i;<br>+<br>+ &nbsp; &nbsp;/* Turn off all layers other than the requested layer, required */
<br>+ &nbsp; &nbsp;/* for msLegendCalcSize() */<br>+ &nbsp; &nbsp;for (i=0; i&lt;map-&gt;numlayers; i++) {<br>+ &nbsp; &nbsp; &nbsp; &nbsp;if (GET_LAYER(map, i) == lp)<br>+ &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;GET_LAYER(map, i)-&gt;status = MS_ON;<br>+ &nbsp; &nbsp; &nbsp; &nbsp;else<br>+ &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;GET_LAYER(map, i)-&gt;status = MS_OFF;
<br>+ &nbsp; &nbsp;}<br>+<br>+ &nbsp; &nbsp;msLegendCalcSize(map, 1, height, width); // Calculate scale-independent legend dimensions<br>+}<br><br>&nbsp;/*<br>&nbsp;** msDumpLayer()<br>@@ -1286,16 +1304,11 @@<br> &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;}<br> &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;if (classnameset)
<br> &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;{<br>- &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; if (map-&gt;legend.keysizex &gt; 0)<br>- &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; sprintf(width, &quot;%d&quot;, map-&gt;legend.keysizex);<br>- &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; else<br>- &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; sprintf(width, &quot;%d&quot;, 20);/* default; */
<br>+ &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; int size_x, size_y;<br>+ &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; msWMSGetLegendURLSize(map, lp, &amp;size_x, &amp;size_y);<br>+ &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; sprintf(width, &nbsp;&quot;%d&quot;, size_x);<br>+ &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; sprintf(height, &quot;%d&quot;, size_y);
<br><br>- &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; if (map-&gt;legend.keysizey &gt; 0)<br>- &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;sprintf(height, &quot;%d&quot;, map-&gt;legend.keysizey);<br>- &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; else<br>- &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; sprintf(height, &quot;%d&quot;, 20);/* default; */
<br>-<br> &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;legendurl = (char*)malloc(strlen(script_url_encoded)+200);<br><br>&nbsp;#ifdef USE_GD_PNG<br><br></blockquote></div><br>