Remove unused attributes from parent elements

package.sh

@@ -1,5 +1,5 @@
 #!/bin/bash
-SCOURVER="0.19"
+SCOURVER="0.20"
 cd ..
 zip scour/tarballs/scour-$SCOURVER.zip scour/scour.py scour/svg_regex.py scour/LICENSE scour/NOTICE scour/README.txt scour/release-notes.html
 cd scour

release-notes.html

@@ -9,14 +9,27 @@
 
 <p>Copyright 2009, Jeff Schiller</p>
 
+<section id="0.20">
+	<header>
+		<h2><a href="#0.20">Version 0.20</a></h2>
+	</header>
+	<p>Aug ??th, 2009</p>
+	<ul>
+		<li>Remove unused attributes from parent elements</li>
+	</ul>
+</section>
+
 <section id="0.19">
 	<header>
 		<h2><a href="#0.19">Version 0.19</a></h2>
 	</header>
-	<p>Aug 9th, 2009</p>
+	<p>Aug 13th, 2009</p>
 	<ul>
 		<li>Fix XML serialization bug: xmlns:XXX prefixes not preserved when not in default namespace</li>
 		<li>Fix XML serialization bug: remapping to default namespace was not actually removing the old prefix</li>
+		<li>Move common attributes to ancestor elements</li>
+		<li>Fix <a href="https://bugs.launchpad.net/scour/+bug/401628">Bug 412754</a>: Elliptical arc commands must have comma/whitespace separating the coordinates</li>
+		<li>Scour lengths for svg x,y,width,height,*opacity,stroke-width,stroke-miterlimit</li>
 	</ul>
 </section>
 

scour.py

@@ -32,19 +32,9 @@
 # Even more ideas here: http://esw.w3.org/topic/SvgTidy
 #  * analysis of path elements to see if rect can be used instead? (must also need to look
 #    at rounded corners)
-#  * removal of unused attributes in groups:
-#    <g fill="blue" ...>
-#      <rect fill="red" ... />
-#      <rect fill="red" ... />
-#      <rect fill="red" ... />
-#    </g>
-#    in this case, fill="blue" should be removed
 
 # Next Up:
-# + analyze all children of a group, if they have common inheritable attributes, then move them to the group
+# + remove unused attributes in parent elements
-# - crunch *opacity, offset, svg:x,y, stroke-miterlimit, stroke-width numbers (remove trailing zeros, reduce prec, integerize)
-# - analyze a group and its children, if a group's attribute is not being used by any children 
-#   (or descendants?) then remove it
 # - add an option to remove ids if they match the Inkscape-style of IDs
 # - investigate point-reducing algorithms
 # - parse transform attribute
@@ -52,7 +42,6 @@
 # - option to remove metadata
 # - prevent elements from being stripped if they are referenced in a <style> element
 #   (for instance, filter, marker, pattern) - need a crude CSS parser
-# - add an option for svgweb compatible markup (no self-closing tags)?
 
 # necessary to get true division
 from __future__ import division
@@ -76,7 +65,7 @@ except ImportError:
 	Decimal = FixedPoint	
 
 APP = 'scour'
-VER = '0.19'
+VER = '0.20'
 COPYRIGHT = 'Copyright Jeff Schiller, 2009'
 
 NS = { 	'SVG': 		'http://www.w3.org/2000/svg', 
@@ -632,9 +621,9 @@ def removeNestedGroups(node):
 
 def moveCommonAttributesToParentGroup(elem):
 	""" 
-	This iterates through all children of the passed in g element and 
+	This recursively calls this function on all children of the passed in element
-	removes common inheritable attributes from the children and places
+	and then iterates over all child elements and removes common inheritable attributes 
-	them in the parent group
+	from the children and places them in the parent group.
 	"""
 	num = 0
 	
@@ -703,6 +692,58 @@ def moveCommonAttributesToParentGroup(elem):
 	num += (len(childElements)-1) * len(commonAttrs)
 	return num
 
+def removeUnusedAttributesOnParent(elem):
+	"""
+	This recursively calls this function on all children of the element passed in,
+	then removes any unused attributes on this elem if none of the children inherit it
+	"""
+	num = 0
+
+	childElements = []
+	# recurse first into the children (depth-first)
+	for child in elem.childNodes:
+		if child.nodeType == 1: 
+			childElements.append(child)
+			num += removeUnusedAttributesOnParent(child)
+	
+	# only process the children if there are more than one element
+	if len(childElements) <= 1: return num
+
+	# get all attribute values on this parent
+	attrList = elem.attributes
+	unusedAttrs = {}
+	for num in range(attrList.length):
+		attr = attrList.item(num)
+		if attr.nodeName in ['clip-rule',
+					'display-align', 
+					'fill', 'fill-opacity', 'fill-rule', 
+					'font', 'font-family', 'font-size', 'font-size-adjust', 'font-stretch',
+					'font-style', 'font-variant', 'font-weight',
+					'letter-spacing',
+					'pointer-events', 'shape-rendering',
+					'stroke', 'stroke-dasharray', 'stroke-dashoffset', 'stroke-linecap', 'stroke-linejoin',
+					'stroke-miterlimit', 'stroke-opacity', 'stroke-width',
+					'text-anchor', 'text-decoration', 'text-rendering', 'visibility', 
+					'word-spacing', 'writing-mode']:
+			unusedAttrs[attr.nodeName] = attr.nodeValue
+	
+	# for each child, if at least one child inherits the parent's attribute, then remove
+	for childNum in range(len(childElements)):
+		child = childElements[childNum]
+		inheritedAttrs = []
+		for name in unusedAttrs.keys():
+			val = child.getAttribute(name)
+			if val == '' or val == None or val == 'inherit':
+				inheritedAttrs.append(name)
+		for a in inheritedAttrs:
+			del unusedAttrs[a]
+	
+	# unusedAttrs now has all the parent attributes that are unused
+	for name in unusedAttrs.keys():
+		elem.removeAttribute(name)
+	
+	return num
+	
 def removeDuplicateGradientStops(doc):
 	global numElemsRemoved
 	num = 0
@@ -2138,9 +2179,11 @@ def scourString(in_string, options=None):
 			pass
 
 	# move common attributes to parent group
-	# TODO: should make sure this is called with most-nested groups first
 	numAttrsRemoved += moveCommonAttributesToParentGroup(doc.documentElement)
 	
+	# remove unused attributes from parent
+	numAttrsRemoved += removeUnusedAttributesOnParent(doc.documentElement)
+
 	while removeDuplicateGradientStops(doc) > 0:
 		pass
 	
@@ -2169,8 +2212,10 @@ def scourString(in_string, options=None):
 	
 	# scour lengths (including coordinates)
 	for type in ['svg', 'image', 'rect', 'circle', 'ellipse', 'line', 'linearGradient', 'radialGradient', 'stop']:
-		for elem in doc.documentElement.getElementsByTagName(type):
+		for elem in doc.getElementsByTagName(type):
-			for attr in ['x', 'y', 'width', 'height', 'cx', 'cy', 'r', 'rx', 'ry', 'x1', 'y1', 'x2', 'y2', 'fx', 'fy', 'offset']:
+			for attr in ['x', 'y', 'width', 'height', 'cx', 'cy', 'r', 'rx', 'ry', 
+						'x1', 'y1', 'x2', 'y2', 'fx', 'fy', 'offset', 'opacity',
+						'fill-opacity', 'stroke-opacity', 'stroke-width', 'stroke-miterlimit']:
 				if elem.getAttribute(attr) != '':
 					elem.setAttribute(attr, scourLength(elem.getAttribute(attr)))
 

testscour.py

@@ -720,15 +720,19 @@ class CollapseSamePathPoints(unittest.TestCase):
 
 class ScourUnitlessLengths(unittest.TestCase):
 	def runTest(self):
-		r = scour.scourXmlFile('unittests/scour-lengths.svg').getElementsByTagNameNS(SVGNS, 'rect')[0];
+		doc = scour.scourXmlFile('unittests/scour-lengths.svg')
+		r = doc.getElementsByTagNameNS(SVGNS, 'rect')[0];
+		svg = doc.documentElement
+		self.assertEquals(svg.getAttribute('x'), '1',
+			'Did not scour x attribute of svg element with unitless number')
 		self.assertEquals(r.getAttribute('x'), '123.46',
-			'Did not scour x attribute unitless number')
+			'Did not scour x attribute of rect with unitless number')
 		self.assertEquals(r.getAttribute('y'), '123',
-			'Did not scour y attribute unitless number')
+			'Did not scour y attribute of rect unitless number')
 		self.assertEquals(r.getAttribute('width'), '300',
-			'Did not scour width attribute unitless number')
+			'Did not scour width attribute of rect with unitless number')
 		self.assertEquals(r.getAttribute('height'), '100',
-			'Did not scour height attribute unitless number')
+			'Did not scour height attribute of rect with unitless number')
 
 class ScourLengthsWithUnits(unittest.TestCase):
 	def runTest(self):
@@ -899,6 +903,19 @@ class PathEllipticalArcParsingCommaWsp(unittest.TestCase):
 		self.assertEquals( p.getAttribute('d'), 'M100,100a100,100,0,1,1,-50,100z',
 			'Did not parse elliptical arc command properly')
 
+class RemoveUnusedAttributesOnParent(unittest.TestCase):
+	def runTest(self):
+		g = scour.scourXmlFile('unittests/remove-unused-attributes-on-parent.svg').getElementsByTagNameNS(SVGNS, 'g')[0]
+		self.assertNotEquals( g.getAttribute('stroke'), '#000',
+			'Unused attributes on group not removed')
+
+class DoNotRemoveCommonAttributesOnParentIfAtLeastOneUsed(unittest.TestCase):
+	def runTest(self):
+		g = scour.scourXmlFile('unittests/remove-unused-attributes-on-parent.svg').getElementsByTagNameNS(SVGNS, 'g')[0]
+		self.assertEquals( g.getAttribute('fill'), '#0F0',
+			'Used attributes on group were removed')
+
+
 # TODO; write a test for embedding rasters
 # TODO: write a test for --disable-embed-rasters
 # TODO: write tests for --keep-editor-data

unittests/move-common-attributes-to-grandparent.svg

@@ -0,0 +1,10 @@
+<?xml version="1.0" encoding="UTF-8" standalone="no"?>
+<svg xmlns="http://www.w3.org/2000/svg">
+<g id="grampa" fill-opacity="0.4">
+  <g stroke-opacity="0.8">
+    <rect fill="#0F0" stroke="#0F0" stroke-width="5" width="100" height="300"/>
+    <rect fill="#0F0" width="200" height="100" />
+  </g>
+  <circle fill="#0F0" stroke="0F0" cx="50" cy="50" r="20" />
+</g>
+</svg>

unittests/remove-unused-attributes-on-parent.svg

@@ -0,0 +1,8 @@
+<?xml version="1.0" encoding="UTF-8" standalone="no"?>
+<svg xmlns="http://www.w3.org/2000/svg">
+ <g fill="#0F0" stroke="#000">
+  <rect stroke="#0F0" stroke-width="5" width="100" height="300"/>
+  <rect fill="#FFF" stroke="#000" width="200" height="100"/>
+  <circle fill="#0F0" stroke="#0F0" cx="50" cy="50" r="20"/>
+ </g>
+</svg>

unittests/scour-lengths.svg

@@ -1,5 +1,5 @@
 <?xml version="1.0" encoding="UTF-8" standalone="no"?>
-<svg xmlns="http://www.w3.org/2000/svg">
+<svg xmlns="http://www.w3.org/2000/svg" x="1.0000000" y="1.0000000">
 	<rect x="123.4567" y="123.00" width="300.00001" height="1E+02" fill="lime" />
 	<rect x="123.4567px" y="35.000ex" width="300.00001pt" height="5E+01%" fill="blue" />
 </svg>