1
\$\begingroup\$

I have code to find images in my selections. But when nothing in selection (loop FOR) and selection is kinda big, it's kinda laggy. How can I improve my code?

  var toStageCoord = map.stageXYToCoordinates(toStage.x + selection.x1, toStage.y + selection.y1);

  var element1 = document.elementFromPoint(toStage.x + selection.x1, toStage.y + selection.y1);
  var element2 = document.elementFromPoint(toStage.x + selection.x2, toStage.y + selection.y2);
  var element3 = document.elementFromPoint(toStage.x + selection.x2, toStage.y + selection.y1);
  var element4 = document.elementFromPoint(toStage.x + selection.x1, toStage.y + selection.y2);
  var found = false;


  if(element1.localName == "image" || element2.localName == "image" || element3.localName == "image" || element4.localName == "image") {
    found = true;console.log("foundfirst");
  }

  for(var y = toStage.y + selection.y1;y < toStage.y + selection.y2 ;y++) {
    if(found){break;}

    for(var x = toStage.x + selection.x1;x < toStage.x + selection.x2;x++) {
      var element = document.elementFromPoint(x,y);

      if(element.localName === "image") {

        console.log("found");
        found = true;
        break;
      }
    }
  }
\$\endgroup\$
1
\$\begingroup\$

Some improvements:

var toStageCoord = map.stageXYToCoordinates(toStage.x + selection.x1, toStage.y + selection.y1);

var element1 = document.elementFromPoint(toStage.x + selection.x1, toStage.y + selection.y1);
var element2 = document.elementFromPoint(toStage.x + selection.x2, toStage.y + selection.y2);
var element3 = document.elementFromPoint(toStage.x + selection.x2, toStage.y + selection.y1);
var element4 = document.elementFromPoint(toStage.x + selection.x1, toStage.y + selection.y2);

if(element1.localName == "image" || element2.localName == "image" || element3.localName == "image" || element4.localName == "image") {
  console.log("foundfirst"); // 4
}
else { // 2
  var maxy = toStage.y + selection.y2; // 1
  var maxx = toStage.x + selection.x2;

  outerloop: // 2
  for(var y = toStage.y + selection.y1; y < maxy; y+=4) { // 1, 3

    for(var x = toStage.x + selection.x1; x < maxx; x+=4) { // 1, 3
      var element = document.elementFromPoint(x,y);

      if(element.localName == "image") { // 4
        console.log("found");
        break outerloop; // 2
      }
    }
  }
}

Comments:

  1. These calculations were performed at every iteration of the loops, while they never changed. So, let's move them outside the loops.
  2. Using a label, like this outerloop: one can direct break to which block/loop to exit. So, the variable found is not needed.
  3. Depending on the images' sizes, you don't need to check every pixel. Try a larger gap, like 4 or even 10 pixels, as this would reduce the number of iterations.
  4. In the first check the operator == is used, while in the loop === is used. As === checks the type of the value, too, it would be slower. If == yields the correct answer, too, then it should be used instead.
\$\endgroup\$
0
\$\begingroup\$

As someone already answered in the copy of this question on StackOverflow, it is much more efficient to iterate over all images in your site, and check for each one if it overlaps the selection, than to iterate all points in the selection (we're talking hundreds of thousands for a moderate selection) and check the existence of images on each one.

This answer provides an example solution. It's written with jQuery but it should be adaptable to vanilla JavaScript.

\$\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.