Only convert color names to #RRGGBB if it will actually be shorter

release-notes.html

@@ -9,6 +9,16 @@
 
 <p>Copyright 2009, Jeff Schiller</p>
 
+<section id="0.17">
+	<header>
+		<h2><a href="#0.17">Version 0.17</a></h2>
+	</header>
+	<p>July 30th, 2009</p>
+	<ul>
+		<li>Only convert to #RRGGBB format if the color name will actually be shorter</li>
+	</ul>
+</section>
+
 <section id="0.16">
 	<header>
 		<h2><a href="#0.16">Version 0.16</a></h2>

scour.py

@@ -38,6 +38,7 @@
 #    This would require my own serialization of the DOM objects (not impossible)
 
 # Next Up:
+# - remove duplicate gradients
 # - scour polyline coordinates just like path coordinates
 # - if after reducing precision we have duplicate path segments, then remove the duplicates and
 #   leave it as a straight line segment
@@ -69,7 +70,7 @@ except ImportError:
 	Decimal = FixedPoint	
 
 APP = 'scour'
-VER = '0.16'
+VER = '0.17'
 COPYRIGHT = 'Copyright Jeff Schiller, 2009'
 
 NS = { 	'SVG': 		'http://www.w3.org/2000/svg', 
@@ -673,7 +674,7 @@ def collapseSinglyReferencedGradients(doc):
 						for stop in stopsToAdd:
 							refElem.appendChild(stop)
 							
-					# adopt the gradientUnits, spreadMethod,  gradientTransform attributess if
+					# adopt the gradientUnits, spreadMethod,  gradientTransform attributes if
 					# they are unspecified on refElem
 					for attr in ['gradientUnits','spreadMethod','gradientTransform']:
 						if refElem.getAttribute(attr) == '' and not elem.getAttribute(attr) == '':
@@ -699,10 +700,19 @@ def collapseSinglyReferencedGradients(doc):
 					# now delete elem
 					elem.parentNode.removeChild(elem)
 					numElemsRemoved += 1
-					num += 1
-					
+					num += 1		
 	return num
+
+def removeDuplicateGradients(doc):
+	global numElemsRemoved
+	num = 0
 	
+	for gradType in ['linearGradient', 'radialGradient']:
+		for grad in doc.getElementsByTagNameNS(NS['SVG'], gradType):
+			pass
+	
+	return num
+
 def repairStyle(node, options):
 	num = 0
 	if node.nodeType == 1 and len(node.getAttribute('style')) > 0 :	
@@ -906,7 +916,7 @@ def convertColor(value):
 	
 def convertColors(element) :
 	"""
-		Recursively converts all color properties into #RRGGBB format
+		Recursively converts all color properties into #RRGGBB format if shorter
 	"""
 	numBytes = 0
 	
@@ -924,11 +934,14 @@ def convertColors(element) :
 
 	# now convert all the color formats
 	for attr in attrsToConvert:
-		val = element.getAttribute(attr)
-		oldBytes = len(val)
-		if val != '':
-			element.setAttribute(attr, convertColor(val))
-			numBytes += (oldBytes - len(element.getAttribute(attr)))
+		oldColorValue = element.getAttribute(attr)
+		if oldColorValue != '':
+			newColorValue = convertColor(oldColorValue)
+			oldBytes = len(oldColorValue)
+			newBytes = len(newColorValue)
+			if oldBytes > newBytes:
+				element.setAttribute(attr, newColorValue)
+				numBytes += (oldBytes - len(element.getAttribute(attr)))
 	
 	# now recurse for our child elements
 	for child in element.childNodes :
@@ -1604,6 +1617,10 @@ def scourString(in_string, options=None):
 	# remove gradients that are only referenced by one other gradient
 	while collapseSinglyReferencedGradients(doc) > 0:
 		pass
+		
+	# remove duplicate gradients
+	while removeDuplicateGradients(doc) > 0:
+		pass
 	
 	# clean path data
 	for elem in doc.documentElement.getElementsByTagNameNS(NS['SVG'], 'path') :

testscour.py

@@ -583,7 +583,7 @@ class TranslateColorNamesIntoHex(unittest.TestCase):
 class TranslateExtendedColorNamesIntoHex(unittest.TestCase):
 	def runTest(self):
 		elem = scour.scourXmlFile('unittests/color-formats.svg').getElementsByTagNameNS(SVGNS, 'solidColor')[0]
-		self.assertEquals( elem.getAttribute('solid-color'), '#800000',
+		self.assertEquals( elem.getAttribute('solid-color'), '#FAFAD2',
 			'Not converting extended color names into hex')
 
 class TranslateLongHexColorIntoShortHex(unittest.TestCase):
@@ -591,7 +591,13 @@ class TranslateLongHexColorIntoShortHex(unittest.TestCase):
 		elem = scour.scourXmlFile('unittests/color-formats.svg').getElementsByTagNameNS(SVGNS, 'ellipse')[0]
 		self.assertEquals( elem.getAttribute('fill'), '#FFF',
 			'Not converting long hex color into short hex')
-		
+
+class DoNotConvertShortColorNames(unittest.TestCase):
+	def runTest(self):
+		elem = scour.scourXmlFile('unittests/dont-convert-short-color-names.svg').getElementsByTagNameNS(SVGNS, 'rect')[0]
+		self.assertEquals( 'red', elem.getAttribute('fill'),
+			'Converted short color name to longer hex string')
+
 class AllowQuotEntitiesInUrl(unittest.TestCase):
 	def runTest(self):
 		grads = scour.scourXmlFile('unittests/quot-in-url.svg').getElementsByTagNameNS(SVGNS, 'linearGradient')
@@ -658,6 +664,34 @@ class AlwaysKeepClosePathSegments(unittest.TestCase):
 		self.assertEquals(p.getAttribute('d'), 'M10,10h100v100h-100z',
 			'Path with closepath not preserved')
 
+class RemoveDuplicateLinearGradients(unittest.TestCase):
+	def runTest(self):
+		svgdoc = scour.scourXmlFile('unittests/remove-duplicate-gradients.svg')
+		lingrads = svgdoc.getElementsByTagNameNS(SVGNS, 'linearGradient')
+		self.assertEquals(1, lingrads.length,
+			'Duplicate linear gradient not removed')
+		
+class RereferenceForLinearGradient(unittest.TestCase):
+	def runTest(self):
+		svgdoc = scour.scourXmlFile('unittests/remove-duplicate-gradients.svg')
+		rects = svgdoc.getElementsByTagNameNS(SVGNS, 'rect')
+		self.assertEquals(rects[0].getAttribute('fill'), rects[1].getAttribute('stroke'),
+			'Rect not changed after removing duplicate linear gradient')
+			
+class RemoveDuplicateRadialGradients(unittest.TestCase):
+	def runTest(self):
+		svgdoc = scour.scourXmlFile('unittests/remove-duplicate-gradients.svg')
+		radgrads = svgdoc.getElementsByTagNameNS(SVGNS, 'radialGradient')
+		self.assertEquals(1, radgrads.length,
+			'Duplicate radial gradient not removed')
+			
+class RereferenceForRadialGradient(unittest.TestCase):
+	def runTest(self):
+		svgdoc = scour.scourXmlFile('unittests/remove-duplicate-gradients.svg')
+		rects = svgdoc.getElementsByTagNameNS(SVGNS, 'rect')
+		self.assertEquals(rects[2].getAttribute('stroke'), rects[3].getAttribute('fill'),
+			'Rect not changed after removing duplicate radial gradient')
+
 # TODO; write a test for embedding rasters
 # TODO: write a test for --disable-embed-rasters
 # TODO: write tests for --keep-editor-data

unittests/color-formats.svg

@@ -3,7 +3,7 @@
   <linearGradient id="g1" x1="0" y1="0" x2="1" y2="0">
     <stop offset="0.5" stop-color="rgb(50.0%, 0%, .0%)" />
   </linearGradient>
-  <solidColor id="c1" solid-color="maroon"/>
+  <solidColor id="c1" solid-color="lightgoldenrodyellow"/>
 </defs>
   <rect id="rect" width="100" height="100" fill="rgb(15,16,17)" stroke="darkgrey" />
   <circle id="circle" cx="100" cy="100" r="30" fill="url(#g1)" stroke="url(#c1)" />

unittests/remove-duplicate-gradients.svg

@@ -0,0 +1,18 @@
+<?xml version="1.0" encoding="UTF-8" standalone="no"?>
+<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink">
+	<linearGradient id="g1" x1="0" y1="0" x2="1" y2="1">
+		<stop offset="0" stop-color="red"/>
+		<stop offset="1" stop-color="blue"/>
+	</linearGradient>
+	<linearGradient id='g2' x1='0' y1='0' x2='1' y2='1'>
+		<stop offset='0' stop-color='red'/>
+		<stop offset='1' stop-color='blue'/>
+	</linearGradient>
+	<radialGradient xlink:href="#g1" id="g3" gradientUnits="userSpaceOnUse" gradientTransform="matrix(1, 0, 0, 0.481529, -718.417, 98.7046)" cx="323.75433" cy="209.73672" fx="323.75433" fy="209.73672" r="6.2794499"/>
+	<radialGradient xlink:href="#g2" id="g4" gradientUnits="userSpaceOnUse" gradientTransform="matrix(1, 0, 0, 0.481529, -718.417, 98.7046)" cx="323.75433" cy="209.73672" fx="323.75433" fy="209.73672" r="6.2794499"/>
+	
+	<rect id="r1" fill="url(#g1)" width="100" height="100"/>
+	<rect id="r2" stroke="url(#g2)" width="100" height="100"/>
+	<rect id="r3" stroke="url(#g3)" width="100" height="100"/>
+	<rect id="r4" fill="url(#g4)" width="100" height="100"/>
+</svg>