GAP length calculation

[havatv]

STYLE->GAP shall specify the spacing between symbols (for instance for styling of lines).

The exact definition of GAP can not be found in the documentation (the documentation says that: "This defines a distance between symbols").

For symbol construction purposes, the most useful definition would be that "GAP defines the centre to centre distance between symbols". In my opinion, it is also the most logical, so I assume that this is what is intended (this is also what I have currently documented for 6.0).

Current trunk behaviour is that GAP specifies the distance between the outer edges of the symbol bounding boxes. This is also a possible interpretation of the documentation, but not as useful.

Another argument for fixing this is that for STYLE->PATTERN, the gap length will be "correct" with BUTT linecaps. If linecap is round or square, the observed gaps will appear smaller (proportional to the width of the line).

Here is an svn diff of a quick and dirty fix that works for me (it turns out that this change actually simplify things).

svn diff mapserver/maprendering.c 
Index: mapserver/maprendering.c
===================================================================
--- mapserver/maprendering.c    (revisjon 11672)
+++ mapserver/maprendering.c    (arbeidskopi)
@@ -281,7 +281,7 @@
    for(i=0; i<p->numlines; i++)
    {
       int line_in = 0;
-      double current_length = (spacing+symbol_width)/2.0; // initial padding fo
r each line
+      double current_length = (spacing)/2.0; // placing of first symbol
       double line_length=0;
       for(j=1; j<p->line[i].numpoints; j++)
       {
@@ -322,14 +322,14 @@
             }
             if( ret != MS_SUCCESS)
                return ret;
-            current_length += symbol_width + spacing;
+            current_length += spacing;
             in = 1;
             line_in=1;
          }
 
          if (in)
          {
-            current_length -= length + symbol_width/2.0;
+            current_length -= length;
          }
          else current_length -= length;
       }
@@ -650,11 +650,11 @@
             }
 
             if(s.scale != 1) {
-               pw = MS_NINT(symbol->sizex * s.scale + s.gap)+1;
-               ph = MS_NINT(symbol->sizey * s.scale + s.gap)+1;
+               pw = MS_NINT(s.gap)+1;
+               ph = MS_NINT(s.gap)+1;
             } else {
-               pw = symbol->sizex + s.gap;
-               ph = symbol->sizey + s.gap;
+               pw = s.gap;
+               ph = s.gap;
             }
             if(pw<1) pw=1;
             if(ph<1) ph=1;

[havatv]

The diff that I provided did a little bit too much. The changes to pw and ph resulted in some strange effects with pixmap fills. When I removed those changes, things seemed to work OK.

Here is a diff for the version that works:

svn diff mapserver/maprendering.c 
Index: mapserver/maprendering.c
===================================================================
--- mapserver/maprendering.c    (revisjon 11672)
+++ mapserver/maprendering.c    (arbeidskopi)
@@ -281,7 +281,7 @@
    for(i=0; i<p->numlines; i++)
    {
       int line_in = 0;
-      double current_length = (spacing+symbol_width)/2.0; // initial padding fo
r each line
+      double current_length = (spacing)/2.0; // placing of first symbol
       double line_length=0;
       for(j=1; j<p->line[i].numpoints; j++)
       {
@@ -322,14 +322,14 @@
             }
             if( ret != MS_SUCCESS)
                return ret;
-            current_length += symbol_width + spacing;
+            current_length += spacing;
             in = 1;
             line_in=1;
          }
 
          if (in)
          {
-            current_length -= length + symbol_width/2.0;
+            current_length -= length;
          }
          else current_length -= length;
       }

[havatv]

Actually, it would be good to have GAP as centre to centre also for polygon fills. In order not to create too much havoc, I suggest that the default (when no GAP is specified) for polygon fills is to use the symbol size as gap. If GAP is specified, it will be center to center distance. This will allow precise overlay for constructing complex symbols, and it will allow overlapping symbols (which is a good thing). Since it is not possible to specify more than one value for gap, the x and y gap will have to be the same.

The file resulting from the following modifications works for me:

svn diff mapserver/maprendering.c
Index: mapserver/maprendering.c
===================================================================
--- mapserver/maprendering.c    (revisjon 11672)
+++ mapserver/maprendering.c    (arbeidskopi)
@@ -281,7 +281,7 @@
    for(i=0; i<p->numlines; i++)
    {
       int line_in = 0;
-      double current_length = (spacing+symbol_width)/2.0; // initial padding fo
r each line
+      double current_length = (spacing)/2.0; // placing of first symbol
       double line_length=0;
       for(j=1; j<p->line[i].numpoints; j++)
       {
@@ -322,14 +322,14 @@
             }
             if( ret != MS_SUCCESS)
                return ret;
-            current_length += symbol_width + spacing;
+            current_length += spacing;
             in = 1;
             line_in=1;
          }
 
          if (in)
          {
-            current_length -= length + symbol_width/2.0;
+            current_length -= length;
          }
          else current_length -= length;
       }
@@ -650,11 +650,21 @@
             }
 
             if(s.scale != 1) {
-               pw = MS_NINT(symbol->sizex * s.scale + s.gap)+1;
-               ph = MS_NINT(symbol->sizey * s.scale + s.gap)+1;
+             if (s.gap > 0) {
+                 pw = MS_NINT(s.gap);
+                 ph = MS_NINT(s.gap);
+             } else {
+                pw = MS_NINT(symbol->sizex * s.scale + s.gap);
+                ph = MS_NINT(symbol->sizey * s.scale + s.gap);
+              }
             } else {
-               pw = symbol->sizex + s.gap;
-               ph = symbol->sizey + s.gap;
+             if (s.gap > 0) {
+                pw = s.gap;
+                ph = s.gap;
+             } else {
+               pw = symbol->sizex + s.gap;
+               ph = symbol->sizey + s.gap;
+             }
             }
             if(pw<1) pw=1;
             if(ph<1) ph=1;

I attach two pngs that shows some of the things that can easily be done when this is fixed. One with styled lines, one with a polygon fill.

[tbonfort]

I see the value for this, but it does have it's drawbacks as you would have to tie the GAP value with the SIZE you have chosen.

setting a GAP that is less than SIZE would not be possible for polygons, as the renderers do not allow this kind of symbology (they use a tile as a texture to apply on the polygon)

too late for 6.0 as this definitely is not backwards compatible.

this at least needs a -dev discussion, and probably an RFC. Too bad this is proposed late, 6.0 would have been a good time to break backwards compatibility...

[havatv]

The documentation has not specified how GAP shall be interpreted. The name suggests that it is the size of the "opening" between the symbols that is specified, but the documentation is talking about distance.

I raised this issue in an email (4/1/2011 1:47 AM) to Thomas. In that email I said that I had changed the documentation to specifically say that GAP is centre-to-centre distance.

Since the changes to the code seem to be quite trivial, and the changes do not conflict with what has been documented earlier, it would be good if we could have this behaviour in for version 6. In my opinion it is a large improvement for a very reasonable price.

In version 6, there have already been some changes to how GAP works, so if we are careful, this won't break backwards compatitibility much more than the changes that are already in the code.

By being careful, I mean that we should keep default behaviour when GAP is not specified, and we could force GAPs for polygon fills to be at least the size of the symbol. I have tried to implement this, and it seems to work - here is my diff output:

svn diff mapserver/maprendering.c
Index: mapserver/maprendering.c
===================================================================
--- mapserver/maprendering.c    (revisjon 11679)
+++ mapserver/maprendering.c    (arbeidskopi)
@@ -281,7 +281,7 @@
    for(i=0; i<p->numlines; i++)
    {
       int line_in = 0;
-      double current_length = (spacing+symbol_width)/2.0; // initial padding fo
r each line
+      double current_length = (spacing)/2.0; // placing of first symbol
       double line_length=0;
       for(j=1; j<p->line[i].numpoints; j++)
       {
@@ -322,14 +322,14 @@
             }
             if( ret != MS_SUCCESS)
                return ret;
-            current_length += symbol_width + spacing;
+            current_length += spacing;
             in = 1;
             line_in=1;
          }
 
          if (in)
          {
-            current_length -= length + symbol_width/2.0;
+            current_length -= length;
          }
          else current_length -= length;
       }
@@ -650,11 +650,21 @@
             }
 
             if(s.scale != 1) {
-               pw = MS_NINT(symbol->sizex * s.scale + s.gap)+1;
-               ph = MS_NINT(symbol->sizey * s.scale + s.gap)+1;
+             if (s.gap > 0) {
+               pw = MS_MAX(MS_NINT(s.gap),symbol->sizex * s.scale);
+               ph = MS_MAX(MS_NINT(s.gap),symbol->sizey * s.scale);
+             } else {
+               pw = MS_NINT(symbol->sizex * s.scale + s.gap);
+               ph = MS_NINT(symbol->sizey * s.scale + s.gap);
+              }
             } else {
-               pw = symbol->sizex + s.gap;
-               ph = symbol->sizey + s.gap;
+             if (s.gap > 0) {
+                pw = MS_MAX(s.gap,symbol->sizex);
+                ph = MS_MAX(s.gap,symbol->sizey);
+             } else {
+               pw = symbol->sizex + s.gap;
+               ph = symbol->sizey + s.gap;
+             }
             }
             if(pw<1) pw=1;
             if(ph<1) ph=1;

Later, new parameters for more powerful mechanisms for spacing symbols along lines and over polygons could be added (sequence of distances + separate specification of the first distance).

[havatv]

It understand that it is too late to fix the code for the first version 6 release, but following my arguments above, I suggest that the documentation states the wanted behaviour (centre-to-centre). Then the code can be fixed in time for the next point release.

[tbonfort]

I'm not against fixing this once 6.0 has been released.

I'm not sure if it should be in 6.0.1 or 6.2 though, thoughts?

[tbonfort]

committed in r12711 alongside the fixes for #3879.

This change needs to be reflected in the docs, and should also probably be mentionned in the migration guide.

[havatv]

Very good! The style document has been updated in trunk (r12720). Very little was changed, since I had already documented the new behaviour (with a note that pointed out the "shortcomings" of the earlier implementations).

It must definitely be mentioned in the migration guide! Since Thomas already has added a note in the migration guide, I am closing this one.

[havatv]

The trunk symbol construction document has been updated (r12724).