Visualization, SelectMgr_ViewerSelector - Graphic3d_TMF_2d persistence sorting issues

Branch [archived branch] has been created by Author.

[revision removed]


Detailed log of new commits:

Author: Author
Date: Thu Feb 7 15:31:50 2019 +0300

    0030484: Visualization - 2d persistent: order of detection doesn't coincide with order of objects creation

Zeroing MinDist for 2D objects is not really helpful - it can provide desired effect only within limited usage scenarios, while in other cases result will become worse.

Consider extending your test case with removal/redisplay of first box:

pload MODELING VISUALIZATION
vclear
vinit View1
vselprops dynHighlight -dispMode 1 -color WHITE

box b1  75 -250 0 200 200 200
box b2 175 -300 0 200 200 200

vdisplay b1 -2d topLeft -top -dispmode 1
vsetmaterial b1  PLASTIC
vsetcolor    b1 DEEPPINK2

vdisplay b2 -2d topLeft -top -dispmode 1
vsetmaterial b2 PLASTIC
vsetcolor    b2 PALEGREEN2

vremove b1
vdisplay b1 -2d topLeft -top -dispmode 1
vsetmaterial b1  PLASTIC
vsetcolor    b1 DEEPPINK2

The result will be that b1 will now visually overlap b2, but highlighting (with patch applied) will tend to highlight b2 behind b1.

The root cause of the issue is an attempt to display two overlapping 2D objects with the same depth, which in general case produce undefined behavior for both - selection and visualization.

OCCT visualization does NOT guarantee rendering order for objects within a single ZLayer and having the same rendering priority - the actual rendering order following the order of displaying objects in AIS_InteractiveContext is a side effect of usage of collections with well-defined ordering in memory like to achieve reproducibility of visual results, and easily messed up after objects movements in context (like selecting/deselecting objects leading to movements from one Priority to another).

So that in this particular use case it is really desired displaying objects with extra information (different display Priority, different ZLayer, artificially different Z) for producing coherent visual and selection results. Note that since Selection currently ignores Display Priority, this property still wouldn't help (although selection itself has its own independent Selection Priorities actually used for sorting).

Branch [archived branch] has been created by Participant.

[revision removed]


Detailed log of new commits:

Author: drochalo
Date: Wed Jan 17 10:36:42 2024 +0000

    0030484: Visualization, SelectMgr_ViewerSelector - Graphic3d_TMF_2d persistence sorting issues
    
    Added property to SelectableObject for selection focused on display priority.
    Modified SelectMgr_ViewerSelector CompareResults based on the focus priority property.
    Added tests for multiple types of sensitive entities.

Branch [archived branch] has been updated by Participant.

[revision removed]


Detailed log of new commits:

Author: drochalo
Date: Wed Jan 17 14:18:15 2024 +0000

    0030484: Visualization, SelectMgr_ViewerSelector - Graphic3d_TMF_2d persistence sorting issues
    
    Logic fix on SelectMgr_ViewerSelector::checkOverlap.

Branch [archived branch] has been updated forcibly by Participant.

[revision removed]

Branch [archived branch] has been updated forcibly by Participant.

[revision removed]

Dear Marina,
please review.

Tests on Jenkins shows no major issues: http://jenkins-test-10.nnov.opencascade.com/view/CR30484_1-master-drochalo/view/COMPARE/

I think we have enough information about transform persistence and object priorities and do not need additional flags (myPriorityFocus).
Please use the available information to solve the task.

Branch [archived branch] has been updated by Participant.

[revision removed]


Detailed log of new commits:

Author: drochalo
Date: Fri Jan 19 15:31:35 2024 +0000

    0030484: Visualization, SelectMgr_ViewerSelector - Graphic3d_TMF_2d persistence sorting issues
    
    Removed variable myPriorityFocus.
    Modified logic to perform selection focused on display priority only for selectable objects with 2d transform persistence.

Branch [archived branch] has been updated forcibly by Participant.

[revision removed]

Dear Marina,

changes were done to use 2d transform persistence in the logic instead of myPriorityFocus.
New tests on jenkins present no major issues http://jenkins-test-10.nnov.opencascade.com/view/CR30484_1-master-drochalo/view/COMPARE/
Please review.

if (aSelectable->Presentations().Size() > 0 && !aSelectable->TransformPersistence().IsNull())
{
  if (!aSelectable->Presentations().First().IsNull() && aSelectable->TransformPersistence()->Mode() == Graphic3d_TMF_2d)
  {
    aCriterion.IsPreferPriority = Standard_True;
    aCriterion.Priority = aSelectable->Presentations().First()->DisplayPriority();
  }
}

It seems new selection logic works only for the first presentation ignoring anything else. What happens with other ones?
The relationship between selection priority and display priority is unclear and work only with Graphic3d_TMF_2d. Selection priority is preferrable in case of selection itself rather than display priority.

Branch [archived branch] has been updated by Participant.

[revision removed]


Detailed log of new commits:

Author: drochalo
Date: Wed Jan 24 11:31:48 2024 +0000

    0030484: Visualization, SelectMgr_ViewerSelector - Graphic3d_TMF_2d persistence sorting issues
    
    Added display priority to the sorting criterion
    Modified vpriority command to change an object's selection priority.
    Set display priority to presentation with same display mode.
    Modified tests.

Branch [archived branch] has been updated forcibly by Participant.

[revision removed]

...
Standard_Real      Tolerance;        //!< tolerance used for selecting candidates
Standard_Integer   Priority;         //!< selection priority
Standard_Integer   ZLayerPosition;   //!< ZLayer rendering order index, stronger than a depth
Standard_Integer   NbOwnerMatches;   //!< overall number of entities collected for the same owner
Standard_Integer   DisplayPriority;  //!< display priority
Standard_Boolean   IsPreferPriority; //!< flag to signal comparison to be done over priority
...

It would be nice to have all priorities close to each other. Also it would be nice to have 'SelectionPriority' instead of just 'Priority' in this case.

...
Graphic3d_DisplayPriority aDisplayPriority = Graphic3d_DisplayPriority_Bottom;
if (aSelectable->TransformPersistence()->Mode() == Graphic3d_TMF_2d)
{
...

Please move 'aDisplayPriority' into the scope where it is really used.

...
aSelectable->DisplayMode();
SelectMgr_SortCriterion aCriterion;
...

Empty unused call of function.

...
if (aSelectable->DisplayMode() == aPrs->Mode())
{
...

Presentable object can not to have its own mode. In that case global parameter is used. Please look at 'HasDisplayMode' method' s documentation.

...
//=======================================================================
//function : VPriority
//purpose  : Prints or sets the display priority for an object
//=======================================================================
...

Unchanged piece of documentation.

...
if (aToSetSelection)
{
  Handle(SelectMgr_EntityOwner) anOwner = anIObj->GlobalSelOwner();
  if (!anOwner.IsNull())
  {
    anOwner->SetPriority (aPriority);
  }
  return 0;
}
...

Seems like selection priority is processed in the same way as display priority (I meand limits checks etc.). Selection and display priorities should be separated and have different flags to print and set. Maybe it worth to have separate method or command for selection priority. Please loook where other selection option is set.

...
if (Abs (Depth - theOther.Depth) <= Precision::Confusion())
{
  return MinDist < theOther.MinDist;
}



return Depth < theOther.Depth;
...

Extra double lines.

Branch [archived branch] has been updated by Participant.

[revision removed]


Detailed log of new commits:

Author: drochalo
Date: Tue Jan 30 09:56:04 2024 +0000

    0030484: Visualization, SelectMgr_ViewerSelector - Graphic3d_TMF_2d persistence sorting issues
    
    Changed test logic in checkOverlap to use Interactive Object context to set display priority.
    Code cleanup.

Branch [archived branch] has been updated forcibly by Participant.

[revision removed]

Dear Ilya,

the requested changes were made.
New tests on Jenkins show no major issues: http://jenkins-test-10.nnov.opencascade.com/view/CR30484_1-master-drochalo/view/COMPARE/
Please review.

if (SelectionPriority > theOther.SelectionPriority)
{
  return true;
}
else if (SelectionPriority != theOther.SelectionPriority)
{
  return false;
}

if (DisplayPriority > theOther.DisplayPriority)
{
  return true;
}
else if (DisplayPriority != theOther.DisplayPriority)
{
  return false;
}

It is more readable to write 'if(something != theOther.something) return something > theOtherSomething;'

Graphic3d_DisplayPriority aDisplayPriority = Graphic3d_DisplayPriority_INVALID;
if (!anObj.IsNull())
{
  Handle(Prs3d_Presentation) aPrs = anObj->Presentation();
  if (!aPrs.IsNull())
  {
    aDisplayPriority = aPrs->DisplayPriority();
  }
}
aCriterion.DisplayPriority = aDisplayPriority;

There is no needs to use 'aDisplayPriority' variable. Everything can be done directly in 'aCriterion.DisplayPriority'

Please get rid of changes in 'ViewerTest_ObjectCommands.cxx' because there are no any actual changes.

Branch [archived branch] has been updated by Participant.

[revision removed]


Detailed log of new commits:

Author: drochalo
Date: Thu Apr 4 17:00:43 2024 +0100

    0030484: Visualization, SelectMgr_ViewerSelector - Graphic3d_TMF_2d persistence sorting issues
    
    Code cleanup.

Branch [archived branch] has been updated forcibly by Participant.

[revision removed]

Dear Ilya,

the requested changes were made. I also updated the branch due to some issues on a few of the tests.
http://jenkins-test-10.nnov.opencascade.com/view/CR30484_1-master-drochalo/view/COMPARE/
Please review.

Need to do something with branch

Branch [archived branch] has been updated forcibly by Participant.

[revision removed]

Dear Commenter 1,
the commits were squashed and the branch was pushed.

Branch [archived branch] has been deleted by Participant.

[revision removed]

Branch [archived branch] has been deleted by Participant.

[revision removed]