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;
 }
 }
 }
200_success
145k22 gold badges190 silver badges478 bronze badges
asked Jan 28, 2017 at 18:10
\$\endgroup\$

2 Answers 2

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.
answered Jan 28, 2017 at 18:51
\$\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.

answered Jan 28, 2017 at 18:53
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.