[GL] fix segfault when drawing Rigid3d frame

[bakpaul]

When trying this dummy example I had a segfault when selecting the mstate to be drawn.
This is beacause the key type mismatched the object real type and thus, finding in map wasn't working correctly so the second call to at, even though the map was filled right before, endend in a segfault.

<Node name="Root" gravity="0 -10 0" time="0" animate="0">
    <RequiredPlugin name="Sofa.Component.StateContainer"/> <!-- Needed to use components [MechanicalObject] -->
    <DefaultAnimationLoop/>
    <MechanicalObject template="Rigid3d" name="dofs" position="0 0 0  0 0 0 1"/>
</Node>

---

By submitting this pull request, I acknowledge that
I have read, understand, and agree SOFA Developer Certificate of Origin (DCO).

---

Reviewers will merge this pull-request only if

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

[damienmarchal]

Hi Paul,

Thank for fixing that, for the moment I don't quite understand the problem and the fix, so unless someone understand that please wait for fastmerge.

NB: I just noticed that the frame drawing has caching mechanisme.. but as you are now using adresses ... I wonder if there is no risk of fill the cache too fast. Maybe a maximum-cache parameters with auto cleaning the least recently used frames.

EDIT: I tried the example and didn't managed to reproduce the problem.

[damienmarchal]

In case the issue is confirmed... (i was not able to reproduced) I made this PR bakpaul#37 to avoid the use of pointer's which are not stable.

Actually I'm a bit confused by the purpose of this caching mechanisme... caching by pointers means that same adress render the same ? Even if the frame moved ?

[damienmarchal]

Hi @bakpaul, I also have problem on crashes related to rigidframe rendering.
It is unclear to me where this come from... but I'm not sure the fix is the proper one.

Actually I don't also understand the logic of the caching system.

[damienmarchal]

Hi @bakpaul

I think the problem is not related to how the caching system (which is still think look very weird to me... ) but to the line:

 glDrawElements(GL_TRIANGLES, comp.triangle_count * 3, GL_UNSIGNED_INT,  comp.triangles );

Because struct Triangles {int v0, v1, v2} and not uint so it should be reinterpreted and correctly typed as in:

glDrawElements(GL_TRIANGLES, comp.triangle_count * 3, GL_INT, reinterpret_cast<const int*>(comp.triangles) );

[damienmarchal]

@fredroy ... could you explain the why of the caching system ?

To me the general design of high performance rendering should be to draw batch of frames to the gl subsystem to minimize gl state change. The two way I could see for doing such a thing are:

• have a drawFrames() API instead of drawFrame to allow the caller to send batches
• have a kind of automatic batching of the frame to draw...and render them once per frame (à la "imgui" :))

[fredroy]

@fredroy ... could you explain the why of the caching system ?

To me the general design of high performance rendering should be to draw batch of frames to the gl subsystem to minimize gl state change. The two way I could see for doing such a thing are:

* have a drawFrames() API instead of drawFrame to allow the caller to send batches

* have a kind of automatic batching of the frame to draw...and render them once per frame (à la "imgui" :))

1- drawFrame() has a annoying size parameter which is not applied linearly on all the frame (different scale between the tip and the body of the arrows)
2- It was done like that before because of the GLU quadrics which was really slow to generate vertices and such.
3- DrawToolAPI is implemented as "direct calls" (or similar) so it is supposed to draw as soon as you call it.

So this map (which was already here before) was/is to store the already created vertices/triangles.
Obviously it could be implemented in a much better and optimized way (first with a batch of Frames like you said) with buffers , instancing, etc but it will need a complete revamp of the DrawTool (and/or using an external library)
And usually DrawTool is used mainly for "debugging purpose" and not so for "high performance" rendering.