-
Notifications
You must be signed in to change notification settings - Fork 311
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
[SofaGeneralLoader] fix GIDMeshLoader and add example #1554
Conversation
Thanks for the PR. After this PR we will take a look at the MeshGmsh one. |
Well:
After digging, I found that using I should also mention that my scene was failing at init (when loading the scene). |
+1 for using getWriteOnlyAccessor. Never ever use begin/endEdit. |
If you all agree with Damien I can make the changes in this PR. |
The bug: the mesh was loaded to many times without clearing the lists, resulting to duplicated positions and elements. The fix: use getWriteOnlyAccessor instead of beginEdit.
So at the end the PR also fixes MeshGmshLoader, MeshOffLoader, and MeshTrianLoader. Breaking: Btw, I've removed the duplicated function addTriangle()... It may break some loaders that I haven't compiled, but I think it's cleaner this way. |
[ci-build][with-all-tests] |
@EulalieCoevoet when you say it fix MeshGmshLoader, you mean only the |
You're right. |
Hi,
Here's my fix for issue #1534. The problem was the use of
beginEdit()
,endEdit()
... I'm not sure what was happening exactly,but it looked like it was triggering extra callback...
beginEdit()
withgetWriteOnlyAccessor()
and removed theendEdit()
when there was any.I'm pretty sure
MeshGmshLoader
is also broken. I was about to fix it, but I came across another problem.MeshGmshLoader
uses calls toaddTriangle(...)
,addEdge(...)
,addQuad(...)
, etc., which are implemented inMeshLoader
.For the moment,
addTriangle
is the only one which has been duplicated to work with data returned bygetWriteOnlyAccessor()
.I think we all agree that duplication of code should be avoided. I'm not sure what we should do here.
Should we generalize the use of
getWriteOnlyAccessor()
in all loaders, and change alladdEdge(...)
,addQuad(...)
, etc., signature once and for all (no duplication)?Or should it be fine to use
beginEdit()
, and my fix doesn't solve the real problem?Fixes #1534
This PR:
Reviewers will merge only if all these checks are true.