fix(convertPathData): introducing isSafeToRemove (#2164)

Removing useless commands: a tricky and inconsistent business. SVGO
currently uses two approaches:
- **!maybeHasStrokeAndLinecap** for the purpose of removing useless line
commands

This works decently. It allows for removing useless lines, while not
removing dots.

Flaw: Even in a path with linecaps, if there are "dot" commands after
the first move, they wouldn't render at all and should be removed. They
currently aren't.
- **isSafeToUseZ** for the purpose of removing useless `z` commands

isSafeToUseZ is defined as either having no stroke or having a all-round
stroke. When that's true, a line home and a z command render the same
way, and can be exchanged or removed.

Flaw: This works great when removing a redundant `z` that comes right
after another command, but what if the `z` is the first move (for the
purpose of creating a dot)? Then it's incorrectly removed. This is shown
in #2158 and #2163.

This PR introduces isSafeToRemove to help. It's worth reading through
the code, but the TLDR is that it uses a more sane method, categorizing
the path into non-stroked, stroked + first draw command, and stroked but
non-first command, and using fitting behavior in each case.

Fixes #2158 and fixes #2163.

Author: KTibow

plugins/convertPathData.js

@@ -155,13 +155,25 @@ export const fn = (root, params) => {
             computedStyle['stroke-linecap'] &&
             (computedStyle['stroke-linecap'].type === 'dynamic' ||
               computedStyle['stroke-linecap'].value !== 'butt');
-          const maybeHasStrokeAndLinecap = maybeHasStroke && maybeHasLinecap;
           const isSafeToUseZ = maybeHasStroke
             ? computedStyle['stroke-linecap']?.type === 'static' &&
               computedStyle['stroke-linecap'].value === 'round' &&
               computedStyle['stroke-linejoin']?.type === 'static' &&
               computedStyle['stroke-linejoin'].value === 'round'
             : true;
+          const isSafeToRemove = (
+            /** @type {boolean} */ isFirstDraw,
+            /** @type {boolean} */ safeIfNotFirstDraw,
+          ) => {
+            if (!maybeHasStroke) {
+              return true;
+            }
+            if (isFirstDraw) {
+              return !maybeHasLinecap;
+            } else {
+              return safeIfNotFirstDraw;
+            }
+          };
 
           let data = path2js(node);
 
@@ -174,7 +186,7 @@ export const fn = (root, params) => {
 
             data = filters(data, newParams, {
               isSafeToUseZ,
-              maybeHasStrokeAndLinecap,
+              isSafeToRemove,
               hasMarkerMid,
             });
 
@@ -378,14 +390,10 @@ const convertToRelative = (pathData) => {
  *
  * @param {import('../lib/types.js').PathDataItem[]} path
  * @param {InternalParams} params
- * @param {{ isSafeToUseZ: boolean, maybeHasStrokeAndLinecap: boolean, hasMarkerMid: boolean }} param2
+ * @param {{ isSafeToUseZ: boolean, isSafeToRemove: (isFirstDraw: boolean, safeIfNotFirstDraw: boolean) => boolean, hasMarkerMid: boolean }} param2
  * @returns {import('../lib/types.js').PathDataItem[]}
  */
-function filters(
-  path,
-  params,
-  { isSafeToUseZ, maybeHasStrokeAndLinecap, hasMarkerMid },
-) {
+function filters(path, params, { isSafeToUseZ, isSafeToRemove, hasMarkerMid }) {
   const stringify = data2Path.bind(null, params);
   const relSubpoint = [0, 0];
   const pathBase = [0, 0];
@@ -835,7 +843,10 @@ function filters(
       }
 
       // remove useless non-first path segments
-      if (params.removeUseless && !maybeHasStrokeAndLinecap) {
+      if (
+        params.removeUseless &&
+        isSafeToRemove(prev.command == 'm' || prev.command == 'M', true)
+      ) {
         // l 0,0 / h 0 / v 0 / q 0,0 0,0 / t 0,0 / c 0,0 0,0 0,0 / s 0,0 0,0
         if (
           (command === 'l' ||
@@ -891,7 +902,10 @@ function filters(
     if (
       (command === 'Z' || command === 'z') &&
       params.removeUseless &&
-      isSafeToUseZ &&
+      isSafeToRemove(
+        prev.command == 'm' || prev.command == 'M',
+        isSafeToUseZ,
+      ) &&
       // @ts-expect-error
       Math.abs(item.base[0] - item.coords[0]) < error / 10 &&
       // @ts-expect-error

test/plugins/convertPathData.38.svg.txt

@@ -0,0 +1,15 @@
+Should not break dots.
+
+See: https://github.com/svg/svgo/issues/2163
+
+===
+
+<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 20 20">
+    <path d="M10 10z" stroke="#00f" stroke-linecap="round" stroke-linejoin="round" />
+</svg>
+
+@@@
+
+<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 20 20">
+    <path d="M10 10z" stroke="#00f" stroke-linecap="round" stroke-linejoin="round"/>
+</svg>

test/plugins/convertPathData.39.svg.txt

@@ -0,0 +1,13 @@
+Should drop truely useless commands even when a linecap is present.
+
+===
+
+<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 20 20">
+    <path d="M 10 50 h 10 v 0" stroke="#00f" stroke-linecap="round" stroke-linejoin="round" />
+</svg>
+
+@@@
+
+<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 20 20">
+    <path d="M10 50h10" stroke="#00f" stroke-linecap="round" stroke-linejoin="round"/>
+</svg>

test/regression/lists/expect-mismatch.txt

@@ -12,7 +12,6 @@ W3C_SVG_11_TestSuite/svg/styling-css-04-f.svg
 
 # broken and pending fix
 charm-icons-0.18.0/docs/img/splash.svg
-charm-icons-0.18.0/icons/atom.svg
 oxygen-icons-5.116.0/scalable/actions/hidef/tools-rip-audio-cd.svg
 oxygen-icons-5.116.0/scalable/actions/im-ban-kick-user.svg
 oxygen-icons-5.116.0/scalable/actions/im-ban-user.svg