From cc592c8e8a870aa7d76d54413c7bf3f69310898e Mon Sep 17 00:00:00 2001 From: Eduard Braun Date: Thu, 18 May 2017 00:53:25 +0200 Subject: [PATCH] Improve and fix behaviour when collapsing straight paths segments (#146) * Do not collapse straight path segments in paths that have intermediate markers (see #145). The intermediate nodes might be unnecessary for the shape of the path, but their markers would be lost. * Collapse subpaths of moveto `m` and lineto `l` commands if they have the same direction (before we only collapsed horizontal/vertical `h`/`v` lineto commands) * Attempt to collapse lineto `l` commands into a preceding moveto `m` command (these are then called "implicit lineto commands") * Preserve empty path segments if they have `stroke-linecap` set to `round` or `square`. They render no visible line but a tiny dot or square. --- scour/scour.py | 127 ++++++++++++------ testscour.py | 97 ++++++++----- unittests/collapse-straight-path-segments.svg | 33 +++++ unittests/consecutive-hlines.svg | 6 - unittests/path-precision.svg | 8 +- unittests/path-with-caps.svg | 8 +- 6 files changed, 194 insertions(+), 85 deletions(-) create mode 100644 unittests/collapse-straight-path-segments.svg delete mode 100644 unittests/consecutive-hlines.svg diff --git a/scour/scour.py b/scour/scour.py index 6081a04..11791c2 100644 --- a/scour/scour.py +++ b/scour/scour.py @@ -403,7 +403,16 @@ default_properties = { # excluded all properties with 'auto' as default } -def isSameSign(a, b): return (a <= 0 and b <= 0) or (a >= 0 and b >= 0) +def is_same_sign(a, b): + return (a <= 0 and b <= 0) or (a >= 0 and b >= 0) + + +def is_same_direction(x1, y1, x2, y2): + if is_same_sign(x1, x2) and is_same_sign(y1, y2): + diff = y1/x1 - y2/x2 + return scouringContext.plus(1 + diff) == 1 + else: + return False scinumber = re.compile(r"[-+]?(\d*\.?)?\d+[eE][-+]?\d+") @@ -2044,10 +2053,23 @@ def cleanPath(element, options): # this gets the parser object from svg_regex.py oldPathStr = element.getAttribute('d') path = svg_parser.parse(oldPathStr) + style = _getStyle(element) - # This determines whether the stroke has round linecaps. If it does, - # we do not want to collapse empty segments, as they are actually rendered. - withRoundLineCaps = element.getAttribute('stroke-linecap') == 'round' + # This determines whether the stroke has round or square linecaps. If it does, we do not want to collapse empty + # segments, as they are actually rendered (as circles or squares with diameter/dimension matching the path-width). + has_round_or_square_linecaps = ( + element.getAttribute('stroke-linecap') in ['round', 'square'] + or 'stroke-linecap' in style and style['stroke-linecap'] in ['round', 'square'] + ) + + # This determines whether the stroke has intermediate markers. If it does, we do not want to collapse + # straight segments running in the same direction, as markers are rendered on the intermediate nodes. + has_intermediate_markers = ( + element.hasAttribute('marker') + or element.hasAttribute('marker-mid') + or 'marker' in style + or 'marker-mid' in style + ) # The first command must be a moveto, and whether it's relative (m) # or absolute (M), the first set of coordinates *is* absolute. So @@ -2057,7 +2079,7 @@ def cleanPath(element, options): # Reuse the data structure 'path', since we're not adding or removing subcommands. # Also reuse the coordinate lists since we're not adding or removing any. x = y = 0 - for pathIndex in range(0, len(path)): + for pathIndex in range(len(path)): cmd, data = path[pathIndex] # Changes to cmd don't get through to the data structure i = 0 # adjust abs to rel @@ -2158,8 +2180,8 @@ def cleanPath(element, options): # remove empty segments # Reuse the data structure 'path' and the coordinate lists, even if we're # deleting items, because these deletions are relatively cheap. - if not withRoundLineCaps: - for pathIndex in range(0, len(path)): + if not has_round_or_square_linecaps: + for pathIndex in range(len(path)): cmd, data = path[pathIndex] i = 0 if cmd in ['m', 'l', 't']: @@ -2253,26 +2275,25 @@ def cleanPath(element, options): prevData = [] newPath = [] for (cmd, data) in path: - # flush the previous command if it is not the same type as the current command - if prevCmd != '': - if cmd != prevCmd or cmd == 'm': - newPath.append((prevCmd, prevData)) - prevCmd = '' - prevData = [] - - # if the previous and current commands are the same type, - # or the previous command is moveto and the current is lineto, collapse, - # but only if they are not move commands (since move can contain implicit lineto commands) - if (cmd == prevCmd or (cmd == 'l' and prevCmd == 'm')) and cmd != 'm': - prevData.extend(data) - - # save last command and data - else: + if prevCmd == '': + # initialize with current path cmd and data prevCmd = cmd prevData = data + else: + # collapse if + # - cmd is not moveto (explicit moveto commands are not drawn) + # - the previous and current commands are the same type, + # - the previous command is moveto and the current is lineto + # (subsequent moveto pairs are treated as implicit lineto commands) + if cmd != 'm' and (cmd == prevCmd or (cmd == 'l' and prevCmd == 'm')): + prevData.extend(data) + # else flush the previous command if it is not the same type as the current command + else: + newPath.append((prevCmd, prevData)) + prevCmd = cmd + prevData = data # flush last command and data - if prevCmd != '': - newPath.append((prevCmd, prevData)) + newPath.append((prevCmd, prevData)) path = newPath # convert to shorthand path segments where possible @@ -2396,22 +2417,52 @@ def cleanPath(element, options): newPath.append((cmd, data)) path = newPath - # for each h or v, collapse unnecessary coordinates that run in the same direction - # i.e. "h-100-100" becomes "h-200" but "h300-100" does not change + # For each m, l, h or v, collapse unnecessary coordinates that run in the same direction + # i.e. "h-100-100" becomes "h-200" but "h300-100" does not change. + # If the path has intermediate markers we have to preserve intermediate nodes, though. # Reuse the data structure 'path', since we're not adding or removing subcommands. # Also reuse the coordinate lists, even if we're deleting items, because these # deletions are relatively cheap. - for pathIndex in range(1, len(path)): - cmd, data = path[pathIndex] - if cmd in ['h', 'v'] and len(data) > 1: - coordIndex = 1 - while coordIndex < len(data): - if isSameSign(data[coordIndex - 1], data[coordIndex]): - data[coordIndex - 1] += data[coordIndex] - del data[coordIndex] - _num_path_segments_removed += 1 - else: - coordIndex += 1 + if not has_intermediate_markers: + for pathIndex in range(len(path)): + cmd, data = path[pathIndex] + + # h / v expects only one parameter and we start drawing with the first (so we need at least 2) + if cmd in ['h', 'v'] and len(data) >= 2: + coordIndex = 0 + while coordIndex+1 < len(data): + if is_same_sign(data[coordIndex], data[coordIndex+1]): + data[coordIndex] += data[coordIndex+1] + del data[coordIndex+1] + _num_path_segments_removed += 1 + else: + coordIndex += 1 + + # l expects two parameters and we start drawing with the first (so we need at least 4) + elif cmd == 'l' and len(data) >= 4: + coordIndex = 0 + while coordIndex+2 < len(data): + if is_same_direction(*data[coordIndex:coordIndex+4]): + data[coordIndex] += data[coordIndex+2] + data[coordIndex+1] += data[coordIndex+3] + del data[coordIndex+2] # delete the next two elements + del data[coordIndex+2] + _num_path_segments_removed += 1 + else: + coordIndex += 2 + + # m expects two parameters but we have to skip the first pair as it's not drawn (so we need at least 6) + elif cmd == 'm' and len(data) >= 6: + coordIndex = 2 + while coordIndex+2 < len(data): + if is_same_direction(*data[coordIndex:coordIndex+4]): + data[coordIndex] += data[coordIndex+2] + data[coordIndex+1] += data[coordIndex+3] + del data[coordIndex+2] # delete the next two elements + del data[coordIndex+2] + _num_path_segments_removed += 1 + else: + coordIndex += 2 # it is possible that we have consecutive h, v, c, t commands now # so again collapse all consecutive commands of the same type into one command @@ -2542,7 +2593,7 @@ def controlPoints(cmd, data): """ cmd = cmd.lower() if cmd in ['c', 's', 'q']: - indices = range(0, len(data)) + indices = range(len(data)) if cmd == 'c': # c: (x1 y1 x2 y2 x y)+ return [(index % 6) < 4 for index in indices] elif cmd in ['s', 'q']: # s: (x2 y2 x y)+ q: (x1 y1 x y)+ diff --git a/testscour.py b/testscour.py index 894733b..8ebaf9d 100755 --- a/testscour.py +++ b/testscour.py @@ -964,10 +964,10 @@ class KeepPrecisionInPathDataIfSameLength(unittest.TestCase): doc = scourXmlFile('unittests/path-precision.svg', parse_args(['--set-precision=1'])) paths = doc.getElementsByTagNameNS(SVGNS, 'path') for path in paths[1:3]: - self.assertEqual(path.getAttribute('d'), "m1 12 123 1e3 1e4 1e5", + self.assertEqual(path.getAttribute('d'), "m1 21 321 4e3 5e4 7e5", 'Precision not correctly reduced with "--set-precision=1" ' 'for path with ID ' + path.getAttribute('id')) - self.assertEqual(paths[4].getAttribute('d'), "m-1-12-123-1e3 -1e4 -1e5", + self.assertEqual(paths[4].getAttribute('d'), "m-1-21-321-4e3 -5e4 -7e5", 'Precision not correctly reduced with "--set-precision=1" ' 'for path with ID ' + paths[4].getAttribute('id')) self.assertEqual(paths[5].getAttribute('d'), "m123 101-123-101", @@ -977,10 +977,10 @@ class KeepPrecisionInPathDataIfSameLength(unittest.TestCase): doc = scourXmlFile('unittests/path-precision.svg', parse_args(['--set-precision=2'])) paths = doc.getElementsByTagNameNS(SVGNS, 'path') for path in paths[1:3]: - self.assertEqual(path.getAttribute('d'), "m1 12 123 1234 12345 1.2e5", + self.assertEqual(path.getAttribute('d'), "m1 21 321 4321 54321 6.5e5", 'Precision not correctly reduced with "--set-precision=2" ' 'for path with ID ' + path.getAttribute('id')) - self.assertEqual(paths[4].getAttribute('d'), "m-1-12-123-1234-12345-1.2e5", + self.assertEqual(paths[4].getAttribute('d'), "m-1-21-321-4321-54321-6.5e5", 'Precision not correctly reduced with "--set-precision=2" ' 'for path with ID ' + paths[4].getAttribute('id')) self.assertEqual(paths[5].getAttribute('d'), "m123 101-123-101", @@ -990,10 +990,10 @@ class KeepPrecisionInPathDataIfSameLength(unittest.TestCase): doc = scourXmlFile('unittests/path-precision.svg', parse_args(['--set-precision=3'])) paths = doc.getElementsByTagNameNS(SVGNS, 'path') for path in paths[1:3]: - self.assertEqual(path.getAttribute('d'), "m1 12 123 1234 12345 123456", + self.assertEqual(path.getAttribute('d'), "m1 21 321 4321 54321 654321", 'Precision not correctly reduced with "--set-precision=3" ' 'for path with ID ' + path.getAttribute('id')) - self.assertEqual(paths[4].getAttribute('d'), "m-1-12-123-1234-12345-123456", + self.assertEqual(paths[4].getAttribute('d'), "m-1-21-321-4321-54321-654321", 'Precision not correctly reduced with "--set-precision=3" ' 'for path with ID ' + paths[4].getAttribute('id')) self.assertEqual(paths[5].getAttribute('d'), "m123 101-123-101", @@ -1003,10 +1003,10 @@ class KeepPrecisionInPathDataIfSameLength(unittest.TestCase): doc = scourXmlFile('unittests/path-precision.svg', parse_args(['--set-precision=4'])) paths = doc.getElementsByTagNameNS(SVGNS, 'path') for path in paths[1:3]: - self.assertEqual(path.getAttribute('d'), "m1 12 123 1234 12345 123456", + self.assertEqual(path.getAttribute('d'), "m1 21 321 4321 54321 654321", 'Precision not correctly reduced with "--set-precision=4" ' 'for path with ID ' + path.getAttribute('id')) - self.assertEqual(paths[4].getAttribute('d'), "m-1-12-123-1234-12345-123456", + self.assertEqual(paths[4].getAttribute('d'), "m-1-21-321-4321-54321-654321", 'Precision not correctly reduced with "--set-precision=4" ' 'for path with ID ' + paths[4].getAttribute('id')) self.assertEqual(paths[5].getAttribute('d'), "m123.5 101-123.5-101", @@ -1036,16 +1036,25 @@ class RemoveEmptyLineSegmentsFromPath(unittest.TestCase): self.assertEqual(path[4][0], 'z', 'Did not remove an empty line segment from path') -# Do not remove empty segments if round linecaps. - -class DoNotRemoveEmptySegmentsFromPathWithRoundLineCaps(unittest.TestCase): +class RemoveEmptySegmentsFromPathWithButtLineCaps(unittest.TestCase): def runTest(self): - doc = scourXmlFile('unittests/path-with-caps.svg') - path = svg_parser.parse(doc.getElementsByTagNameNS(SVGNS, 'path')[0].getAttribute('d')) - self.assertEqual(len(path), 2, - 'Did not preserve empty segments when path had round linecaps') + doc = scourXmlFile('unittests/path-with-caps.svg', parse_args(['--disable-style-to-xml'])) + for id in ['none', 'attr_butt', 'style_butt']: + path = svg_parser.parse(doc.getElementById(id).getAttribute('d')) + self.assertEqual(len(path), 1, + 'Did not remove empty segments when path had butt linecaps') + + +class DoNotRemoveEmptySegmentsFromPathWithRoundSquareLineCaps(unittest.TestCase): + + def runTest(self): + doc = scourXmlFile('unittests/path-with-caps.svg', parse_args(['--disable-style-to-xml'])) + for id in ['attr_round', 'attr_square', 'style_round', 'style_square']: + path = svg_parser.parse(doc.getElementById(id).getAttribute('d')) + self.assertEqual(len(path), 2, + 'Did remove empty segments when path had round or square linecaps') class ChangeLineToHorizontalLineSegmentInPath(unittest.TestCase): @@ -1215,35 +1224,51 @@ class RemoveFontStylesFromNonTextShapes(unittest.TestCase): 'font-size not removed from rect') -class CollapseConsecutiveHLinesSegments(unittest.TestCase): +class CollapseStraightPathSegments(unittest.TestCase): def runTest(self): - p = scourXmlFile('unittests/consecutive-hlines.svg').getElementsByTagNameNS(SVGNS, 'path')[0] - self.assertEqual(p.getAttribute('d'), 'm100 100h200v100h-200z', - 'Did not collapse consecutive hlines segments') + doc = scourXmlFile('unittests/collapse-straight-path-segments.svg', parse_args(['--disable-style-to-xml'])) + paths = doc.getElementsByTagNameNS(SVGNS, 'path') + path_data = [path.getAttribute('d') for path in paths] + path_data_expected = ['m0 0h30', + 'm0 0v30', + 'm0 0h10.5v10.5', + 'm0 0h10-1v10-1', + 'm0 0h30', + 'm0 0h30', + 'm0 0h10 20', + 'm0 0h10 20', + 'm0 0h10 20', + 'm0 0h10 20', + 'm0 0 20 40v1l10 20', + 'm0 0 10 10-20-20 10 10-20-20', + 'm0 0 1 2m1 2 2 4m1 2 2 4', + 'm6.3228 7.1547 81.198 45.258'] + self.assertEqual(path_data[0:3], path_data_expected[0:3], + 'Did not collapse h/v commands into a single h/v commands') + self.assertEqual(path_data[3], path_data_expected[3], + 'Collapsed h/v commands with different direction') + self.assertEqual(path_data[4:6], path_data_expected[4:6], + 'Did not collapse h/v commands with only start/end markers present') + self.assertEqual(path_data[6:10], path_data_expected[6:10], + 'Did not preserve h/v commands with intermediate markers present') -class CollapseConsecutiveHLinesCoords(unittest.TestCase): - - def runTest(self): - p = scourXmlFile('unittests/consecutive-hlines.svg').getElementsByTagNameNS(SVGNS, 'path')[1] - self.assertEqual(p.getAttribute('d'), 'm100 300h200v100h-200z', - 'Did not collapse consecutive hlines coordinates') - - -class DoNotCollapseConsecutiveHLinesSegsWithDifferingSigns(unittest.TestCase): - - def runTest(self): - p = scourXmlFile('unittests/consecutive-hlines.svg').getElementsByTagNameNS(SVGNS, 'path')[2] - self.assertEqual(p.getAttribute('d'), 'm100 500h300-100v100h-200z', - 'Collapsed consecutive hlines segments with differing signs') + self.assertEqual(path_data[10], path_data_expected[10], + 'Did not collapse lineto commands into a single (implicit) lineto command') + self.assertEqual(path_data[11], path_data_expected[11], + 'Collapsed lineto commands with different direction') + self.assertEqual(path_data[12], path_data_expected[12], + 'Collapsed first parameter pair of a moveto subpath') + self.assertEqual(path_data[13], path_data_expected[13], + 'Did not collapse the nodes of a straight real world path') class ConvertStraightCurvesToLines(unittest.TestCase): def runTest(self): p = scourXmlFile('unittests/straight-curve.svg').getElementsByTagNameNS(SVGNS, 'path')[0] - self.assertEqual(p.getAttribute('d'), 'm10 10l40 40 40-40z', + self.assertEqual(p.getAttribute('d'), 'm10 10 40 40 40-40z', 'Did not convert straight curves into lines') @@ -1380,7 +1405,7 @@ class CollapseSamePathPoints(unittest.TestCase): def runTest(self): p = scourXmlFile('unittests/collapse-same-path-points.svg').getElementsByTagNameNS(SVGNS, 'path')[0] - self.assertEqual(p.getAttribute('d'), "m100 100l100.12 100.12c14.877 4.8766-15.123-5.1234 0 0z", + self.assertEqual(p.getAttribute('d'), "m100 100 100.12 100.12c14.877 4.8766-15.123-5.1234 0 0z", 'Did not collapse same path points') @@ -1986,7 +2011,7 @@ class PathEmptyMove(unittest.TestCase): def runTest(self): doc = scourXmlFile('unittests/path-empty-move.svg') - self.assertEqual(doc.getElementsByTagName('path')[0].getAttribute('d'), 'm100 100l200 100z') + self.assertEqual(doc.getElementsByTagName('path')[0].getAttribute('d'), 'm100 100 200 100z') self.assertEqual(doc.getElementsByTagName('path')[1].getAttribute('d'), 'm100 100v200l100 100z') diff --git a/unittests/collapse-straight-path-segments.svg b/unittests/collapse-straight-path-segments.svg new file mode 100644 index 0000000..fa8e030 --- /dev/null +++ b/unittests/collapse-straight-path-segments.svg @@ -0,0 +1,33 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/unittests/consecutive-hlines.svg b/unittests/consecutive-hlines.svg deleted file mode 100644 index caae623..0000000 --- a/unittests/consecutive-hlines.svg +++ /dev/null @@ -1,6 +0,0 @@ - - - - - - diff --git a/unittests/path-precision.svg b/unittests/path-precision.svg index 9f2bc38..9222ed3 100644 --- a/unittests/path-precision.svg +++ b/unittests/path-precision.svg @@ -2,10 +2,10 @@ - - - - + + + + diff --git a/unittests/path-with-caps.svg b/unittests/path-with-caps.svg index 0e7ab1a..3c24163 100644 --- a/unittests/path-with-caps.svg +++ b/unittests/path-with-caps.svg @@ -1,4 +1,10 @@ - + + + + + + +