Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Accelerate the Merge vertex post processing step#4527

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
kimkulling merged 6 commits intoassimp:masterfrommotazmuhammad:master
May 14, 2022

Conversation

motazmuhammad
Copy link
Contributor

@motazmuhammadmotazmuhammad commentedMay 13, 2022•
edited
Loading

The function int JoinVerticesProcess::ProcessMesh( aiMesh* pMesh, unsigned int meshIndex)
can be accelerated by using an std::unordered_map instead of binary search to create the unique vertices.
To do that the following was done.

  1. Define a hash for a vertex
  2. Define an equal_to for a vertex
  3. loop over all the vertices if the vertex is already taken use the map to get its index, else add it.

Effectively, this should reduce the complexity from O(nlog(n)) to O(n). Therefore, accelerating the postprocessing step

Copy link
Member

@kimkullingkimkulling left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Looks fine, I will merge it,

@kimkulling
Copy link
Member

kimkulling commentedMay 14, 2022•
edited
Loading

Nice approach, hopefully, the memory consumption will not explode for big models. But this will be a different story.

@kimkullingkimkulling merged commitc8dafe0 intoassimp:masterMay 14, 2022
@kimkulling
Copy link
Member

Merged, thanks a lot for your contribution.

@motazmuhammad
Copy link
ContributorAuthor

motazmuhammad commentedMay 14, 2022 via email

Thank you very much for accepting my contribution.
…
-- This message and its contents, including attachments are intended solelyfor the original recipient. If you are not the intended recipient or havereceived this message in error, please notify me immediately and deletethis message from your computer system. Any unauthorized use ordistribution is prohibited. Please consider the environment before printingthis email.

@CarpetHead
Copy link

CarpetHead commentedAug 23, 2022•
edited
Loading

We are having an issue in assimp v5.2.4 where the merge vertices postprocessing step breaks rigged models. Could this PR be the cause? We are now seeing duplicated vertex weights. See attached image for 5.2.4 and 5.2.3 vertex weights per bone
image

@nezticle
Copy link

We are having an issue in assimp v5.2.4 where the merge vertices postprocessing step breaks rigged models. Could this PR be the cause? We are now seeing duplicated vertex weights. See attached image for 5.2.4 and 5.2.3 vertex weights per boneimage

We have experience this same thing in Qt Quick 3D with 5.2.4. It breaks Morph animations because all of the weights are identical. We're reverting to 5.2.3 for the time being.

@JG-Adams
Copy link
Contributor

JG-Adams commentedAug 26, 2022•
edited
Loading

I have not experienced this issue. And I'm not sure how to recreate it.
But I tried to understand the change made here. It look like inside JoinVerticesProcess::ProcessMesh the mechanism to avoid doubles was replaced with something outside of it? Where does it function in?
unordered map by itself shouldn't take more than one of any kind.
This need some explanation, because it don't make a whole lot of sense to me.
We should review the old code. Because I notice the comment says it does something important that otherwise would mess up the mesh.

Edit: then again. It shouldn't have caused any problem should it?

@inhosens
Copy link
Contributor

This patch set does not compare color, texcoords and anim meshes. I am not sure it can accelerate the routine when the whole comparisons are added.

@CarpetHead
Copy link

btw it looks like this PR fixed the issuehttps://github.com/assimp/assimp/pull/4707/files

@JG-Adams
Copy link
Contributor

btw it looks like this PR fixed the issuehttps://github.com/assimp/assimp/pull/4707/files

That look like it does. :)

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@kimkullingkimkullingkimkulling left review comments

Assignees
No one assigned
Labels
None yet
Projects
No open projects
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

6 participants
@motazmuhammad@kimkulling@CarpetHead@nezticle@JG-Adams@inhosens
[8]ページ先頭
©2009-2025 Movatter.jp