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

Geom#linear_combination breaking API change in SU23 #1007

Open
Eneroth3 opened this issue Oct 28, 2024 · 4 comments
Open

Geom#linear_combination breaking API change in SU23 #1007

Eneroth3 opened this issue Oct 28, 2024 · 4 comments

Comments

@Eneroth3
Copy link

The behavior of Geom.linear_combination changed between SketchUp 2022 and 2023. In older versions the vectors where normalized (length set to 1) but in newer versions the vectors are combined, with their original length. This creates different geometric output and can make an extension behave incorrectly.

vector1 = Geom::Vector3d.new(10, 0, 0)
vector2 = Geom::Vector3d.new(0, 1, 0)

Geom.linear_combination(0.5, vector1, 0.5, vector2)

# SketchUp 2022 and earlier
#=> Vector3d(0.5, 0.5, 0)

# SketchUp 2023 and later
# => => Vector3d(5, 0.5, 0)
Eneroth3 added a commit to Eneroth3/townhouse-system that referenced this issue Oct 28, 2024
@Fredosixx
Copy link

I noticed it too, and had to change my code in one place by forcing normalization.

The main issue is that it was not easy to find out that Geom.linear_combination on vectors was the culprit from the change of behaviour in the plugin reported by users.

I did not log the problem here because there is nothing in the API documentation stating that vector should be normalized. So, I anticipated that the issue would be rejected anyway (or at best ignored).

@Eneroth3
Copy link
Author

I'm sorry you feel disincentivized to log issues. I hope we can change this.

It can vey well be argued the original behavior was a bug that got fixed in 2023. However at the least the docs need to mention the change.

This is a change in the public SketchUp Ruby API, not in the internal workings of SketchUp that is only accessed by unofficial API, and it breaks extensions.

@Fredosixx
Copy link

It is true that we often refer to the API documentation text as the truth for dos and don'ts. On this particular issue, I considered it was me making the mistake (i.e. linear_combination of vectors does not do normalization by itself).

If you have the power to update the online API documentation with the change, then I guess you could do it and close the issue.

@thomthom
Copy link
Member

thomthom commented Oct 30, 2024

It can vey well be argued the original behavior was a bug that got fixed in 2023. However at the least the docs need to mention the change.

Yea, actually sounds like a bug in the "old" behaviour. The docs describe:

A linear combination is a standard term for vector math. It is defined as vector = weight1 * vector1 + weight2 * vector2.

Normalizing doesn't sound expected.

Though changing API behaviour isn't ideal. If we'd caught this before release I think I'd preferred it stayed the same. But now that it is released I think its best to keep it and document it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants