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

[glm] Bump to 2023-06-08 & fix usage #32685

Merged
merged 3 commits into from
Aug 24, 2023
Merged

Conversation

xiaozhuai
Copy link
Contributor

@xiaozhuai xiaozhuai commented Jul 21, 2023

  • Changes comply with the maintainer guide
  • SHA512s are updated for each updated download
  • The "supports" clause reflects platforms that may be fixed by this new version
  • Any fixed CI baseline entries are removed from that file.
  • Any patches that are no longer applied are deleted from the port's directory.
  • The version database is fixed by rerunning ./vcpkg x-add-version --all and committing the result.
  • Only one version is added to each modified port's versions file.
  1. Bump to 0.9.9.8+20230608. Since latest release (0.9.9.8) of glm is about 3 years ago, there have been many important fixes and improvements during this time.
  2. Fix usage

BillyONeal
BillyONeal previously approved these changes Jul 21, 2023
@LilyWangLL LilyWangLL added the category:port-update The issue is with a library, which is requesting update new revision label Jul 24, 2023
@LilyWangLL
Copy link
Contributor

Usage passed on x64-windows.

LilyWangLL
LilyWangLL previously approved these changes Jul 25, 2023
@LilyWangLL LilyWangLL added the info:reviewed Pull Request changes follow basic guidelines label Jul 25, 2023
@BillyONeal
Copy link
Member

BillyONeal commented Jul 25, 2023

Unfortunately for version precedence rules 0.9.9.8+20230608 == 0.9.9.8, as things after the + are the 'build metadata' and not relevant for purposes of ordering. See https://semver.org/#spec-item-10

@christophe-lunarg Any chance of glm doing a real release soon?

If upstream won't do a release I think we have no choice but to switch to version-date.

@BillyONeal BillyONeal added depends:upstream-changes Waiting on a change to the upstream project and removed info:reviewed Pull Request changes follow basic guidelines labels Jul 25, 2023
@BillyONeal BillyONeal marked this pull request as draft July 25, 2023 18:09
@xiaozhuai
Copy link
Contributor Author

If upstream won't do a release I think we have no choice but to switch to version-date.

@BillyONeal
How about keep the version 0.9.9.8 and just increase port-version?

@BillyONeal
Copy link
Member

@BillyONeal How about keep the version 0.9.9.8 and just increase port-version?

That would sort the right way. Do you think users would be 'surprised' at the changes they get since then? Has upstream indicated they are going for 'live at head'? If the changes since then aren't 'small bugfixes' then I think we should go version-date.

@dg0yt
Copy link
Contributor

dg0yt commented Jul 27, 2023

Do you think users would be 'surprised' at the changes they get since then?

Repology will say that vcpkg is lying about the version.

for version precedence rules 0.9.9.8+20230608 == 0.9.9.8

Then how about using 0.9.9.8+20230608 and increasing the port number?
(I guess the key difficulty is explaining this to the reviewers...)

@xiaozhuai
Copy link
Contributor Author

Then how about using 0.9.9.8+20230608 and increasing the port number?

I vote this.

@BillyONeal @dg0yt

What's your opinion?

@xiaozhuai xiaozhuai dismissed stale reviews from LilyWangLL and BillyONeal via 332e48a August 17, 2023 04:42
@xiaozhuai xiaozhuai changed the title [glm] Bump to 0.9.9.8+20230608 & fix usage [glm] Bump to 2023-06-08 & fix usage Aug 17, 2023
@xiaozhuai
Copy link
Contributor Author

@BillyONeal @dg0yt
It looks like there is no interest upstream in releasing a new version, I've update this pr and switch to version-date

@xiaozhuai xiaozhuai marked this pull request as ready for review August 17, 2023 04:48
@dan-shaw dan-shaw merged commit a880596 into microsoft:master Aug 24, 2023
15 checks passed
@xiaozhuai xiaozhuai deleted the dev-glm branch August 24, 2023 05:19
@LilyWangLL LilyWangLL added info:reviewed Pull Request changes follow basic guidelines and removed depends:upstream-changes Waiting on a change to the upstream project labels Aug 24, 2023
@dsa-t
Copy link

dsa-t commented Oct 16, 2023

This update breaks our application (KiCad), because values in quaternions are stored differently compared to 0.9.9.8.

Order in 0.9.9.8: x, y, z, w
https://github.com/g-truc/glm/blob/0.9.9.8/glm/detail/type_quat.hpp#L48C31-L48C31

Order in 2023-06-08: w, x, y, z
https://github.com/g-truc/glm/blob/5c46b9c07008ae65cb81ab79cd677ecc1934b903/glm/detail/type_quat.hpp#L48C31-L48C31

Looks like glm should be updated again, with GLM_FORCE_QUAT_DATA_XYZW set to restore the order.

@xiaozhuai
Copy link
Contributor Author

@dsa-t
Thanks for reporting this, I look into it, but there is nothing we should do with this port.
You can define GLM_FORCE_QUAT_DATA_WXYZ in your user project.

@dsa-t
Copy link

dsa-t commented Oct 16, 2023

Another thing is, GLM_FORCE_QUAT_DATA_XYZW likely won't work properly before g-truc/glm@c660699

@xiaozhuai
Copy link
Contributor Author

Another thing is, GLM_FORCE_QUAT_DATA_XYZW likely won't work properly before g-truc/glm@c660699

Yes, this port currently use g-truc/glm@5c46b9c, which is more later than g-truc/glm@c660699

@dsa-t
Copy link

dsa-t commented Oct 16, 2023

@xiaozhuai xiaozhuai mentioned this pull request Oct 17, 2023
7 tasks
@xiaozhuai
Copy link
Contributor Author

xiaozhuai commented Oct 17, 2023

@dsa-t
Confirmed, I've submit a pr #34523 to fix it. Thanks for reporting.
BTW, the default behavior of GLM requires the users to define GLM_FORCE_QUAT_DATA_XYZW themselves, so vcpkg will not define it by default.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:port-update The issue is with a library, which is requesting update new revision info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants