View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
0029938 | Open CASCADE | OCCT:Visualization | public | 2018-07-08 11:33 | 2019-02-09 01:22 |
Reporter | kgv | Assigned To | bugmaster | ||
Priority | normal | Severity | minor | ||
Status | closed | Resolution | fixed | ||
Product Version | 7.3.0 | ||||
Target Version | 7.4.0 | Fixed in Version | 7.4.0 | ||
Summary | 0029938: Visualization - SelectMgr_ViewerSelector::PickedPoint() should return point lying on an object | ||||
Description | SelectMgr_ViewerSelector::PickedPoint() returns a 3D point picked by selected. In normal cases, this point is an intersection between picking ray and an entity (e.g. triangle). However, in case when there is no actual intersection (e.g. detection is done with a tolerance), selection manager calculates an approximate distance within picking ray (lying on the ray) to the object, and SelectMgr_ViewerSelector::PickedPoint() actually returns a 3D point on a Ray. This is not what user usually expects - normally user expects a point lying actually on a picked object (which does not exist in this case, but can be approximated). | ||||
Steps To Reproduce | testgrid bugs vis bug29938 | ||||
Tags | No tags attached. | ||||
Test case number | bugs/vis/bug29938 | ||||
|
Branch CR29938 has been created by mnv. SHA-1: 8a9020ee88a8d30b34fdebdace3536981f9d095a Detailed log of new commits: Author: mnv Date: Wed Jul 18 18:24:41 2018 +0300 0029938: Visualization - SelectMgr_ViewerSelector::PickedPoint() should return point lying on an object Added algorithm for transformation 3D point on the ray to 3D point on a picked object. |
|
Branch CR29938_1 has been created by mnv. SHA-1: a53bdf215a1ca2e5b0945335dd7c9c5461392049 Detailed log of new commits: Author: mnv Date: Wed Jul 18 18:24:41 2018 +0300 0029938: Visualization - SelectMgr_ViewerSelector::PickedPoint() should return point lying on an object Added algorithm for transformation 3D point on the ray to 3D point on a picked object. |
|
Branch CR29938_1 has been updated forcibly by mnv. SHA-1: 569c403012f3051696f1d7a771b68259fb10a324 |
|
Branch CR29938_1 has been updated by mnv. SHA-1: eceab9382bd20f975a15c149a4b2e63ff761b456 Detailed log of new commits: Author: mnv Date: Thu Jul 19 13:38:01 2018 +0300 Added test case. |
|
Branch CR29938_1 has been updated forcibly by mnv. SHA-1: 6b304f6672e91d214bb3991cce834bcae2b422be |
|
Patch is ready for review. http://jenkins-test-11.nnov.opencascade.com:8080/view/CR29938-master-MNV/ |
|
+ gp_Pnt myObjPickedPnt; //!< User-picked selection point onto object Storing point in SelectMgr_RectangularFrustum class field breaks conception of SelectMgr_RectangularFrustum::Overlaps(), which is expected to return results through method arguments (see theDepth). I think that these methods should be actually declared as const. From my understanding, 3D point should be stored within SelectBasics_PickResult structure alongside with Depth value, so that existing code filling and using this structure should be updated accordingly. + if {$pnt != "$x $y $z"} { + puts "Error: Calculate point ($x $y $z) is not equal to target ($pnt)" } ... +} \ No newline at end of file Please do not use tabulation symbols (adjust settings in used Text Editor(s)), and put empty line at the end of file. |
|
Branch CR29938_1 has been updated by mnv. SHA-1: de85118b3945238308666d742d09f12426c1d213 Detailed log of new commits: Author: mnv Date: Mon Jul 23 11:25:11 2018 +0300 Added new field to SelectBasics_PickResult, which contained the value of the 3d point on the selected object. |
|
Branch CR29938_2 has been created by mnv. SHA-1: 547696be34a1c4aec23a9fcf610bba0beb141260 Detailed log of new commits: Author: mnv Date: Mon Jul 23 15:08:01 2018 +0300 0029938: Visualization - SelectMgr_ViewerSelector::PickedPoint() should return point lying on an object Extended SelectBasics_PickResult structure by myObjPickedPnt field, which contained the value of the 3d point on the selected object. Changed all Overlaps methods. Parameter theDepth replaced on object of the structure SelectBasics_PickResult. This approach will be able to add new fields to SelectBasics_PickResult structure without big changes in modules which contained Overlaps method. |
|
Branch CR29938_2 has been updated by mnv. SHA-1: b7c8c153bb4fd633dfe68cb8e8ec7f5bd9c83de4 Detailed log of new commits: Author: mnv Date: Mon Jul 23 17:12:12 2018 +0300 Fixed regressions |
|
Branch CR29938_2 has been updated forcibly by mnv. SHA-1: eee3c9df9f9a1ba8616fd98429e38e25f01d4e3a |
|
Branch CR29938_2 has been updated forcibly by mnv. SHA-1: 70acfdbe8c977a00fa0c8083132042821bda2332 |
|
Branch CR29938_2 has been updated forcibly by mnv. SHA-1: 24dbd4dfd936cd7cd9f0585cb29474ca26caec78 |
|
Branch CR29938_2 has been updated forcibly by mnv. SHA-1: 49d49174eb192269b7f4c877b2cf297afe50b10d |
|
Branch CR29938_2 has been updated forcibly by mnv. SHA-1: 1060526995f306aa64c142bbb27de48a03f995f3 |
|
Patch is ready for review http://jenkins-test-11.nnov.opencascade.com:8080/view/CR29938-master-MNV/ |
|
When Sensitive has several Overlap results and pick up the nearest (aMinDepth) one, 3D point should be also set according to this depth. |
|
Branch CR29938_2 has been updated by mnv. SHA-1: 395c3665d7c3bd2b080f648e7db1b02ae8f2e1d2 Detailed log of new commits: Author: mnv Date: Wed Jul 25 11:13:43 2018 +0300 Added setting 3d point according to nearest depth value. |
|
Branch CR29938_3 has been created by mnv. SHA-1: 05d3a51e8422a3a94671852cc0ee9a4ca6a25c94 Detailed log of new commits: Author: mnv Date: Mon Jul 23 15:08:01 2018 +0300 0029938: Visualization - SelectMgr_ViewerSelector::PickedPoint() should return point lying on an object Extended SelectBasics_PickResult structure by myObjPickedPnt field, which contained the value of the 3d point on the selected object. Changed all Overlaps methods. Parameter theDepth replaced on object of the structure SelectBasics_PickResult. This approach will be able to add new fields to SelectBasics_PickResult structure without big changes in modules which contained Overlaps method. |
|
Branch CR29938_3 has been updated forcibly by mnv. SHA-1: f5fae78f1db58482fdfc236515e98f2834e93266 |
|
Patch is ready for review |
|
- Standard_Real aDepthMin = RealLast(); - Standard_Real aDistToCOG = RealLast(); + //Standard_Real aMinDepth = RealLast(); Dead code should be eliminated. + const SelectBasics_PickResult& GetMinPickResult(const SelectBasics_PickResult& thePickResult) This syntax looks awkward - it would be nicer if method will be declared static taking two arguments: > const SelectBasics_PickResult& Min (const SelectBasics_PickResult& theRes1, > const SelectBasics_PickResult& theRes2); - inline Standard_Real Depth() const + Standard_Real GetDepth() const ... + Standard_Real GetDistToGeomCenter() const ... + const gp_Pnt& GetPickedPnt() const This contradicts to recommended naming style - getter should be without "Get" prefix. Please also provide missing description for new methods. //!< Depth to detected point Standard_Real myDepth; //!< Distance from 3d projection user-picked selection point to entity's geometry center Standard_Real myDistToCenter; + //!< User-picked selection point onto object + gp_Pnt myObjPickedPnt; These Doxygen comments are broken - "//!<" should be at right side of commented line, or there should be just "//!". --- a/src/SelectBasics/SelectBasics_SelectingVolumeManager.hxx +++ b/src/SelectBasics/SelectBasics_SelectingVolumeManager.hxx @@ -45,7 +46,7 @@ public: //! Returns true if selecting volume is overlapped by box theBox virtual Standard_Boolean Overlaps (const NCollection_Vec3<Standard_Real>& theBoxMin, const NCollection_Vec3<Standard_Real>& theBoxMax, - Standard_Real& theDepth) = 0; + SelectBasics_PickResult& thePickResult) = 0; Please mention these API changes in dox/dev_guides/upgrade/upgrade.md. + Standard_Real aDistToCOG = thePickResult.GetDepth() == RealLast() ? RealLast() : theMgr.DistToGeometryCenter (CenterOfGeometry()); + thePickResult.SetDistToGeomCenter (aDistToCOG); - return aDepthMin != RealLast(); + return thePickResult.GetDepth() != RealLast(); Please add appropriate method SelectBasics_PickResult::IsValid() returning if Pick Result is actually defined (Depth!=RealLast internally) and use it in all such cases. - theDepth = DBL_MAX; + thePickResult.SetDepth(RealLast()); Please add method SelectBasics_PickResult::Invalidate() instead. + SelectBasics_PickResult aPickResult1; + SelectBasics_PickResult aPickResult2; Can be > SelectBasics_PickResult aPickResult1, aPickResult2; for compactness. + if {$pnt != "$x $y $z"} { + puts "Error: Calculate point ($x $y $z) is not equal to target ($pnt)" } Two-spaces indentation. |
|
Branch CR29938_4 has been created by mnv. SHA-1: 97806d41671fbdd67dba3e24b49e49d366b7cf3e Detailed log of new commits: Author: mnv Date: Mon Jul 23 15:08:01 2018 +0300 0029938: Visualization - SelectMgr_ViewerSelector::PickedPoint() should return point lying on an object Extended SelectBasics_PickResult structure by myObjPickedPnt field, which contained the value of the 3d point on the selected object. Changed all Overlaps methods. Parameter theDepth replaced on object of the structure SelectBasics_PickResult. This approach will be able to add new fields to SelectBasics_PickResult structure without big changes in modules which contained Overlaps method. |
|
Branch CR29938_3 has been updated by mnv. SHA-1: a7b5ce490b8954e5dc359b9104a0258a711acacf Detailed log of new commits: Author: mnv Date: Wed Jul 25 18:25:12 2018 +0300 Changes according to review comments. |
|
Branch CR29938_4 has been updated forcibly by mnv. SHA-1: 2097c5ed9ed35f428ae6709ff484db170d59995d |
|
Patch is ready for review. http://jenkins-test-11.nnov.opencascade.com:8080/view/CR29938-master-MNV/ |
|
Branch CR29938_5 has been created by kgv. SHA-1: ff78f794d0c9818498fba4edbb09abb399291cd1 Detailed log of new commits: Author: mnv Date: Mon Jul 23 15:08:01 2018 +0300 0029938: Visualization - SelectMgr_ViewerSelector::PickedPoint() should return point lying on an object Extended SelectBasics_PickResult structure by myObjPickedPnt field, which contained the value of the 3d point on the selected object. Changed all Overlaps methods. Parameter theDepth replaced on object of the structure SelectBasics_PickResult. This approach will be able to add new fields to SelectBasics_PickResult structure without big changes in modules which contained Overlaps method. |
|
SelectMgr_RectangularFrustum::segmentPlaneIntersection() does not clamp 3D point to segment ends. |
|
Branch CR29938_6 has been created by mnv. SHA-1: 2870bc77c0d83a63cb24c12427bc89361f020491 Detailed log of new commits: Author: mnv Date: Mon Jul 23 15:08:01 2018 +0300 0029938: Visualization - SelectMgr_ViewerSelector::PickedPoint() should return point lying on an object Extended SelectBasics_PickResult structure by myObjPickedPnt field, which contained the value of the 3d point on the selected object. Changed all Overlaps methods. Parameter theDepth replaced on object of the structure SelectBasics_PickResult. This approach will be able to add new fields to SelectBasics_PickResult structure without big changes in modules which contained Overlaps method. |
|
Branch CR29938_5 has been updated by mnv. SHA-1: fd871a29dda66a3b05ba684ed639f1e1799709ef Detailed log of new commits: Author: mnv Date: Thu Aug 2 10:54:21 2018 +0300 Added processing for failed boundary cases |
|
Branch CR29938_6 has been updated forcibly by mnv. SHA-1: 2b49498b0b4b074a7f66bc0e9f7e3d73a55bba8c |
|
Please test patch. |
|
Branch CR29938_6 has been updated forcibly by mnv. SHA-1: 27e057549420bed120ed73652b60fc3de2cf7e8d |
|
Combination - OCCT branch : CR29938_4 SHA - 2097c5ed9ed35f428ae6709ff484db170d59995d Products branch : CR29938_3 SHA - bf6e4d086107b4f0166860e1a64b143182f72420 was compiled on Linux, MacOS and Windows platforms and tested in optimize mode. Number of compiler warnings: No new/fixed warnings Regressions/Differences/Improvements: No regressions/differences CPU differences: Debian70-64: OCCT Total CPU difference: 16988.440000000042 / 17220.20999999982 [-1.35%] Products Total CPU difference: 7433.420000000025 / 7395.360000000046 [+0.51%] Windows-64-VC10: OCCT Total CPU difference: 16885.81344169862 / 17014.420666098493 [-0.76%] Products Total CPU difference: 8212.345042899891 / 8218.070279599868 [-0.07%] Image differences : No differences that require special attention Memory differences : No differences that require special attention |
|
Branch CR29938_6 has been deleted by inv. SHA-1: 27e057549420bed120ed73652b60fc3de2cf7e8d |
|
Branch CR29938_5 has been deleted by inv. SHA-1: fd871a29dda66a3b05ba684ed639f1e1799709ef |
|
Branch CR29938_4 has been deleted by inv. SHA-1: 2097c5ed9ed35f428ae6709ff484db170d59995d |
|
Branch CR29938_3 has been deleted by inv. SHA-1: a7b5ce490b8954e5dc359b9104a0258a711acacf |
|
Branch CR29938_2 has been deleted by inv. SHA-1: 395c3665d7c3bd2b080f648e7db1b02ae8f2e1d2 |
|
Branch CR29938_1 has been deleted by inv. SHA-1: de85118b3945238308666d742d09f12426c1d213 |
|
Branch CR29938 has been deleted by inv. SHA-1: 8a9020ee88a8d30b34fdebdace3536981f9d095a |
occt: master 17017555 2018-07-23 12:08:01
Committer: bugmaster Details Diff |
0029938: Visualization - SelectMgr_ViewerSelector::PickedPoint() should return point lying on an object Extended SelectBasics_PickResult structure by myObjPickedPnt field, which contained the value of the 3d point on the selected object. Changed all Overlaps methods. Parameter theDepth replaced on object of the structure SelectBasics_PickResult. This approach will be able to add new fields to SelectBasics_PickResult structure without big changes in modules which contained Overlaps method. |
Affected Issues 0029938 |
|
mod - dox/dev_guides/upgrade/upgrade.md | Diff File | ||
mod - src/MeshVS/MeshVS_CommonSensitiveEntity.cxx | Diff File | ||
mod - src/MeshVS/MeshVS_CommonSensitiveEntity.hxx | Diff File | ||
mod - src/MeshVS/MeshVS_SensitivePolyhedron.cxx | Diff File | ||
mod - src/MeshVS/MeshVS_SensitiveQuad.cxx | Diff File | ||
mod - src/Select3D/Select3D_InteriorSensitivePointSet.cxx | Diff File | ||
mod - src/Select3D/Select3D_InteriorSensitivePointSet.hxx | Diff File | ||
mod - src/Select3D/Select3D_SensitiveBox.cxx | Diff File | ||
mod - src/Select3D/Select3D_SensitiveCircle.cxx | Diff File | ||
mod - src/Select3D/Select3D_SensitiveGroup.cxx | Diff File | ||
mod - src/Select3D/Select3D_SensitiveGroup.hxx | Diff File | ||
mod - src/Select3D/Select3D_SensitivePoint.cxx | Diff File | ||
mod - src/Select3D/Select3D_SensitivePoly.cxx | Diff File | ||
mod - src/Select3D/Select3D_SensitivePoly.hxx | Diff File | ||
mod - src/Select3D/Select3D_SensitivePrimitiveArray.cxx | Diff File | ||
mod - src/Select3D/Select3D_SensitivePrimitiveArray.hxx | Diff File | ||
mod - src/Select3D/Select3D_SensitiveSegment.cxx | Diff File | ||
mod - src/Select3D/Select3D_SensitiveSet.cxx | Diff File | ||
mod - src/Select3D/Select3D_SensitiveSet.hxx | Diff File | ||
mod - src/Select3D/Select3D_SensitiveTriangle.cxx | Diff File | ||
mod - src/Select3D/Select3D_SensitiveTriangulation.cxx | Diff File | ||
mod - src/Select3D/Select3D_SensitiveTriangulation.hxx | Diff File | ||
mod - src/Select3D/Select3D_SensitiveWire.cxx | Diff File | ||
mod - src/Select3D/Select3D_SensitiveWire.hxx | Diff File | ||
mod - src/SelectBasics/SelectBasics_PickResult.hxx | Diff File | ||
mod - src/SelectBasics/SelectBasics_SelectingVolumeManager.hxx | Diff File | ||
mod - src/SelectMgr/SelectMgr_BaseFrustum.cxx | Diff File | ||
mod - src/SelectMgr/SelectMgr_BaseFrustum.hxx | Diff File | ||
mod - src/SelectMgr/SelectMgr_RectangularFrustum.cxx | Diff File | ||
mod - src/SelectMgr/SelectMgr_RectangularFrustum.hxx | Diff File | ||
mod - src/SelectMgr/SelectMgr_SelectingVolumeManager.cxx | Diff File | ||
mod - src/SelectMgr/SelectMgr_SelectingVolumeManager.hxx | Diff File | ||
mod - src/SelectMgr/SelectMgr_TriangularFrustum.cxx | Diff File | ||
mod - src/SelectMgr/SelectMgr_TriangularFrustum.hxx | Diff File | ||
mod - src/SelectMgr/SelectMgr_TriangularFrustumSet.cxx | Diff File | ||
mod - src/SelectMgr/SelectMgr_TriangularFrustumSet.hxx | Diff File | ||
mod - src/SelectMgr/SelectMgr_ViewerSelector.cxx | Diff File | ||
mod - src/SelectMgr/SelectMgr_ViewerSelector.hxx | Diff File | ||
add - tests/bugs/vis/bug29938 | Diff File |
Date Modified | Username | Field | Change |
---|---|---|---|
2018-07-08 11:33 | kgv | New Issue | |
2018-07-08 11:33 | kgv | Assigned To | => kgv |
2018-07-16 17:13 | kgv | Assigned To | kgv => mnv |
2018-07-16 17:13 | kgv | Status | new => assigned |
2018-07-18 18:31 | git | Note Added: 0077791 | |
2018-07-19 09:35 | git | Note Added: 0077802 | |
2018-07-19 09:38 | git | Note Added: 0077803 | |
2018-07-19 13:39 | git | Note Added: 0077815 | |
2018-07-19 13:41 | git | Note Added: 0077816 | |
2018-07-19 15:01 |
|
Note Added: 0077822 | |
2018-07-19 15:01 |
|
Assigned To | mnv => kgv |
2018-07-19 15:01 |
|
Status | assigned => resolved |
2018-07-19 15:01 |
|
Steps to Reproduce Updated | |
2018-07-20 16:27 | kgv | Note Added: 0077858 | |
2018-07-20 16:27 | kgv | Assigned To | kgv => mnv |
2018-07-20 16:27 | kgv | Status | resolved => assigned |
2018-07-20 16:27 | kgv | Note Edited: 0077858 | |
2018-07-20 16:28 | kgv | Note Edited: 0077858 | |
2018-07-20 16:30 | kgv | Note Edited: 0077858 | |
2018-07-23 11:28 | git | Note Added: 0077919 | |
2018-07-23 15:19 | git | Note Added: 0077926 | |
2018-07-23 17:13 | git | Note Added: 0077933 | |
2018-07-23 17:40 | git | Note Added: 0077936 | |
2018-07-24 09:52 | git | Note Added: 0077955 | |
2018-07-24 12:12 | git | Note Added: 0077957 | |
2018-07-24 14:49 | git | Note Added: 0077962 | |
2018-07-24 15:07 | git | Note Added: 0077963 | |
2018-07-25 08:39 |
|
Note Added: 0077983 | |
2018-07-25 08:39 |
|
Assigned To | mnv => kgv |
2018-07-25 08:39 |
|
Status | assigned => resolved |
2018-07-25 09:54 | kgv | Note Added: 0077987 | |
2018-07-25 09:54 | kgv | Assigned To | kgv => mnv |
2018-07-25 09:54 | kgv | Status | resolved => assigned |
2018-07-25 11:22 | git | Note Added: 0077988 | |
2018-07-25 11:23 | git | Note Added: 0077989 | |
2018-07-25 13:56 | git | Note Added: 0077992 | |
2018-07-25 13:58 |
|
Note Added: 0077993 | |
2018-07-25 13:58 |
|
Assigned To | mnv => kgv |
2018-07-25 13:58 |
|
Status | assigned => resolved |
2018-07-25 16:59 | kgv | Note Added: 0078000 | |
2018-07-25 17:00 | kgv | Assigned To | kgv => mnv |
2018-07-25 17:00 | kgv | Status | resolved => assigned |
2018-07-25 18:29 | git | Note Added: 0078008 | |
2018-07-25 18:30 | git | Note Added: 0078009 | |
2018-07-26 14:12 | git | Note Added: 0078032 | |
2018-07-26 15:52 |
|
Note Added: 0078041 | |
2018-07-26 15:52 |
|
Assigned To | mnv => kgv |
2018-07-26 15:52 |
|
Status | assigned => resolved |
2018-08-01 17:51 | git | Note Added: 0078278 | |
2018-08-01 18:19 | kgv | Note Added: 0078286 | |
2018-08-01 18:19 | kgv | Assigned To | kgv => mnv |
2018-08-01 18:19 | kgv | Status | resolved => assigned |
2018-08-02 11:02 | git | Note Added: 0078307 | |
2018-08-02 11:03 | git | Note Added: 0078308 | |
2018-08-02 11:08 |
|
Assigned To | mnv => kgv |
2018-08-02 11:08 |
|
Status | assigned => resolved |
2018-08-02 11:40 | git | Note Added: 0078310 | |
2018-08-02 12:06 | kgv | Note Added: 0078313 | |
2018-08-02 12:06 | kgv | Assigned To | kgv => bugmaster |
2018-08-02 12:06 | kgv | Status | resolved => reviewed |
2018-08-02 12:09 | git | Note Added: 0078314 | |
2018-08-02 18:20 | bugmaster | Test case number | => bugs/vis/bug29938 |
2018-08-02 18:23 | bugmaster | Note Added: 0078334 | |
2018-08-02 18:23 | bugmaster | Status | reviewed => tested |
2018-08-04 16:51 | bugmaster | Changeset attached | => occt master 17017555 |
2018-08-04 16:51 | bugmaster | Status | tested => verified |
2018-08-04 16:51 | bugmaster | Resolution | open => fixed |
2018-08-04 17:09 | git | Note Added: 0078378 | |
2018-08-04 17:09 | git | Note Added: 0078379 | |
2018-08-04 17:09 | git | Note Added: 0078392 | |
2018-08-04 17:09 | git | Note Added: 0078394 | |
2018-08-04 17:09 | git | Note Added: 0078395 | |
2018-08-04 17:09 | git | Note Added: 0078396 | |
2018-08-04 17:09 | git | Note Added: 0078397 | |
2019-02-09 01:22 | kgv | Relationship added | parent of 0030486 |