Opened 20 years ago

Closed 20 years ago

Last modified 20 years ago

#729 closed defect (fixed)

Problems with tiled raster layers

Reported by: dmorissette Owned by: sdlime
Priority: high Milestone:
Component: MapServer C Library Version: 4.3
Severity: normal Keywords:
Cc: jmalczyk@…

Description

We've found problems when using tiled raster layers from MapScript: the map
rendering was aborted when a given tiled raster layer was used.  This was not
noticeable with shp2img which would silently exit, but MapScript would have a
problem since the map->draw() call would return an error.

The problem is in msDrawRasterLayerLow(), in the code that handles tile indexes.
 The possibility of msLayerWhichShapes() returning MS_DONE was not handled and
was treated as a failure.

A few other issues were noted:
- The code that cleaned up the temporary tile layer was using an 
  if(tileitemindex == -1). I believe it should be if(tilelayerindex == -1)
  so I changed it.
- There was a TODO note about cleaning up when msLayerWhichShapes() didn't
return at least one tile... I added the same cleanup as was found at the end of
the function, hopefully that does it.
- The function used to return(0) on success.  I fixed it to return(MS_SUCCESS)
as it should (MS_SUCCESS==0, but it's still better to explicitly use it).


The diff with the 4.3 CVS is attached below.  Can you please review it and
confirm that all is ok so that I can apply it to the 4.2 branch.  This is an
important issue for users of tiled rasters in 4.2.



===================================================================
RCS file: /data2/cvsroot/mapserver/mapraster.c,v
retrieving revision 1.117
diff -u -r1.117 mapraster.c
--- mapraster.c 1 Jun 2004 14:33:43 -0000       1.117
+++ mapraster.c 15 Jun 2004 15:56:43 -0000
@@ -1317,7 +1317,17 @@
     if((map->projection.numargs > 0) && (layer->projection.numargs > 0))
msProjectRect(&map->projection, &layer->projection, &searchrect);
 #endif
     status = msLayerWhichShapes(tlp, searchrect);
-    if(status != MS_SUCCESS) return(MS_FAILURE); // TODO: probably need more
clean up here
+    if (status != MS_SUCCESS) {
+        // Can be either MS_DONE or MS_FAILURE
+        msLayerClose(tlp);
+        if(tilelayerindex == -1) {
+            freeLayer(tlp);  // cleanup temporary tile layer
+            free(tlp);
+        }
+        if (status == MS_DONE)
+            return MS_SUCCESS;
+        return MS_FAILURE;
+    }
   }

   done = MS_FALSE;
@@ -1508,13 +1518,13 @@

   if(layer->tileindex) { // tiling clean-up
     msLayerClose(tlp);
-    if(tileitemindex == -1) {
+    if(tilelayerindex == -1) {
       freeLayer(tlp);
       free(tlp);
     }
   }

-  return 0;
+  return MS_SUCCESS;
 }

Change History (11)

comment:1 by dmorissette, 20 years ago

For the record, this comes from DMSG internal bug 2821

comment:2 by sdlime, 20 years ago

Looks ok to me.

comment:3 by dmorissette, 20 years ago

Status: newassigned
I was trying to quickly build a patched 4.2 version by copying mapraster.c from
4.3 CVS over a 4.2 source tree and that doesn't seem to work: I still get the
same error as before.

Before I run this in the debugger again: are you aware of other fixes in the 4.3
branch that I need to backport to 4.2 for this to work?

comment:4 by dmorissette, 20 years ago

Actually I have no way to tell if the error comes from the same place as before
until I run in the debugger, but the symptoms from MapScript 4.2 are the same as
before the patch: $map->draw() fails to draw the map.

comment:5 by sdlime, 20 years ago

There was a simple memory leak fix in the tiling code that was fixed in the 4.3 
source. Other than that I've not touched the raster code.

Do you have sample mapfile and a simple script. I can look too.

comment:6 by dmorissette, 20 years ago

The app in which this happens has 575MB of data... I could probably produce a
smaller testcase, but I think I'll try debugging before spending time on a
testcase... I was just hoping that you or Frank would have run into this before.

comment:7 by dmorissette, 20 years ago

Resolution: fixed
Status: assignedclosed
I must have done something stupid in my initial test: I applied the patch to the
4.2 branch and all it's working great now.  Committed to 4.2 branch in CVS.

comment:8 by sdlime, 20 years ago

Excellent. You'll patch dev version too?

comment:9 by dmorissette, 20 years ago

4.3 CVS was already patched earlier today.

comment:10 by fwarmerdam, 20 years ago

Cc: jmalczyk@… added

comment:11 by fwarmerdam, 20 years ago

*** Bug 747 has been marked as a duplicate of this bug. ***
Note: See TracTickets for help on using tickets.