1
\$\begingroup\$

This question was moved from StackOverflow as suggested by some users.

I developed a function that aims to (should) provide a simple and effective method to transform a polygon <path> from relative to absolute. The purpose of this function is to replace similar SnapSVG function from my code, since I'm not processing any other type of path commands.

I need suggestions on how improve the code. Here's the thingy:

// simple pathToAbsolute for polygons
// returns an array with ABSOLUTE points of the polygon path
var pathToAbsolute = function(p) { 
  var np = p.match(/(m[^l]*|l[^(l|z)]*)/gi), // we extract the segments string from path string
      l = np.length, s, c, r, x = 0, y = 0;
  
  for (var i = 0; i<l; i++){ // here we start processing each segment string
    np[i] = np[i].trim(); // we trim the segment string
    c = np[i][0].trim(); r = new RegExp(c); // the path command
    np[i] = np[i].replace(/(^|[^,])\s*-/g, '$1,-') // fix the x,y 186.101-87.287 to -186.101,-87.287 separation
                 .replace(/[\s+\,|\s|\,]/g,'|') // replace any spacing and comma with another separator
                 .replace(r,'') // remove the path command from string
                 .split('|'); // split by new separator

    np[i][0] === '' && np[i].shift(); // I need a way to avoid this

    np[i][0] = parseFloat(np[i][0]); // make sure we do math with numbers
    np[i][1] = parseFloat(np[i][1]);

    if (i === 0) { x+=np[i][0]; y +=np[i][1]; } // for M, we only need it's values

    if (i>0){ // for other segments
      x= !!np[i-1] ? np[i-1][0] : np[0][0]; // if we have a previous point
      y= !!np[i-1] ? np[i-1][1] : np[0][1]; // let's grab its x,y values for relative points

      np[i][0] = c === 'l' ? np[i][0] + x : np[i][0]; // here we fix the value of any 
      np[i][1] = c === 'l' ? np[i][1] + y : np[i][1]; // relative point
    }
  }
  return np; // in the end we return the newPoints array
}

// testing the function
var star = document.getElementById('star'), svg = star.parentNode,
    starPoints = pathToAbsolute(star.getAttribute('d'));

var newStar = document.createElementNS('http://www.w3.org/2000/svg','path');
newStar.setAttribute('d', 'M'+starPoints.join('L')+'z');
newStar.setAttribute('fill','green');
newStar.setAttribute('transform','translate(30,30)');
svg.appendChild(newStar);
<div style="width:200px">
  <svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 800 800" >

   <path id="star" 
         fill="#eee" 
         d="M301.113,12.011l99.25,179.996l201.864,38.778L461.706,380.808 l25.508,203.958l-186.101-87.287L115.01,584.766l25.507-203.958L0,230.785l201.86-38.778L301.113,12.011z"
    ></path>
                    
  </svg>
</div>

As you can see, the function works pretty much as expected, but in the comments I've put some points of concerns where I'm not sure it's accurate or generally applicable.

My most concern is the way to calculate the new values for relative points now becoming absolute. Also I need a way to avoid shifting via np[0].shift() any segment that is not properly processed before. Perhaps I can improve some RegExp, just I'm not completely sure.

So I am open to any other improvement suggestion you may have, so please jump in.

I need something that should work IE9+, Chrome 4+, Safari 5+, regardless of the fact that pathSegList or pathDataAPI is supported or not, just like the original functions from SnapSVG. No polyfills are acceptable.

Your suggested improved function must return same thingy: the array with absolute points, probably supporting H and V path commands. The array format is required for other functions that process the array for other uses.

\$\endgroup\$
  • \$\begingroup\$ To be clear, you only need support for move-to and line-to path commands (i.e. m and l)? Because there's also h and v line commands, the a arc-to command, and of course the c, s, q and t curve commands... \$\endgroup\$ – Flambino Mar 22 '16 at 15:16
  • \$\begingroup\$ Thanks, only for MLZ based paths. If I can mix H and V would be probably great. \$\endgroup\$ – thednp Mar 22 '16 at 15:20
2
\$\begingroup\$

Your variable names are overly terse. It's had to follow the logic when it's all to do with a two-letter variable that also happens to be a 2D array.

Basically, you seem intent writing it as densely as possible rather than making it readable. But minifying javascript is what minification tools are for.

It's also strange that you pass in a complete path string, but you get an array back. An array that's missing the commands, and where you manually have to prefix M and postfix z. Why not output a complete path?

Your test just assumes that all the commands (apart from the initial M) was a L command. Which, since that the only command your code can interpret, isn't too terrible. It'd still be neater to rewrite the path code itself, though, and return a string.

Adding the z smells especially buggy, as it closes the path even if the original path wasn't closed.

As for overall structure/approach, I'd use replace with a callback and a global pattern, rather than create a match object, and much around with its contents.

In other words, I'd do this:

function pathToAbsolute(relativePath) {
  var pattern = /([ml])\s*(-?\d*\.?\d+)\s*,\s*(-?\d*\.?\d+)/ig,
      offset = null;
      
  // Scan the string for an M/L command (case-insensitive) and its two
  // coordinates, replacing any lower-case (relative) command with an
  // absolute version
  return relativePath.replace(pattern, function (match, command, x, y) {
    x = parseFloat(x);
    y = parseFloat(y);
    
    // If we don't have an offset yet or the command is absolute already,
    // then use the matched coordinates as the offset, and return the matched
    // string unaltered
    if(offset === null || command.toUpperCase() === command) {
      offset = { x: x, y: y };
      return match;
    }
    
    // If we're here, the command is relative, so add its coordinates to the
    // offset, and output the absolute version of the matched string
    offset.x += x;
    offset.y += y;
    
    return command.toUpperCase() + offset.x + "," + offset.y;
  });
}

// ========================
// Test code

var star = document.getElementById('star'),
    svg = star.parentNode,
    absPath = pathToAbsolute(star.getAttribute('d'));

var newStar = document.createElementNS('http://www.w3.org/2000/svg','path');
newStar.setAttribute('d', absPath);
newStar.setAttribute('fill','green');
newStar.setAttribute('transform','translate(30,30)');
svg.appendChild(newStar);
<svg width="200" height="200" xmlns="http://www.w3.org/2000/svg" viewBox="0 0 800 800">
   <path id="star" fill="#eee" d="M301.113,12.011l99.25,179.996l201.864,38.778L461.706,380.808 l25.508,203.958l-186.101-87.287L115.01,584.766l25.507-203.958L0,230.785l201.86-38.778L301.113,12.011z"></path>
</svg>

Like yours it only looks for M and L commands - not any of the many other SVG commands. Though it shouldn't actually be too terrible to extend it to handle those as well. Especially, as this returns a complete path string that can be used right away. Not joining necessary, and no assuming all commands are Ls.

It does however still assume that the path is valid; that it starts with an m or M. So it's gigo: Garbage in, garbage out.


Edit: Although I do feel the code above answers the question as originally stated, here's a version that doesn't "transform a path", but parses it and returns an array.

function pathToAbsolute(relativePath) {
  var pattern = /([ml])\s*(-?\d*\.?\d+)\s*,\s*(-?\d*\.?\d+)/ig,
      coords = [];

  relativePath.replace(pattern, function (match, command, x, y) {
    var prev;

    x = parseFloat(x);
    y = parseFloat(y);

    if(coords.length === 0 || command.toUpperCase() === command) {
      coords.push([x, y]);
    } else {
      prev = coords[coords.length-1];
      coords.push([x + prev[0], y + prev[1]]);
    }
  });

  return coords;
}
\$\endgroup\$
  • \$\begingroup\$ Thanks for the time you've spent. Nevertheless I need an array, no matter what, and if you happen to have a super quick idea to make it work with H and V, I am happy above normal. \$\endgroup\$ – thednp Mar 22 '16 at 16:14
  • \$\begingroup\$ I like your code @Flambino just I need the function to return an array with the points, perhaps even with some H or V into L points would be a great addition, I don't know how to change it to my need, so come on, let me reward you with a marked question. \$\endgroup\$ – thednp Mar 22 '16 at 16:55
  • \$\begingroup\$ @thednp I think we're at an impasse here. I did review the code, and provided a solution for the problem as originally stated: "transform a polygon <path> from relative to absolute". The bit about returning an array was not mentioned (you edited your question after I'd submitted my answer, which is a no-no). I did alter the return type of the function, so I'm not 100% right here either. Still, I acted in good faith. I'll add a version that returns an array, although the question still isn't about that. I won't add support for H/L, though; that's for you to handle \$\endgroup\$ – Flambino Mar 22 '16 at 22:55
  • \$\begingroup\$ It's true I forgot to mention that I need an array, I'm sorry. I'll keep that in mind for next questions, this was my first question here. But thanks, you've done great helping with other ideas :) \$\endgroup\$ – thednp Mar 22 '16 at 22:59
  • \$\begingroup\$ @thednp Added another function that returns an array. And I found a dumb bug in my regex (unescaped .) that also affected the first code block - fixed now \$\endgroup\$ – Flambino Mar 22 '16 at 23:10

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.