Merge pull request #108 from Ede123/precision

Improve path scouring

scour/scour.py

@@ -64,12 +64,7 @@ import optparse
 from scour.yocto_css import parseCssString
 import six
 from six.moves import range
-
-# Python 2.3- did not have Decimal
-try:
-   from decimal import Decimal, InvalidOperation, getcontext
-except ImportError:
-   sys.stderr.write("Scour requires at least Python 2.7 or Python 3.3+.")
+from decimal import Context, Decimal, InvalidOperation, getcontext
 
 # select the most precise walltime measurement function available on the platform
 if sys.platform.startswith('win'):
@@ -2291,8 +2286,12 @@ def cleanPath(element, options) :
    path = newPath
 
    newPathStr = serializePath(path, options)
-   numBytesSavedInPathData += ( len(oldPathStr) - len(newPathStr) )
-   element.setAttribute('d', newPathStr)
+
+   # if for whatever reason we actually made the path longer don't use it
+   # TODO: maybe we could compare path lengths after each optimization step and use the shortest
+   if len(newPathStr) <= len(oldPathStr):
+      numBytesSavedInPathData += ( len(oldPathStr) - len(newPathStr) )
+      element.setAttribute('d', newPathStr)
 
 
 
@@ -2474,13 +2473,19 @@ def scourUnitlessLength(length, needsRendererWorkaround=False): # length is of a
    This is faster than scourLength on elements guaranteed not to
    contain units.
    """
-   # reduce to the proper number of digits
    if not isinstance(length, Decimal):
       length = getcontext().create_decimal(str(length))
-   # if the value is an integer, it may still have .0[...] attached to it for some reason
-   # remove those
-   if int(length) == length:
-      length = getcontext().create_decimal(int(length))
+
+   # reduce numeric precision
+   # plus() corresponds to the unary prefix plus operator and applies context precision and rounding
+   length = scouringContext.plus(length)
+
+   # remove trailing zeroes as we do not care for significance
+   intLength = length.to_integral_value()
+   if length == intLength:
+      length = Decimal(intLength)
+   else:
+      length = length.normalize()
 
    # gather the non-scientific notation version of the coordinate.
    # this may actually be in scientific notation if the value is
@@ -2492,14 +2497,22 @@ def scourUnitlessLength(length, needsRendererWorkaround=False): # length is of a
       elif len(nonsci) > 3 and nonsci[:3] == '-0.':
          nonsci = '-' + nonsci[2:] # remove the 0, leave the minus and dot
 
-   if len(nonsci) > 3: # avoid calling normalize unless strictly necessary
-      # and then the scientific notation version, with E+NUMBER replaced with
-      # just eNUMBER, since SVG accepts this.
-      sci = six.text_type(length.normalize()).lower().replace("e+", "e")
+   # Gather the scientific notation version of the coordinate which
+   # can only be shorter if the length of the number is at least 4 characters (e.g. 1000 = 1e3).
+   if len(nonsci) > 3: 
+      # We have to implement this ourselves since both 'normalize()' and 'to_sci_string()'
+      # don't handle negative exponents in a reasonable way (e.g. 0.000001 remains unchanged)
+      exponent = length.adjusted() # how far do we have to shift the dot?
+      length = length.scaleb(-exponent).normalize() # shift the dot and remove potential trailing zeroes
 
-      if len(sci) < len(nonsci): return sci
-      else: return nonsci
-   else: return nonsci
+      sci = six.text_type(length) + 'e' + six.text_type(exponent)
+
+      if len(sci) < len(nonsci):
+         return sci
+      else:
+         return nonsci
+   else:
+      return nonsci
 
 
 
@@ -3071,7 +3084,11 @@ def scourString(in_string, options=None):
    # sanitize options (take missing attributes from defaults, discard unknown attributes)
    options = sanitizeOptions(options)
 
-   getcontext().prec = options.digits
+   # create decimal context with reduced precision for scouring numbers
+   # calculations should be done in the default context (precision defaults to 28 significant digits) to minimize errors
+   global scouringContext
+   scouringContext = Context(prec = options.digits)
+
    global numAttrsRemoved
    global numStylePropsFixed
    global numElemsRemoved

testscour.py

@@ -685,6 +685,14 @@ class ChangeQuadToShorthandInPath(unittest.TestCase):
 		self.assertEqual(path.getAttribute('d'), 'm10 100q50-50 100 0t100 0',
 			'Did not change quadratic curves into shorthand curve segments in path')
 
+class DoNotOptimzePathIfLarger(unittest.TestCase):
+	def runTest(self):
+		p = scour.scourXmlFile('unittests/path-no-optimize.svg').getElementsByTagNameNS(SVGNS, 'path')[0];
+		self.assertTrue(len(p.getAttribute('d')) <= len("M100,100 L200.12345,200.12345 C215,205 185,195 200.12,200.12 Z"),
+			'Made path data longer during optimization')
+        # this was the scoured path data as of 2016-08-31 without the length check in cleanPath(): 
+        #    d="m100 100l100.12 100.12c14.877 4.8766-15.123-5.1234-0.00345-0.00345z"
+
 class HandleEncodingUTF8(unittest.TestCase):
 	def runTest(self):
 		doc = scour.scourXmlFile('unittests/encoding-utf8.svg')
@@ -877,7 +885,7 @@ class RereferenceForRadialGradient(unittest.TestCase):
 class CollapseSamePathPoints(unittest.TestCase):
 	def runTest(self):
 		p = scour.scourXmlFile('unittests/collapse-same-path-points.svg').getElementsByTagNameNS(SVGNS, 'path')[0];
-		self.assertEqual(p.getAttribute('d'), "m100 100l100.12 100.12c14.88 4.88-15.12-5.12 0 0z",
+		self.assertEqual(p.getAttribute('d'), "m100 100l100.12 100.12c14.877 4.8766-15.123-5.1234 0 0z",
 			'Did not collapse same path points')
 
 class ScourUnitlessLengths(unittest.TestCase):

unittests/collapse-same-path-points.svg

@@ -1,4 +1,4 @@
 <?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" width="210" height="210">
-  <path stroke="yellow" fill="red" d="M100,100 L200.12345,200.12345 C215,205 185,195 200.12,200.12 Z"/>
+  <path stroke="yellow" fill="red" d="M100,100 L200.12345,200.12345 C215,205 185,195 200.12345,200.12345 Z"/>
 </svg>

unittests/path-no-optimize.svg

@@ -0,0 +1,4 @@
+<?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" width="210" height="210">
+  <path stroke="yellow" fill="red" d="M100,100 L200.12345,200.12345 C215,205 185,195 200.12,200.12 Z"/>
+</svg>