Refactor Strokes (#112)

* Wip

* refactor strokes

* fix

* fix

* fix

* refactor

* more refactor

* add changeset and fix issue fix line node

* refactor

* continue the refactor

* refactor fills

* fix

* wip

* try another approach

* refactor

* refactor

* more refactor

* refactor

* wip

* wip

* minor improvements

* minor fixes

* refactor

---------

Co-authored-by: Jordi Sala Morales <jordism91@gmail.com>

.changeset/polite-pears-repair.md

@@ -0,0 +1,5 @@
+---
+"penpot-exporter": patch
+---
+
+Arrows on complex svgs are now better represented inside Penpot

.changeset/popular-rats-cheer.md

@@ -0,0 +1,5 @@
+---
+"penpot-exporter": patch
+---
+
+Fix line node svg path

plugin-src/transformers/partials/transformStrokes.ts

@@ -1,17 +1,14 @@
-import { translateStrokes } from '@plugin/translators';
+import { Command } from 'svg-path-parser';
+
+import { translateStrokeCap, translateStrokes } from '@plugin/translators';
 
 import { ShapeAttributes } from '@ui/lib/types/shapes/shape';
+import { Stroke } from '@ui/lib/types/utils/stroke';
 
 const isVectorLike = (node: GeometryMixin | VectorLikeMixin): node is VectorLikeMixin => {
   return 'vectorNetwork' in node;
 };
 
-const isIndividualStrokes = (
-  node: GeometryMixin | IndividualStrokesMixin
-): node is IndividualStrokesMixin => {
-  return 'strokeTopWeight' in node;
-};
-
 const hasFillGeometry = (node: GeometryMixin): boolean => {
   return node.fillGeometry.length > 0;
 };
@@ -19,12 +16,54 @@ const hasFillGeometry = (node: GeometryMixin): boolean => {
 export const transformStrokes = async (
   node: GeometryMixin | (GeometryMixin & IndividualStrokesMixin)
 ): Promise<Partial<ShapeAttributes>> => {
+  const vectorNetwork = isVectorLike(node) ? node.vectorNetwork : undefined;
+
+  const strokeCaps = (stroke: Stroke) => {
+    if (!hasFillGeometry(node) && vectorNetwork && vectorNetwork.vertices.length > 0) {
+      stroke.strokeCapStart = translateStrokeCap(vectorNetwork.vertices[0]);
+      stroke.strokeCapEnd = translateStrokeCap(
+        vectorNetwork.vertices[vectorNetwork.vertices.length - 1]
+      );
+    }
+
+    return stroke;
+  };
+
   return {
-    strokes: await translateStrokes(
-      node,
-      hasFillGeometry(node),
-      isVectorLike(node) ? node.vectorNetwork : undefined,
-      isIndividualStrokes(node) ? node : undefined
-    )
+    strokes: await translateStrokes(node, strokeCaps)
   };
 };
+
+export const transformStrokesFromVector = async (
+  node: VectorNode,
+  vector: Command[],
+  vectorRegion: VectorRegion | undefined
+): Promise<Partial<ShapeAttributes>> => {
+  const strokeCaps = (stroke: Stroke) => {
+    if (vectorRegion !== undefined) return stroke;
+
+    const startVertex = findVertex(node.vectorNetwork.vertices, vector[0]);
+    const endVertex = findVertex(node.vectorNetwork.vertices, vector[vector.length - 1]);
+
+    if (!startVertex || !endVertex) return stroke;
+
+    stroke.strokeCapStart = translateStrokeCap(startVertex);
+    stroke.strokeCapEnd = translateStrokeCap(endVertex);
+
+    return stroke;
+  };
+
+  return {
+    strokes: await translateStrokes(node, strokeCaps)
+  };
+};
+
+const findVertex = (
+  vertexs: readonly VectorVertex[],
+  command: Command
+): VectorVertex | undefined => {
+  if (command.command !== 'moveto' && command.command !== 'lineto' && command.command !== 'curveto')
+    return;
+
+  return vertexs.find(vertex => vertex.x === command.x && vertex.y === command.y);
+};

plugin-src/transformers/partials/transformVectorPaths.ts

@@ -1,32 +1,26 @@
+import { parseSVG } from 'svg-path-parser';
+
 import {
   transformBlend,
   transformDimensionAndPositionFromVectorPath,
   transformEffects,
   transformProportion,
   transformSceneNode,
-  transformStrokes
+  transformStrokesFromVector
 } from '@plugin/transformers/partials';
-import { createLineGeometry, translateVectorPath, translateVectorPaths } from '@plugin/translators';
 import { translateFills } from '@plugin/translators/fills';
+import {
+  translateCommandsToSegments,
+  translateLineNode,
+  translateVectorPaths,
+  translateWindingRule
+} from '@plugin/translators/vectors';
 
 import { PathAttributes } from '@ui/lib/types/shapes/pathShape';
 import { PathShape } from '@ui/lib/types/shapes/pathShape';
-import { Children } from '@ui/lib/types/utils/children';
-
-const getVectorPaths = (node: VectorNode | StarNode | LineNode | PolygonNode): VectorPaths => {
-  switch (node.type) {
-    case 'STAR':
-    case 'POLYGON':
-      return node.fillGeometry;
-    case 'VECTOR':
-      return node.vectorPaths;
-    case 'LINE':
-      return createLineGeometry(node);
-  }
-};
 
 export const transformVectorPathsAsContent = (
-  node: VectorNode | StarNode | LineNode | PolygonNode,
+  node: StarNode | LineNode | PolygonNode,
   baseX: number,
   baseY: number
 ): PathAttributes => {
@@ -37,14 +31,20 @@ export const transformVectorPathsAsContent = (
   };
 };
 
-export const transformVectorPathsAsChildren = async (
+export const transformVectorPaths = async (
   node: VectorNode,
   baseX: number,
   baseY: number
-): Promise<Children> => {
-  return {
-    children: await Promise.all(
-      node.vectorPaths.map((vectorPath, index) =>
+): Promise<PathShape[]> => {
+  const pathShapes = await Promise.all(
+    node.vectorPaths
+      .filter((vectorPath, index) => {
+        return (
+          nodeHasFills(node, vectorPath, (node.vectorNetwork.regions ?? [])[index]) ||
+          node.strokes.length > 0
+        );
+      })
+      .map((vectorPath, index) =>
         transformVectorPath(
           node,
           vectorPath,
@@ -53,8 +53,47 @@ export const transformVectorPathsAsChildren = async (
           baseY
         )
       )
-    )
-  };
+  );
+
+  const geometryShapes = await Promise.all(
+    node.fillGeometry
+      .filter(
+        geometry =>
+          !node.vectorPaths.find(
+            vectorPath => normalizePath(vectorPath.data) === normalizePath(geometry.data)
+          )
+      )
+      .map(geometry => transformVectorPath(node, geometry, undefined, baseX, baseY))
+  );
+
+  return [...geometryShapes, ...pathShapes];
+};
+
+const getVectorPaths = (node: StarNode | LineNode | PolygonNode): VectorPaths => {
+  switch (node.type) {
+    case 'STAR':
+    case 'POLYGON':
+      return node.fillGeometry;
+    case 'LINE':
+      return translateLineNode(node);
+  }
+};
+
+const normalizePath = (path: string): string => {
+  // Round to 2 decimal places all numbers
+  const str = path.replace(/(\d+\.\d+|\d+)/g, (match: string) => {
+    return parseFloat(match).toFixed(2);
+  });
+  // remove spaces
+  return str.replace(/\s/g, '');
+};
+
+const nodeHasFills = (
+  node: VectorNode,
+  vectorPath: VectorPath,
+  vectorRegion: VectorRegion | undefined
+): boolean => {
+  return !!(vectorPath.windingRule !== 'NONE' && (vectorRegion?.fills || node.fills));
 };
 
 const transformVectorPath = async (
@@ -64,12 +103,20 @@ const transformVectorPath = async (
   baseX: number,
   baseY: number
 ): Promise<PathShape> => {
+  const normalizedPaths = parseSVG(vectorPath.data);
+
   return {
     type: 'path',
     name: 'svg-path',
-    content: translateVectorPath(vectorPath, baseX + node.x, baseY + node.y),
-    fills: await translateFills(vectorRegion?.fills ?? node.fills),
-    ...(await transformStrokes(node)),
+    content: translateCommandsToSegments(normalizedPaths, baseX + node.x, baseY + node.y),
+    fills:
+      vectorPath.windingRule === 'NONE'
+        ? []
+        : await translateFills(vectorRegion?.fills ?? node.fills),
+    svgAttrs: {
+      fillRule: translateWindingRule(vectorPath.windingRule)
+    },
+    ...(await transformStrokesFromVector(node, normalizedPaths, vectorRegion)),
     ...transformEffects(node),
     ...transformDimensionAndPositionFromVectorPath(vectorPath, baseX, baseY),
     ...transformSceneNode(node),

plugin-src/transformers/transformGroupNode.ts

@@ -15,12 +15,14 @@ export const transformGroupNode = async (
 ): Promise<GroupShape> => {
   return {
     ...transformGroupNodeLike(node, baseX, baseY),
+    ...transformEffects(node),
+    ...transformBlend(node),
     ...(await transformChildren(node, baseX, baseY))
   };
 };
 
 export const transformGroupNodeLike = (
-  node: BaseNodeMixin & DimensionAndPositionMixin & BlendMixin & SceneNodeMixin & MinimalBlendMixin,
+  node: BaseNodeMixin & DimensionAndPositionMixin & SceneNodeMixin,
   baseX: number,
   baseY: number
 ): GroupShape => {
@@ -28,8 +30,6 @@ export const transformGroupNodeLike = (
     type: 'group',
     name: node.name,
     ...transformDimensionAndPosition(node, baseX, baseY),
-    ...transformEffects(node),
-    ...transformSceneNode(node),
-    ...transformBlend(node)
+    ...transformSceneNode(node)
   };
 };

plugin-src/transformers/transformPathNode.ts

@@ -11,12 +11,12 @@ import {
 
 import { PathShape } from '@ui/lib/types/shapes/pathShape';
 
-const hasFillGeometry = (node: VectorNode | StarNode | LineNode | PolygonNode): boolean => {
+const hasFillGeometry = (node: StarNode | LineNode | PolygonNode): boolean => {
   return 'fillGeometry' in node && node.fillGeometry.length > 0;
 };
 
 export const transformPathNode = async (
-  node: VectorNode | StarNode | LineNode | PolygonNode,
+  node: StarNode | LineNode | PolygonNode,
   baseX: number,
   baseY: number
 ): Promise<PathShape> => {

plugin-src/transformers/transformVectorNode.ts

@@ -1,9 +1,9 @@
-import { transformVectorPathsAsChildren } from '@plugin/transformers/partials';
+import { transformVectorPaths } from '@plugin/transformers/partials';
 
 import { GroupShape } from '@ui/lib/types/shapes/groupShape';
 import { PathShape } from '@ui/lib/types/shapes/pathShape';
 
-import { transformGroupNodeLike, transformPathNode } from '.';
+import { transformGroupNodeLike } from '.';
 
 /*
  * Vector nodes can have multiple vector paths, each with its own fills.
@@ -16,12 +16,17 @@ export const transformVectorNode = async (
   baseX: number,
   baseY: number
 ): Promise<GroupShape | PathShape> => {
-  if ((node.vectorNetwork.regions ?? []).length === 0) {
-    return transformPathNode(node, baseX, baseY);
+  const children = await transformVectorPaths(node, baseX, baseY);
+
+  if (children.length === 1) {
+    return {
+      ...children[0],
+      name: node.name
+    };
   }
 
   return {
     ...transformGroupNodeLike(node, baseX, baseY),
-    ...(await transformVectorPathsAsChildren(node, baseX, baseY))
+    children
   };
 };

plugin-src/translators/fills/translateFills.ts

@@ -23,12 +23,13 @@ export const translateFill = async (fill: Paint): Promise<Fill | undefined> => {
 };
 
 export const translateFills = async (
-  fills: readonly Paint[] | typeof figma.mixed
+  fills: readonly Paint[] | typeof figma.mixed | undefined
 ): Promise<Fill[]> => {
-  const figmaFills = fills === figma.mixed ? [] : fills;
+  if (fills === undefined || fills === figma.mixed) return [];
+
   const penpotFills: Fill[] = [];
 
-  for (const fill of figmaFills) {
+  for (const fill of fills) {
     const penpotFill = await translateFill(fill);
     if (penpotFill) {
       // fills are applied in reverse order in Figma, that's why we unshift

plugin-src/translators/index.ts

@@ -2,4 +2,3 @@ export * from './translateBlendMode';
 export * from './translateBlurEffects';
 export * from './translateShadowEffects';
 export * from './translateStrokes';
-export * from './translateVectorPaths';

plugin-src/translators/translateStrokes.ts

@@ -3,69 +3,46 @@ import { translateFill } from '@plugin/translators/fills';
 import { Stroke, StrokeAlignment, StrokeCaps } from '@ui/lib/types/utils/stroke';
 
 export const translateStrokes = async (
-  nodeStrokes: MinimalStrokesMixin,
-  hasFillGeometry?: boolean,
-  vectorNetwork?: VectorNetwork,
-  individualStrokes?: IndividualStrokesMixin
+  node: MinimalStrokesMixin | (MinimalStrokesMixin & IndividualStrokesMixin),
+  strokeCaps: (stroke: Stroke) => Stroke = stroke => stroke
 ): Promise<Stroke[]> => {
+  const sharedStrokeProperties: Partial<Stroke> = {
+    strokeWidth: translateStrokeWeight(node),
+    strokeAlignment: translateStrokeAlignment(node.strokeAlign),
+    strokeStyle: node.dashPattern.length ? 'dashed' : 'solid'
+  };
+
   return await Promise.all(
-    nodeStrokes.strokes.map(async (paint, index) => {
-      const fill = await translateFill(paint);
-      const stroke: Stroke = {
-        strokeColor: fill?.fillColor,
-        strokeOpacity: fill?.fillOpacity,
-        strokeWidth: translateStrokeWeight(nodeStrokes.strokeWeight, individualStrokes),
-        strokeAlignment: translateStrokeAlignment(nodeStrokes.strokeAlign),
-        strokeStyle: nodeStrokes.dashPattern.length ? 'dashed' : 'solid',
-        strokeImage: fill?.fillImage
-      };
-
-      if (!hasFillGeometry && index === 0 && vectorNetwork && vectorNetwork.vertices.length > 0) {
-        stroke.strokeCapStart = translateStrokeCap(vectorNetwork.vertices[0]);
-        stroke.strokeCapEnd = translateStrokeCap(
-          vectorNetwork.vertices[vectorNetwork.vertices.length - 1]
-        );
-      }
-
-      return stroke;
-    })
+    node.strokes.map(
+      async (paint, index) =>
+        await translateStroke(paint, sharedStrokeProperties, strokeCaps, index === 0)
+    )
   );
 };
 
-const translateStrokeWeight = (
-  strokeWeight: number | typeof figma.mixed,
-  individualStrokes?: IndividualStrokesMixin
-): number => {
-  if (strokeWeight !== figma.mixed) {
-    return strokeWeight;
+export const translateStroke = async (
+  paint: Paint,
+  sharedStrokeProperties: Partial<Stroke>,
+  strokeCaps: (stroke: Stroke) => Stroke,
+  firstStroke: boolean
+): Promise<Stroke> => {
+  const fill = await translateFill(paint);
+
+  let stroke: Stroke = {
+    strokeColor: fill?.fillColor,
+    strokeOpacity: fill?.fillOpacity,
+    strokeImage: fill?.fillImage,
+    ...sharedStrokeProperties
+  };
+
+  if (firstStroke) {
+    stroke = strokeCaps(stroke);
   }
 
-  if (!individualStrokes) {
-    return 1;
-  }
-
-  return Math.max(
-    individualStrokes.strokeTopWeight,
-    individualStrokes.strokeRightWeight,
-    individualStrokes.strokeBottomWeight,
-    individualStrokes.strokeLeftWeight
-  );
+  return stroke;
 };
 
-const translateStrokeAlignment = (
-  strokeAlign: 'CENTER' | 'INSIDE' | 'OUTSIDE'
-): StrokeAlignment => {
-  switch (strokeAlign) {
-    case 'CENTER':
-      return 'center';
-    case 'INSIDE':
-      return 'inner';
-    case 'OUTSIDE':
-      return 'outer';
-  }
-};
-
-const translateStrokeCap = (vertex: VectorVertex): StrokeCaps | undefined => {
+export const translateStrokeCap = (vertex: VectorVertex): StrokeCaps | undefined => {
   switch (vertex.strokeCap as StrokeCap | ConnectorStrokeCap) {
     case 'ROUND':
       return 'round';
@@ -85,3 +62,41 @@ const translateStrokeCap = (vertex: VectorVertex): StrokeCaps | undefined => {
       return;
   }
 };
+
+const translateStrokeWeight = (
+  node: MinimalStrokesMixin | (MinimalStrokesMixin & IndividualStrokesMixin)
+): number => {
+  if (node.strokeWeight !== figma.mixed) {
+    return node.strokeWeight;
+  }
+
+  if (!isIndividualStrokes(node)) {
+    return 1;
+  }
+
+  return Math.max(
+    node.strokeTopWeight,
+    node.strokeRightWeight,
+    node.strokeBottomWeight,
+    node.strokeLeftWeight
+  );
+};
+
+const isIndividualStrokes = (
+  node: MinimalStrokesMixin | IndividualStrokesMixin
+): node is IndividualStrokesMixin => {
+  return 'strokeTopWeight' in node;
+};
+
+const translateStrokeAlignment = (
+  strokeAlign: 'CENTER' | 'INSIDE' | 'OUTSIDE'
+): StrokeAlignment => {
+  switch (strokeAlign) {
+    case 'CENTER':
+      return 'center';
+    case 'INSIDE':
+      return 'inner';
+    case 'OUTSIDE':
+      return 'outer';
+  }
+};

plugin-src/translators/vectors/index.ts

@@ -0,0 +1,3 @@
+export * from './translateLineNode';
+export * from './translateVectorPaths';
+export * from './translateWindingRule';

plugin-src/translators/vectors/translateLineNode.ts

@@ -0,0 +1,23 @@
+export const translateLineNode = (node: LineNode): VectorPaths => {
+  const commands = [
+    {
+      command: 'moveto',
+      code: 'M',
+      x: 0,
+      y: 0
+    },
+    {
+      command: 'lineto',
+      code: 'L',
+      x: node.width,
+      y: node.height
+    }
+  ];
+
+  return [
+    {
+      windingRule: 'NONE',
+      data: commands.reduce((acc, { code, x, y }) => acc + `${code} ${x} ${y}`, '')
+    }
+  ];
+};

plugin-src/translators/vectors/translateVectorPaths.ts

@@ -1,4 +1,4 @@
-import { CurveToCommand, LineToCommand, MoveToCommand, parseSVG } from 'svg-path-parser';
+import { Command, CurveToCommand, LineToCommand, MoveToCommand, parseSVG } from 'svg-path-parser';
 
 import { Segment } from '@ui/lib/types/shapes/pathShape';
 
@@ -10,16 +10,20 @@ export const translateVectorPaths = (
   let segments: Segment[] = [];
 
   for (const path of paths) {
-    segments = [...segments, ...translateVectorPath(path, baseX, baseY)];
+    const normalizedPaths = parseSVG(path.data);
+
+    segments = [...segments, ...translateCommandsToSegments(normalizedPaths, baseX, baseY)];
   }
 
   return segments;
 };
 
-export const translateVectorPath = (path: VectorPath, baseX: number, baseY: number): Segment[] => {
-  const normalizedPaths = parseSVG(path.data);
-
-  return normalizedPaths.map(command => {
+export const translateCommandsToSegments = (
+  commands: Command[],
+  baseX: number,
+  baseY: number
+): Segment[] => {
+  return commands.map(command => {
     switch (command.command) {
       case 'moveto':
         return translateMoveToCommand(command, baseX, baseY);
@@ -36,30 +40,6 @@ export const translateVectorPath = (path: VectorPath, baseX: number, baseY: numb
   });
 };
 
-export const createLineGeometry = (node: LineNode): VectorPaths => {
-  const commands = [
-    {
-      command: 'moveto',
-      code: 'M',
-      x: 0,
-      y: 0
-    },
-    {
-      command: 'lineto',
-      code: 'L',
-      x: node.width,
-      y: node.height
-    }
-  ];
-
-  return [
-    {
-      windingRule: 'NONZERO',
-      data: commands.map(({ code, x, y }) => `${code} ${x} ${y}`).join(' ') + ' Z'
-    }
-  ];
-};
-
 const translateMoveToCommand = (command: MoveToCommand, baseX: number, baseY: number): Segment => {
   return {
     command: 'move-to',

plugin-src/translators/vectors/translateWindingRule.ts

@@ -0,0 +1,10 @@
+import { FillRules } from '@ui/lib/types/shapes/pathShape';
+
+export const translateWindingRule = (windingRule: WindingRule | 'NONE'): FillRules | undefined => {
+  switch (windingRule) {
+    case 'EVENODD':
+      return 'evenodd';
+    case 'NONZERO':
+      return 'nonzero';
+  }
+};

ui-src/lib/types/shapes/pathShape.ts

@@ -9,6 +9,9 @@ export type PathShape = ShapeBaseAttributes &
 export type PathAttributes = {
   type?: 'path';
   content: PathContent;
+  svgAttrs?: {
+    fillRule?: FillRules;
+  };
 };
 
 export const VECTOR_LINE_TO: unique symbol = Symbol.for('line-to');
@@ -28,6 +31,8 @@ export type Command =
   | typeof VECTOR_MOVE_TO
   | typeof VECTOR_CURVE_TO;
 
+export type FillRules = 'evenodd' | 'nonzero';
+
 type LineTo = {
   command: 'line-to' | typeof VECTOR_LINE_TO;
   params: {