From eedd2f72a2d8db878af4fff1d16d6841d7a9a8c3 Mon Sep 17 00:00:00 2001
From: Dane Springmeyer <dane@dbsgeo.com>
Date: Tue, 23 Feb 2010 02:17:34 +0000
Subject: [PATCH] load_map: fix the behavior of PolygonPatternSymbolizer and
 LinePatternSymbolizerby truly making it optional to not supply the width,
 height, and type parameters and allow them to be read automatically - closes
 #508

---
 CHANGELOG                                     |  4 ++
 src/load_map.cpp                              | 24 ++++--------
 tests/data/good_maps/point_symbolizer.xml     | 19 ++++++++-
 .../point_symbolizer_on_polygon_map.xml       | 39 +++++++++++++++++++
 4 files changed, 67 insertions(+), 19 deletions(-)
 create mode 100644 tests/data/good_maps/point_symbolizer_on_polygon_map.xml

diff --git a/CHANGELOG b/CHANGELOG
index 796260acf..462e764ce 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -15,6 +15,10 @@ For a complete change history, see the SVN log.
 Mapnik 0.7.1 Release
 --------------------
 
+- XML: Fixed behavior of PolygonPatternSymbolizer and LinePatternSymbolizer whereby width, height,
+  and type of images is actually allowed to be optionally ommitted (#508). This was added in r1543 but
+  only worked correctly for PointSymbolizer and ShieldSymbolizer. 
+
 - Fixed reading of PostGIS data on Big Endian systems (#515)
 
 - PostGIS: Added better support for alterative schemas (#500)
diff --git a/src/load_map.cpp b/src/load_map.cpp
index 8f8b2af7a..ea217fe12 100644
--- a/src/load_map.cpp
+++ b/src/load_map.cpp
@@ -661,7 +661,7 @@ void map_parser::parse_point_symbolizer( rule_type & rule, ptree const & sym )
             
 	optional<unsigned> width = get_opt_attr<unsigned>(sym, "width");
 	optional<unsigned> height = get_opt_attr<unsigned>(sym, "height");
-	//   
+	   
 	if (file)
 	{              
             try
@@ -724,16 +724,6 @@ void map_parser::parse_point_symbolizer( rule_type & rule, ptree const & sym )
 		}
             }
 	}
-	//else if (file || type || width || height)
-	//{
-	//   std::ostringstream os;
-	//  os << "Missing required attributes: ";
-	//  if ( ! file ) os << "file ";
-	//  if ( ! type ) os << "type ";
-	//  if ( ! width ) os << "width ";
-	//  if ( ! height ) os << "height ";
-	//  throw config_error( os.str() );
-	//}
 	else
 	{
             rule.append(point_symbolizer());
@@ -752,9 +742,9 @@ void map_parser::parse_line_pattern_symbolizer( rule_type & rule, ptree const &
     {
 	std::string file = get_attr<string>(sym, "file");
 	optional<std::string> base = get_opt_attr<string>(sym, "base");
-	optional<std::string> type = get_attr<string>(sym, "type");
-	optional<unsigned> width = get_attr<unsigned>(sym, "width");
-	optional<unsigned> height = get_attr<unsigned>(sym, "height");
+	optional<std::string> type = get_opt_attr<string>(sym, "type");
+	optional<unsigned> width = get_opt_attr<unsigned>(sym, "width");
+	optional<unsigned> height = get_opt_attr<unsigned>(sym, "height");
 
 	try
 	{
@@ -822,9 +812,9 @@ void map_parser::parse_polygon_pattern_symbolizer( rule_type & rule,
     {
 	std::string file = get_attr<string>(sym, "file");
 	optional<std::string> base = get_opt_attr<string>(sym, "base");
-	optional<std::string> type = get_attr<string>(sym, "type");
-	optional<unsigned> width = get_attr<unsigned>(sym, "width");
-	optional<unsigned> height = get_attr<unsigned>(sym, "height");
+	optional<std::string> type = get_opt_attr<string>(sym, "type");
+	optional<unsigned> width = get_opt_attr<unsigned>(sym, "width");
+	optional<unsigned> height = get_opt_attr<unsigned>(sym, "height");
           
 	try
 	{
diff --git a/tests/data/good_maps/point_symbolizer.xml b/tests/data/good_maps/point_symbolizer.xml
index b6cb927f7..00b4b06c4 100644
--- a/tests/data/good_maps/point_symbolizer.xml
+++ b/tests/data/good_maps/point_symbolizer.xml
@@ -1,6 +1,6 @@
 <?xml version="1.0" encoding="utf-8"?>
 <!DOCTYPE Map  >
-<Map bgcolor="#b5d0d0" srs="+proj=merc +datum=WGS84  +k=1.0 +units=m +over +no_defs" paths_from_xml="false">
+<Map bgcolor="#b5d0d0" srs="+proj=merc +datum=WGS84  +k=1.0 +units=m +over +no_defs" min_version="0.7.1">
     <Style name="test 1">
         <Rule name="rule t1">
             <Filter>[landuse] = 'meadow' or [landuse] = 'wood' or [landuse] = 'forest'</Filter>
@@ -12,7 +12,22 @@
         <Rule title="foo">
             <ElseFilter/>
             <MaxScaleDenominator>1000000</MaxScaleDenominator>
-            <PointSymbolizer file="../data/images/dummy.png" type="png" width="16" height="16"/>
+            <PointSymbolizer file="../images/dummy.png" type="png" width="16" height="16"/>
+        </Rule>
+    </Style>
+    <Style name="test 3">
+        <Rule title="foo">
+            <PointSymbolizer file="../images/dummy.png" type="png"/>
+        </Rule>
+    </Style>
+    <Style name="test 4">
+        <Rule title="foo">
+            <PointSymbolizer file="../images/dummy.png" width="16" height="16"/>
+        </Rule>
+    </Style>
+    <Style name="test 5">
+        <Rule title="foo">
+            <PointSymbolizer file="../images/dummy.png"/>
         </Rule>
     </Style>
 </Map>
diff --git a/tests/data/good_maps/point_symbolizer_on_polygon_map.xml b/tests/data/good_maps/point_symbolizer_on_polygon_map.xml
new file mode 100644
index 000000000..3caff1170
--- /dev/null
+++ b/tests/data/good_maps/point_symbolizer_on_polygon_map.xml
@@ -0,0 +1,39 @@
+<?xml version="1.0" encoding="utf-8"?>
+<!DOCTYPE Map  >
+<Map bgcolor="#b5d0d0" min_version="0.7.1">
+    <Style name="test">
+        <Rule title="asia">
+            <Filter>([REGION]=142)</Filter>
+            <PointSymbolizer file="../images/dummy.png" allow_overlap="true"/>
+        </Rule>
+        <Rule title="europe">
+            <Filter>([REGION]=150)</Filter>
+            <!-- requires at least Mapnik 0.7.1 to work due to http://trac.mapnik.org/ticket/508 -->
+            <PolygonPatternSymbolizer file="../images/dummy.png" />
+        </Rule>
+        <Rule title="americas">
+            <Filter>([REGION]=19)</Filter>
+            <!-- requires at least Mapnik 0.7.1 to work due to http://trac.mapnik.org/ticket/508 -->
+            <LinePatternSymbolizer file="../images/dummy.png" />
+        </Rule>
+        <Rule title="Africa">
+            <Filter>([REGION]=2)</Filter>
+            <ShieldSymbolizer name="NAME" placement="vertex" face_name="DejaVu Sans Bold" size="10" fill="#000000" file="../images/dummy.png" />
+        </Rule>
+
+        <Rule title="rest">
+            <ElseFilter />
+            <LineSymbolizer>
+                    <CssParameter name="stroke-width">.1</CssParameter>
+            </LineSymbolizer>
+        </Rule>
+    </Style>
+
+    <Layer name="world_merc">
+        <StyleName>test</StyleName>
+        <Datasource>
+            <Parameter name="file">../../data/shp/world_merc</Parameter>
+            <Parameter name="type">shape</Parameter>
+        </Datasource>
+    </Layer>
+</Map>