wrong arrguments order in msAddLabel

[bogdan@…]

Currently  signature for msAddLabel:
-----------------------------------
int msAddLabel(mapObj *map, int layerindex, int classindex, int shapeindex, int
tileindex, pointObj *point, char *string, double featuresize)

should be changed to:
----------------------
int msAddLabel(mapObj *map, int layerindex, int classindex, int tileindex, int
shapeindex, pointObj *point, char *string, double featuresize)

All calls to msAddLabel are using  shp->tileindex, shp->index as 4th and 5th
argument.
The current implementation will swap tileindex to shapeindex and when it comes
to find the tile file it will try o find the file to which the shapeidx points to.

As an example:  I have tileidx 64 and  a shapeidx 122. Instead getting a tile
file for Canada for shape 122 (a steet) I will get a tile file for Germany for
shape 64 ( a park).

[dmorissette]

It seems that this was fixed in the development trunk (4.9) in mapdraw.c r1.99,
as part of applying the patch for bug 1620.

However the problem seems to be there in the latest 4.8 source. I wonder how
this code ever worked, or are the arguments simply not used?

Adding Steve to CC.. any hint? Should the fix be backported to 4.8?

[sdlime]

That tells you how often folks use tiled data.  We can get this into the 4.8.3 
release ASAP... Do you want to make the change in the 4.8 branch?

Steve

[dmorissette]

I have verified by commenting out the shapeindex and tileindex members in the
labelCacheMemberObj definition that those are unused except in
msDrawStartShapeUsingIdxSWF() (mapswf.c) ... and since this code never blowed up
then it was probably never called.

I am not too keen on making a change in a stable release for a bug that I have
no way to reproduce and test. Since this is fixed already in 4.9 I would be
tempted to close this as FIXED and leave 4.8 untouched.

Bogdan, how did you come accross this issue? Which version of MapServer were you
using? Do you have a testcase to reproduce this?

[sdlime]

For what it's worth, the idea behind capturing those indexes was that you could 
use the label cache for other purposes (creating imagemaps for example) so you 
could tie a label back to it's feature.

Steve

[bogdan@…]

I am using only tiles, but I don't have test case to post here.
I use Mapserver 4.2, but I noticed that the bug has been there for quite a while
and I was surprised that it is still there in 4.8.1

I stumbled upon it when I enebled the METADATA section in the layer section and
the server was crashing because it was not finding the tile file.

By the way this code is a time bomb (mapswf.c) :
-------------------------------------------------
void msDrawStartShapeUsingIdxSWF(mapObj *map, layerObj *layer, imageObj *image,
                                 int shapeidx)
{
    shapeObj shape;

    if (map && layer && image && shapeidx >=0)
    {
        msInitShape(&shape);
        msLayerGetShape(layer, &shape, -1, shapeidx);
        msDrawStartShapeSWF(map, layer, image, &shape);
    }
}

should be changed to :
---------------------
void msDrawStartShapeUsingIdxSWF(mapObj *map, layerObj *layer, imageObj *image,
labelCacheMemberObj * cachePtr)
{
   shapeObj shape;
   int retval = 0;
   if (map && layer && image && cachePtr->shapeindex >=0)
   {
      msInitShape(&shape);
      //retval = msLayerGetShape(layer, &shape, -1, shapeidx);  
	  retval = msLayerGetShape(layer, &shape, cachePtr->tileindex,
cachePtr->shapeindex);
	  
	  if (retval == MS_SUCCESS) {
        msDrawStartShapeSWF(map, layer, image, &shape);
	  } else {
		msSetError(MS_MISCERR, "Cannot find shape for shapeidx:%d", 
"msDrawStartShapeUsingIdxSWF()", cachePtr->shapeindex);
		//writeError();
	  }
   }
}

I hope that this will make it too in 4.9. 
This code was there in 4.2 and it is still there in 4.8

Regards,
Bogdan

[warmerdam]

According to the first comment from Steve, this was fixed for the 4.10 release. Since we are well past providing new 4.8.x releases, I am closing this bug report. If anyone believes the problem persists in trunk (or even 4.10.x) please reopen with supporting info.

[dmorissette]

For the record, the other issue reported in comment:5 above about msDrawStartShapeUsingIdxSWF() not checking the return value of msLayerGetShape() was also reported in ticket #1744 and fixed in v4.10.