6
\$\begingroup\$

The problem I am solving:

  • I select multiple 2D SVG elements in an editor(the elements are in arbitrary positions on my canvas).
  • I run the function calcMultiTrack() (see below).
  • The elements should now have equal spacing between them equivalent to the trackingAmount parameter I passed when executing calcMultiTrack(). Spacing means only width-wise.

A live demonstration of what I mean(this is a demo of the function I built, at work):

enter image description here

Some additional notes:

  • calcMultiTrack() uses a global variable, multiselectedCopy. This is an array that holds all the elements I have currently multi-selected.
  • The algo must handle negative spacing (negative trackingAmount)
  • The most-left item in a multiselection does not move. The tracking is calculated by figuring out how much spacing each element has from the previous one(starting from most-left) and then moving each element until the trackingAmount has been reached.

There are 2 utility functions which are not defined in the code below.

  • svgCanvas.moveSingleElement (moves an element by a certain x,y value which as passed as parameters)
  • svgCanvas.getStrokedBBox (returns only x,y,width,height values for an element)

What I want to solve.

  1. Well, is this the best way? Are there any bottlenecks?

    The user drags a slider to adjust tracking. Because the calculations take some time as they are, I debounce the slider until the user stops sliding. Otherwise the browser hangs/is choppy. If this has many bottlenecks that could be eliminated, I could have the user dragging the slider and seeing the results in real-time. This will make it far better for the user since immediate feedback is much better for such type of fine-adjustments.

  2. Element handling

    Too much negative spacing jumbles up the letters. Not all elements have equal width so applying negative tracking can move an element (e.g the comma (,) in Hello , World), to a position that is wrong - less x position than it's previous letter which is (O) in this case. So when re-applying positive tracking the result becomes Hell,O World(a lil bit of irony here).

  3. Most importantly, robustness

    This might be a reason to downvote this question since I'm not sure I can explain what I mean here. This 'algorithm' is destined for a client, so I would like some expert second opinion before demonstrating it live

What I have so far:

function calcMultiTrack() {

      //Startup Variables
        var trackTolerance = 0.01;
        var compPrecision = 2; 


        //Get value from input element. How much spacing?
        var trackingAmount = parseInt($("#tracking").val());

        //Define new empty array
        var selItemsDimArray = [];

        //Push into new array the same elements as multiSelectedCopy but with more properties per element(x2,y2 is calculated here)
        for (var i = multiselectedCopy.length - 1; i >= 0; i--) {

            var bb = svgCanvas.getStrokedBBox([multiselectedCopy[i]]); //getStrokedBBox returns x,y,width,height for an element.

            selItemsDimArray.push({
                'examinedElem': multiselectedCopy[i],
                'x': bb.x,
                'y': bb.y,
                'x2': (bb.x + bb.width),
                'y2': (bb.y + bb.height),
                'height': bb.height,
                'width': bb.width
            });

        };

        //Sort array starting from most left(smallest x position meaning it's the most left element.)
        selItemsDimArray.sort(function(a, b) {
            return a.x - b.x;
        });


        //Most left item does not move so define now a calc distance of 0. calc distance is the distance we will have to move an element later.
        selItemsDimArray[0]['calcDistance'] = 0;


        //Calculate and push diff - diff is the distance of an element from the previous(next smallest x position)
        for (var i = 1; i < selItemsDimArray.length; i++) {
            var current = selItemsDimArray[i];
            var previous = selItemsDimArray[i - 1];
            var diff = current.x - previous.x2;

            selItemsDimArray[i]['diff'] = diff;


            //Handle negative diffs (e.g letter has negative spacing from previous)
            if (diff < 0) {
                selItemsDimArray[i]['calcDistance'] = (trackingAmount + (-diff)) + selItemsDimArray[i - 1].calcDistance
            } else {
                selItemsDimArray[i]['calcDistance'] = (trackingAmount - diff) + selItemsDimArray[i - 1].calcDistance
            };
        };


        //Check if element needs to actually move or not. If yes, move it - Use "calcDistance" property that was calculated earlier.
        for (var i = 1; i < selItemsDimArray.length; i++) {
            var aboutCompare = selItemsDimArray[i].diff.toFixed(compPrecision); //Strip off unnecessary decimal points - We don't need to split atoms.
            if (aboutCompare != trackingAmount) {          
                svgCanvas.moveSingleElement(selItemsDimArray[i].examinedElem, selItemsDimArray[i].calcDistance, 0, true); //moveSingleElement moves an element by an x,y value.
            }
        }

//Check if elements really reached their final position? Check is done by averaging all the elements current spacing.
       var average=0;
       for (var i = 0; i < selItemsDimArray.length; i++) {
        if(typeof selItemsDimArray[i].diff !== 'undefined'){
            average = average+selItemsDimArray[i].diff;
 };
       };

       average = average/(selItemsDimArray.length-1);

       //if the average spacing is not the same as the tracking we need, run again the whole algorithm. 
       if (Math.abs(average - trackingAmount) > trackTolerance){
        calcMultiTrack();
       }
    }
    //End of algorithm
\$\endgroup\$
2
\$\begingroup\$

Picking up on I will show this to a client

  • Run this through a jsbeautifier for indenting, also 2 new lines is overkill in code
  • Run this code through jshint, the biggest detractors are
    • selItemsDimArray[i]['calcDistance'] should be selItemsDimArray[i].calcDistance
    • Both missing and extraneous semicolons

Furthermore, from a once over:

  • var trackingAmount = parseInt($("#tracking").val()); either use a radix for parseInt or stop using parseInt and use + instead:
    var trackingAmount = +$("#tracking").val());
  • I am for commenting, but, comments have to add value, this does not add value:

    //Define new empty array
    var selItemsDimArray = [];
    
  • I think Math.abs could help here:

        if (diff < 0) {
            selItemsDimArray[i]['calcDistance'] = (trackingAmount + (-diff)) + selItemsDimArray[i - 1].calcDistance
        } else {
            selItemsDimArray[i]['calcDistance'] = (trackingAmount - diff) + selItemsDimArray[i - 1].calcDistance
        };
    
  • for (var i = 1; i < selItemsDimArray.length; i++) { <- You have this loop twice, you should merge those 2 loops
  • This is interesting:

    //if the average spacing is not the same as the tracking we need, run again the whole algorithm. 
    if (Math.abs(average - trackingAmount) > trackTolerance){
      calcMultiTrack();
    }
    

You are using recursive calls here for no good reason, you should work this into a while statement inside your function.

\$\endgroup\$

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.