[all] Use drawtool everywhere #704

hugtalbot · 2018-06-28T10:14:41Z

Apply the use of drawTool in the open-source core of SOFA

This now makes SOFA a priori independent from OpenGL using the drawTools.
Fix #653

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.

…ivePerformer, EvalPointsDistance, EvalSurfaceDistance

…Field SofaValidation

…id, SofaGeneralSimpleFem, SofaGeneralVisual, SofaMiscFem

…ctor)

ng branch 'upstream/master' into use_drawtool_everywhere # Please enter a commit message to explain why this merge is necessary, # especially if it merges an updated upstream into a topic branch. # # Lines starting with '#' will be ignored, and an empty message aborts # the commit.

damienmarchal · 2018-07-04T07:13:24Z

[ci-build]

damienmarchal · 2018-07-04T07:13:34Z

[ci-build][with-scene-tests]

damienmarchal · 2018-07-04T08:46:43Z

SofaKernel/framework/sofa/core/visual/DrawToolGL.cpp

+void DrawToolGL::drawTriangles(const std::vector<Vector3> &points, const std::vector< Vec4f > &colour)
+{
+    std::vector<Vector3> normal;
+    normal.clear();


This means that if the normal is empty it does nothing ?
Maybe add some comment to explain the logic the declaration of drawTriangles()

No, actually if you look closer, this function creates a vector of normals and then calls another function (l. 277) that computes normals if the vector has a null size

damienmarchal · 2018-07-04T08:49:02Z

SofaKernel/framework/sofa/core/visual/DrawToolGL.cpp

 {
+    glLineWidth(size);


this will break code that have set the drawLine before.
cannot push/pop the attrib otherwise ?

actually no since @fredroy set a default value for "size" to one, for functions calling it without specifying the value

The normal convention is to save the attributes before changing them and restoring them to their old value when you are done. In your case you restore it to 1.0 regardless of its initial value this means that calling the function has a side effect on all future opengl drawing operation. It is way better to have side effect free functions so that the one that call them does not have to care about what the function is doing.

damienmarchal · 2018-07-04T08:50:31Z

SofaKernel/modules/SofaBaseTopology/TriangleSetGeometryAlgorithms.inl

@@ -2401,7 +2401,7 @@ void TriangleSetGeometryAlgorithms<DataTypes>::draw(const core::visual::VisualPa

        const sofa::helper::vector<Triangle> &triangleArray = this->m_topology->getTriangles();

-        helper::vector<defaulttype::Vector3> positions;
+        std::vector<defaulttype::Vector3> positions;


Shouldn't we use helper::vector as much as possible ?

we could but @fredroy told me that he wanted to make it as independent from SOFA as possible (in this optic, we should also remove the defaulttype calls).
But in general you're right, for the rest of SOFA, we should use helper::vector as much as possible

I fear so much inconsistancies :)

damienmarchal · 2018-07-04T08:51:22Z

applications/plugins/SofaMiscCollision/TetrahedronModel.h

@@ -108,6 +108,7 @@ class SOFA_MISC_COLLISION_API TetrahedronModel : public core::CollisionModel
 protected:

    TetrahedronModel();
+    void addTetraToDraw(const Tetrahedron& t, std::vector<sofa::defaulttype::Vector3>& tetraVertices, std::vector<sofa::defaulttype::Vector3>& normalVertices);


Aren't the other parameters const ?

nope, by looking into this function, its purpose is actually to build a vector and set new values in it

hugtalbot · 2018-07-04T08:52:52Z

Remark : I need to fix the test on planeForceField

tgaugry · 2018-07-04T08:48:45Z

applications/plugins/SofaMiscCollision/TetrahedronModel.cpp

+    p3 += (c - p3)*0.1f;
+    p4 += (c - p4)*0.1f;
+    Coord n1, n2, n3, n4;
+    n1 = cross(p3 - p1, p2 - p1); n1.normalize();


maybe use a lambda to reduce the amount of code and make it simpler ?

update done

tgaugry · 2018-07-04T08:52:00Z

modules/SofaConstraint/SlidingConstraint.inl

-    glEnd();
-    glPointSize(1);
-#endif /* SOFA_NO_OPENGL */
+        color = sofa::defaulttype::RGBAColor::magenta();


Same for the following block of code. it would be simpler with a do_whatever(...) lambda function

tgaugry · 2018-07-04T08:53:41Z

modules/SofaGeneralDeformable/QuadularBendingSprings.inl

            }
            else
            {
                if (d2<edgeInf[i].restlength2*0.9999)
-                    glColor4f(1,0.5f,0,1);
+                {
+                    colors.push_back(sofa::defaulttype::RGBAColor(1,0.5, 0,1));


Maybe put sofa::defaulttype::RGBAColor(1,0.5, 0,1) and friends as constexpr ?

update done

guparan · 2018-07-18T09:05:48Z

TODO: solve conflict and build with scene tests

…l_everywhere # Conflicts: # modules/SofaMiscForceField/MeshMatrixMass.inl

Data to show the plane is reactivated Names are updated in showPlane and showPlaneScale

fredroy and others added 19 commits June 20, 2018 18:04

UPDATE: Use DrawTools...

4aacbfb

UPDATE: Use DrawTools...

12de07b

Update drawTools for PlaneForceField, RayTraceDetection, RemovePrimit…

c014d3e

…ivePerformer, EvalPointsDistance, EvalSurfaceDistance

UPDATE: Use DrawTools...

a6d6fe4

Update drawTools in SofaUserInteraction SofaMiscMapping SofaMiscForce…

4b0a72e

…Field SofaValidation

UPDATE: Use DrawTools...

998a44e

UPDATE: Use DrawTools...

153e41a

UPDATE: Use DrawTools...

a938a92

UPDATE: Use DrawTools...

7ce2120

Implementation of new classes in drawTool

294becc

Update with drawTools in SofaGeneralObjectInteraction, SofaGeneralRig…

91e54a7

…id, SofaGeneralSimpleFem, SofaGeneralVisual, SofaMiscFem

UPDATE: Use DrawTools...

84607bb

UPDATE: Use DrawTools...

d425fa1

UPDATE: Use DrawTools...

8e2ae17

UPDATE: Use DrawTools...

7e7f55b

UPDATE: Use DrawTools...

09abc82

DrawTool: use only std::vector for draw3dText (instead of helper:::ve…

fcf55ab

…ctor)

FIX compilation and update drawTools in SPHFluid

b030405

hugtalbot added enhancement About a possible enhancement pr: status to review To notify reviewers to review this pull-request STC#5 labels Jun 28, 2018

hugtalbot changed the title ~~Use drawtool everywhere~~ [all] Use drawtool everywhere Jun 28, 2018

damienmarchal reviewed Jul 4, 2018

View reviewed changes

tgaugry reviewed Jul 4, 2018

View reviewed changes

sofa-framework deleted a comment from guparan Jul 10, 2018

guparan added pr: status wip Development in the pull-request is still in progress and removed pr: status to review To notify reviewers to review this pull-request labels Jul 11, 2018

Minor changes further to review

ae0c2a6

hugtalbot added pr: status to review To notify reviewers to review this pull-request and removed pr: status wip Development in the pull-request is still in progress labels Jul 13, 2018

Merge remote-tracking branch 'sofa-framework/master' into use_drawtoo…

7067e4b

…l_everywhere # Conflicts: # modules/SofaMiscForceField/MeshMatrixMass.inl

guparan added pr: status wip Development in the pull-request is still in progress and removed pr: status to review To notify reviewers to review this pull-request labels Jul 25, 2018

hugtalbot added 2 commits July 31, 2018 16:31

Merge remote-tracking branch 'upstream' into use_drawtool_everywhere

d2fd7d9

Fix test failing PlaneForceField due to removed data

c6b2fef

Data to show the plane is reactivated Names are updated in showPlane and showPlaneScale

FIX test due to change of name color into planeColor

2686575

hugtalbot added pr: status ready Approved a pull-request, ready to be squashed and removed pr: status wip Development in the pull-request is still in progress labels Aug 2, 2018

damienmarchal merged commit 099888f into sofa-framework:master Aug 3, 2018

guparan added this to the v18.12 milestone Oct 26, 2018

fredroy deleted the use_drawtool_everywhere branch July 2, 2020 12:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[all] Use drawtool everywhere #704

[all] Use drawtool everywhere #704

hugtalbot commented Jun 28, 2018 •

edited

Loading

damienmarchal commented Jul 4, 2018

damienmarchal commented Jul 4, 2018

damienmarchal Jul 4, 2018

hugtalbot Jul 13, 2018

damienmarchal Jul 4, 2018

hugtalbot Jul 13, 2018

damienmarchal Jul 13, 2018

damienmarchal Jul 4, 2018

hugtalbot Jul 13, 2018

damienmarchal Jul 13, 2018

damienmarchal Jul 4, 2018

hugtalbot Jul 13, 2018

hugtalbot commented Jul 4, 2018

tgaugry Jul 4, 2018

hugtalbot Jul 13, 2018

tgaugry Jul 4, 2018

tgaugry Jul 4, 2018

hugtalbot Jul 13, 2018

guparan commented Jul 18, 2018 •

edited

Loading

[all] Use drawtool everywhere #704

[all] Use drawtool everywhere #704

Conversation

hugtalbot commented Jun 28, 2018 • edited Loading

damienmarchal commented Jul 4, 2018

damienmarchal commented Jul 4, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hugtalbot commented Jul 4, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

guparan commented Jul 18, 2018 • edited Loading

hugtalbot commented Jun 28, 2018 •

edited

Loading

guparan commented Jul 18, 2018 •

edited

Loading