[SofaGeneralLoader] fix GIDMeshLoader and add example

[EulalieCoevoet]

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...

• I've replaced all beginEdit() with getWriteOnlyAccessor() and removed the endEdit() 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 to addTriangle(...), addEdge(...), addQuad(...), etc., which are implemented in MeshLoader.
For the moment, addTriangle is the only one which has been duplicated to work with data returned by getWriteOnlyAccessor().
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 all addEdge(...), 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:

• builds with SUCCESS for all platforms on the CI.
• does not generate new warnings.
• does not generate new unit test failures.
• does not generate new scene test failures.
• does not break API compatibility.
• is more than 1 week old (or has fast-merge label).

Reviewers will merge only if all these checks are true.

[hugtalbot]

Hi @EulalieCoevoet

Thanks for the PR.
You mentioned in #1534 that "The problem I have is that the component is not working with the update callback mechanism". What was the update process you were expecting? Could you detail what you meant by not working please?

After this PR we will take a look at the MeshGmsh one.

[EulalieCoevoet]

Well:

• doLoad() was called three times, without clearing the lists (e.g. d_position, d_tetra...). So I had duplicated positions, triangles and tetra in my scene, which caused failures. Those calls seemed to be triggered by addUpdateCallback in MeshLoader.
• the transforms wasn't called three times, so the last positions were not transformed.... but that's not really important I guess.

After digging, I found that using getWriteOnlyAccessor() to access the d_positions... instead of beginEdit(), like it was done in most other loaders, reduced this number of calls, which fixed everything for me.

I should also mention that my scene was failing at init (when loading the scene).

[damienmarchal]

+1 for using getWriteOnlyAccessor. Never ever use begin/endEdit.

[EulalieCoevoet]

If you all agree with Damien I can make the changes in this PR.
No need to change addEdge() etc... I see what should be done now.

[EulalieCoevoet]

So at the end the PR also fixes MeshGmshLoader, MeshOffLoader, and MeshTrianLoader.
If you want to reproduce the bug you can run the corresponding examples on master, you'll see that too many positions are being loaded.

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.

[epernod]

[ci-build][with-all-tests]

[epernod]

@EulalieCoevoet when you say it fix MeshGmshLoader, you mean only the getWriteOnlyAccessor or was this loader also loading too much point at start?
I'm using this loader a lot to work on topology and I didn't notice that problem.

[EulalieCoevoet]

You're right.
When I opened square3.msh (from the example) with Gmsh it didn't show me the same number of elements, so I assumed that the component had the same problem (still don't know where this number comes from).
Anyway you're right the PR doesn't change the behavior of MeshGmshLoader, only the others.