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

Update Magnum submodules to latest, fix AssimpImporter assertion #1911

Merged
merged 1 commit into from
Nov 4, 2022

Conversation

mosra
Copy link
Collaborator

@mosra mosra commented Oct 24, 2022

Motivation and Context

Mainly includes a change to AssimpImporter that avoids returning a growable array. Which is problematic when a static plugin gets used across more than one dynamic library, leading to the following assertion:

Trade::AbstractImporter::mesh(): implementation is not allowed to use a custom Array deleter

Details in mosra/magnum-plugins@10b7e94.

How Has This Been Tested

My CIs pass, ⚠️ however ⚠️ there's a very slight chance that the change could make previously-silently-wrongly-imported models now assert -- tighter checks in the plugin could trigger potential new Assimp bugs. So please verify with any OBJ/PLY/STL/COLLADA models you have, as well as the one that caused the above assertion in the first place.

Types of changes

  • Docs change / refactoring / dependency upgrade
  • Bug fix (non-breaking change which fixes an issue)

Mainly includes a change to AssimpImporter that avoids returning a
growable array which is problematic when a static plugin gets used
across more than one dynamic library.
@facebook-github-bot facebook-github-bot added the CLA Signed Do not delete this pull request or issue due to inactivity. label Oct 24, 2022
jrreyna added a commit that referenced this pull request Oct 25, 2022
Merging from PR: "Update Magnum submodules to latest, fix AssimpImporter assertion #1911" to bring Mosra's AssetImporter fix into my branch
jrreyna added a commit that referenced this pull request Oct 25, 2022
…orter assertion #1911' into my branch to fix an assertion error with AssetImporter
@mosra mosra merged commit c606c77 into main Nov 4, 2022
@mosra mosra deleted the update-magnum18 branch November 4, 2022 18:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed Do not delete this pull request or issue due to inactivity.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants