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

Fix Vector2f/d.perpendicular() #230

Merged
merged 4 commits into from
Apr 28, 2020
Merged

Conversation

ieperen3039
Copy link
Contributor

@ieperen3039 ieperen3039 commented Apr 28, 2020

the calculation for y used the new value of x

ofc in Vector2d, the temp is a double
@httpdigest
Copy link
Member

Thanks for finding and fixing! Those kind of bugs were all introduced by 50359e9#diff-5f25b65b863f5962b1afa652e0479798L507 (see the Vector2d diff).
Sadly, this was a simple "inline refactor" done via Eclipse, which I assumed would be correct. However it totally screwed things up in this case.

@httpdigest httpdigest added this to the 1.9.25 release milestone Apr 28, 2020
@httpdigest httpdigest merged commit 2c119bd into JOML-CI:master Apr 28, 2020
@httpdigest
Copy link
Member

httpdigest commented Apr 28, 2020

Yeah, Eclipse's method invocation inlining refactor (Shift+Alt+I) is to blame. Take for example this simplified case:

public class Vector2f {
  public float x, y;
  public Vector2f perpendicular() {
    return this.set(y, -x);
  }
  private Vector2f set(float x, float y) {
    this.x = x;
    this.y = y;
    return this;
  }
}

and inline the this.set(y, -x) invocation in perpendicular(). The result will be:

public Vector2f perpendicular() {
  this.x = y;
  this.y = -x;
  return this;
}

which is totally wrong here. Apparently, Eclipse's method inline refactor (Eclipse version 2020.03) is broken, not taking into account Java's call-by-value argument passing, necessitating a temporary variable in this case.

@ieperen3039
Copy link
Contributor Author

I believe IntelliJ does it correctly

@ieperen3039 ieperen3039 deleted the patch-2 branch April 29, 2020 09:55
@httpdigest httpdigest changed the title Fix perpendicular caluclations of Vector2f and Vector2d Fix Vector2f/d.perpendicular() May 31, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants