Visualization - Selection does not work for simple shape

Branch [archived branch] has been created by Commenter 1.

[revision removed]


Detailed log of new commits:

Author: Commenter 1
Date: Fri Jul 19 00:16:11 2024 +0100

    0033664: Visualization - Selection does not work for simple shape
    
    Fixed direction calculation for Select3D_SensitiveCylinder created from Geom_CylindricalSurface

Please review CR33664

Tests: http://jenkins-test-10.nnov.opencascade.com/view/CR33664-master-Commenter 1/view/ALL/

It is not accepted to have std::vector<gp_Pnt>. There are 2 possible way: Using Ncollection_List/Sequence.
Or using vector with NCollection_Allocator as a second template argument.
Commenter 4

Additionally looks like you need to have only 2 elements in that vector. That is very not economic :)
Please reorganize solution to not occupate memory.
Commenter 4

Branch [archived branch] has been updated forcibly by Commenter 1.

[revision removed]

Looks well. Could you please combine commits together (in the same branch, force push)
Commenter 4

Branch [archived branch] has been updated by Commenter 1.

[revision removed]


Detailed log of new commits:

Author: Commenter 1
Date: Tue Jul 30 16:21:07 2024 +0100

    # remarks from Commenter 5

Branch [archived branch] has been updated forcibly by Commenter 1.

[revision removed]

Commenter 4 there will be a warning when going from 2 to 1 case. Need to add macros in that case.
Probably needs to add default case for switch (to avoid warning, but not sure).
There are not breaks, in that case need to add macros to go to next case.

+    case 2:
+    {
+      if (!aPos[0].IsEqual (aPos[1], Precision::Confusion()))
+      {
+        theDirection = gp_Dir(aPos[1].XYZ() - aPos[0].XYZ());
+      }
+    }
+    case 1:
+    {

switch (mySize) {
    case 1:
        // Some code
        Standard_FALLTHROUGH;
    case 2:
        // Some code
        break;
    default:
        // Some code
        break;
}

Additionally, what about change from int to size_t?

Branch [archived branch] has been updated forcibly by Commenter 1.

[revision removed]

Commenter 4 please check remarks from Martin in #33755
The issue is not fully fixed.

Branch [archived branch] has been updated by Commenter 1.

[revision removed]


Detailed log of new commits:

Author: Commenter 1
Date: Wed Jul 31 11:44:11 2024 +0100

    # Remarks from Commenter 5

Branch [archived branch] has been updated by Commenter 1.

[revision removed]


Detailed log of new commits:

Author: Commenter 1
Date: Wed Jul 31 12:39:56 2024 +0100

    # Fix cone

Branch [archived branch] has been updated by Commenter 1.

[revision removed]


Detailed log of new commits:

Author: Commenter 1
Date: Wed Jul 31 13:51:43 2024 +0100

    # Add test for cone

It is not recommended to not initiate variable. Some scenarios have no else conditions that is why value can be a garbage.

Standard_Real aRad1;
         Standard_Real aRad2;
-        if (aRad1 == 0.0)
+        Standard_Real aHeight;

Branch [archived branch] has been updated by Commenter 6.

[revision removed]


Detailed log of new commits:

Author: Vitaut Tryputsin
Date: Wed Jul 31 17:33:12 2024 +0300

    #fix cone

Found that selection doesn't work properly because of optimization algo for selection for cones and cylinders it doesn't include holes on surfaces

probably we should disable optimization and create separate ticket for it if needed

Branch [archived branch] has been updated by Commenter 1.

[revision removed]


Detailed log of new commits:

Author: Commenter 1
Date: Wed Jul 31 16:04:36 2024 +0100

    # fix cone with holes

Branch [archived branch] has been updated by Commenter 1.

[revision removed]


Detailed log of new commits:

Author: Commenter 1
Date: Wed Jul 31 16:20:55 2024 +0100

    # Check circles nb

Branch [archived branch] has been created by Commenter 1.

[revision removed]


Detailed log of new commits:

Author: Commenter 1
Date: Fri Jul 19 00:16:11 2024 +0100

    0033664: Visualization - Selection does not work for simple shape
    
    Fixed direction calculation for Select3D_SensitiveCylinder created from Geom_CylindricalSurface

Branch [archived branch] has been updated forcibly by Commenter 1.

[revision removed]

Please review CR33664_1

Tests: http://jenkins-test-10.nnov.opencascade.com/view/CR33664-master-Commenter 1/view/ALL/

looks ok to me

Dear Commenter 1. The solution is not optimal for memory.
Sequence - bidirectional list, List - forward list. (You solution occupate more x2 memory based on incorrect container).
But in our case we don't need any special container. We fully don't need to use dynamic memory(it is very not optimal memory for fast operations).
I recommend to you 2 option:
 - Create a special internal structure to keep 3 field: gp_Pnt first, gp_Pnt Last and size.
 - Extend signature of method with new 3 out parameters and return nothing.
As for me first option looks better. Everything inside single source file and you are free to create a middle structure to improve your code.
As for a speed up for salome - no needs, they already create their own workaround.
(if you will create a struct, please put it into anonymous namespace together with internal functions in the top of the source file)

Branch [archived branch] has been updated forcibly by Commenter 1.

[revision removed]

Branch [archived branch] has been updated forcibly by Commenter 1.

[revision removed]

NCollection_Array1 is better. But still use dynamic memory and keeps any circles. But in your case you never touch elements in the middle, only first or last.

PR: https://github.com/Open-Cascade-SAS/OCCT/pull/182