symbol written to mapfile and symbolset.

[jef]

If a mapfile is saved after the symbolset of the mapfile is saved, the symbols are also written to the mapfile. Doing it repeatedly produces a huge mapfile.

Also PATTERNs and STYLEs of CARTOLINE symbol are not written to the symbolset.

The attached patch fixes both.

[sholl]

is this patch already applied in MS 6.0.0?

I would appreciate this since we are activly working with symbols generated through mapscript.

TIA

Stephan

[jef]

Replying to sholl:

is this patch already applied in MS 6.0.0?

I would appreciate this since we are activly working with symbols generated through mapscript.

Apparently no. CARTOLINE was dropped. And the other references seem unchanged: ​mapfile.c:5245, ​mapsymbol.c:277, ​mapsymbol.c:735

Jürgen

[sholl]

Jürgen,

are you in the position to apply the patch tu trunk? It would be very helpful having this issue fixed in the next releases though.

TIA

Stephan

[jef]

Replying to sholl:

are you in the position to apply the patch tu trunk? It would be very helpful having this issue fixed in the next releases though.

No, sorry. If I was, I would probably have commited it right away ;)

[sdlime]

Is this patch valid against trunk or 6.0? Seems like it needs to be reworked a bit.

Steve

[jef]

Replying to sdlime:

Is this patch valid against trunk or 6.0? Seems like it needs to be reworked a bit.

It was made against trunk at r10677. With CARTOLINEs gone, what's left is (untested):

Index: mapfile.c
===================================================================
--- mapfile.c   (revision 11753)
+++ mapfile.c   (working copy)
@@ -5244,7 +5244,8 @@
 
   /* write symbol with INLINE tag in mapfile */
   for(i=0; i<map->symbolset.numsymbols; i++) {
-    writeSymbol(map->symbolset.symbol[i], stream);
+    if( map->symbolset.symbol[i]->inmapfile ) 
+      writeSymbol(map->symbolset.symbol[i], stream); 
   }
 
   writeProjection(stream, indent, &(map->projection));
Index: mapsymbol.c
===================================================================
--- mapsymbol.c (revision 11753)
+++ mapsymbol.c (working copy)
@@ -274,8 +274,6 @@
 {
   int i;
 
-  if(s->inmapfile != MS_TRUE) return;
-
   fprintf(stream, "  SYMBOL\n");
   if(s->name != NULL) fprintf(stream, "    NAME \"%s\"\n", s->name);
   
@@ -734,7 +732,6 @@
     }
     /* Don't ever write out the default symbol at index 0 */
     for (i=1; i<symbolset->numsymbols; i++) {
-        symbolset->symbol[i]->inmapfile = MS_TRUE;
         writeSymbol((symbolset->symbol[i]), stream);
     }
     return MS_SUCCESS;

[sdlime]

I will try and patch 6.0 branch and trunk... Steve

[sholl]

Steve,

did you already commit this patch?

TIA

[sdlime]

I haven't (been out of town). Will try this evening. -Steve

[sdlime]

I applied to trunk just now (r11843). Any chance someone can confirm the fix works and then I can patch the 6.0 branch?

This still doesn't seem right though. Moving the check of "inmapfile" outside msWriteSymbol() makes sense. Seems like it should be checked in msSaveSymbolSetStream() too so that only symbols with inmapfile=false are written...

Steve

[sdlime]

In r11844 I added logic to make sure inmapfile=false for symbols written by msSaveSymbolSetStream(). That seems to make the most sense to me.

Steve

[sholl]

Steve,

I tried to compile under windows (VC9.0) and it bailed out here:

mapsymbol.c(640) : warning C4018: '>': Konflikt zwischen 'signed' und 'unsigned'
mapsymbol.c(641) : warning C4018: '>': Konflikt zwischen 'signed' und 'unsigned'
mapsymbol.c(735) : error C2065: 'map': nichtdeklarierter Bezeichner
mapsymbol.c(735) : error C2223: Der linke Teil von '->symbolset' muss auf eine Struktur/Union zeigen
NMAKE : fatal error U1077: ""c:\Program Files\Microsoft Visual Studio 9.0\VC\BIN\cl.EXE"": Rückgabe-Code "0x2"
Stop.
NMAKE : fatal error U1077: ""c:\Program Files\Microsoft Visual Studio 9.0\VC\BIN\nmake.EXE"": Rückgabe-Code "0x2"
Stop.

Sorry for the german error-messages, but it cannot be switched since we have the _german_ localized VC9.

Best

Stephan

[jef]

Replying to sholl:

> mapsymbol.c(735) : error C2065: 'map': nichtdeklarierter Bezeichner
> mapsymbol.c(735) : error C2223: Der linke Teil von '->symbolset' muss auf eine Struktur/Union zeigen

Try this fix:

Index: mapsymbol.c
===================================================================
--- mapsymbol.c (revision 11845)
+++ mapsymbol.c (working copy)
@@ -732,7 +732,7 @@
   }
   /* Don't ever write out the default symbol at index 0 */
   for (i=1; i<symbolset->numsymbols; i++) {
-    if(!map->symbolset.symbol[i]->inmapfile) writeSymbol((symbolset->symbol[i]), stream);
+    if(!symbolset.symbol[i]->inmapfile) writeSymbol((symbolset->symbol[i]), stream);
   }
   return MS_SUCCESS;
 }

[jef]

Ouch, sorry. Better this one - but beware, that didn't see any compiler yet either.

Index: mapsymbol.c
===================================================================
--- mapsymbol.c (revision 11845)
+++ mapsymbol.c (working copy)
@@ -732,7 +732,7 @@
   }
   /* Don't ever write out the default symbol at index 0 */
   for (i=1; i<symbolset->numsymbols; i++) {
-    if(!map->symbolset.symbol[i]->inmapfile) writeSymbol((symbolset->symbol[i]), stream);
+    if(!symbolset->symbol[i]->inmapfile) writeSymbol((symbolset->symbol[i]), stream);
   }
   return MS_SUCCESS;
 }

[sholl]

Replying to jef:

Ouch, sorry. Better this one - but beware, that didn't see any compiler yet either.

This patch makes it compile under MSVC9.0. I will check the functionality though.

Thanks for your input, jef.

[dmorissette]

I came across the same build issue this morning and committed the fix in r11846.

[sdlime]

Crap, sorry about that. Dan, how is msSaveSymbolSetStream() used? Now I'm not confident in my last commit to limit to symbols to those with (!symbolset->symbol[i]->inmapfile).

It seems like a useless function with that limit since you'd already have a symbolset on disk someplace.

Steve

[dmorissette]

I didn't know either, but thanks to 'svn blame', I found out that msSaveSymbolSetStream() was introduced in r2851 to allow the addition of a symbolset.save() method in SWIG MapScript (not currently implemented in PHP MapScript).

So it is being used, and your fix seems right (famous last words, I know).

[sdlime]

You don't think this should be back ported to 6.0? Seems like a bug to me. -Steve

[dmorissette]

sholl wrote that they make lots of use of symbols generated through MapScript. Maybe if they can confirm that the fix works fine and causes no side-effect then we could backport.

[sholl]

We can confirm that it is working as expected. Though there was no time for really intensive testing yet, just a basic test in our environment to make sure that new (mapscript-generated) symbols fall into the giben symbolset and not directly in the mapfile.

It would be good to have this in 6.0-branch as well.

TIA

Stephan

[dmorissette]

Steve, are you going to backport your fix or should I do it?

[sdlime]

I'll take care of it... Steve

[sdlime]

Committed to branch6-0 in r11882. Marking as fixed.

Steve