Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optimise Side.fromDirection(direction: Vector): Side #3006

Closed
ikudrickiy opened this issue Apr 5, 2024 · 3 comments · Fixed by #3114
Closed

Optimise Side.fromDirection(direction: Vector): Side #3006

ikudrickiy opened this issue Apr 5, 2024 · 3 comments · Fixed by #3114
Labels
stale This issue or PR has not had any activity recently

Comments

@ikudrickiy
Copy link
Contributor

ikudrickiy commented Apr 5, 2024

Context

Now this function to process dominant direction from a vector finds the most directionally similar vector between Vector.Left, Vector.Right, Vector.Up and Vector.Down computing the dot product twice(!) for each pair and finding the maximum result to get corresponding array index later. For some reason it also treats -Number.MAX_VALUE as the lowest number ever when in reality it is -Infinity.

Proposal

I propose to determine the dominant direction by comparing the coordinates of the vector with each other to determine a quarter of plane this vector belongs to and along with this the dominant direction. The code below will have one shared ray assigned to the bottom instead of right. But it can be changed adding extra comparison. Among other things, such code is less prone to rounding errors.

Plane partitioning diagram

image

Javascript code demonstrating the idea:

My original solution
if (y >= x) {
  //left or bottom
  if (y <= -x)
    //LEFT or top
    return Side.Left;

  //if (y === x) return Side.Right;

  //right or BOTTOM
  return Side.Bottom;
}

//right or top
if (y >= -x)
  //RIGHT or bottom
  return Side.Right;

//left or TOP
return Side.Up;
More readable solution from `BoundingBox.getSideFromIntersection(intersection: Vector): Side`
if (Math.abs(x) >= Math.abs(y)) {
  if (x <= 0) return Side.Left;

  return Side.Right;
}

if (y <= 0) return Side.Top;

return Side.Bottom;
@ikudrickiy
Copy link
Contributor Author

ikudrickiy commented Apr 6, 2024

public static getSideFromIntersection(intersection: Vector): Side {

has similar logic but its solution is way more readable

P.S. in this file the minimum number is rigthly -Infinity opposing the main file ot this Issue

    let maxX = -Infinity;

Copy link

This issue hasn't had any recent activity lately and is being marked as stale automatically.

@github-actions github-actions bot added the stale This issue or PR has not had any activity recently label Jun 15, 2024
@eonarheim
Copy link
Member

@ikudrickiy Sorry for the delayed response! But this looks great!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale This issue or PR has not had any activity recently
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants