Skip to content

Add pick layer to SoPickStyle for ordered on-top picking - #24

Open
Lgt2x wants to merge 1 commit into
FreeCAD:freecad-masterfrom
Lgt2x:add-pick-layer
Open

Add pick layer to SoPickStyle for ordered on-top picking#24
Lgt2x wants to merge 1 commit into
FreeCAD:freecad-masterfrom
Lgt2x:add-pick-layer

Conversation

@Lgt2x

@Lgt2x Lgt2x commented Aug 8, 2026

Copy link
Copy Markdown
Member

Downstream backport of coin3d#633 by @kadet1090

Original description:

This will let picked points be ordered across the scene graph independent of depth. SoPickStyle gains a `layer` field (default 0), carried through traversal by the new SoPickLayerElement. SoRayPickAction sorts picked points by layer (descending) and then by distance (ascending), so a higher layer always picks in front while true depth order is preserved within a layer.

The existing *_ON_TOP styles are unchanged: they ignore the layer and still sort frontmost. This replaces the previous behaviour where on-top picks collapsed their distance to 0 as the only way to bias picking to the front.

This will let picked points be ordered across the scene graph independent
of depth. SoPickStyle gains a `layer` field (default 0), carried through
traversal by the new SoPickLayerElement. SoRayPickAction sorts picked
points by layer (descending) and then by distance (ascending), so a higher
layer always picks in front while true depth order is preserved within a
layer.

The existing *_ON_TOP styles are unchanged: they ignore the layer and still
sort frontmost. This replaces the previous behaviour where on-top picks
collapsed their distance to 0 as the only way to bias picking to the front.
@kadet1090

Copy link
Copy Markdown
Member

@tritao mind taking a look and potentially merging? I'd love to get rid of a bug that this enables me to fix.

@tritao

tritao commented Aug 12, 2026

Copy link
Copy Markdown
Collaborator

@Lgt2x Can you fix CI:

diff --git a/include/Inventor/elements/SoPickLayerElement.h b/include/Inventor/elements/SoPickLayerElement.h
--- a/include/Inventor/elements/SoPickLayerElement.h
+++ b/include/Inventor/elements/SoPickLayerElement.h
@@ -45,7 +45,7 @@ protected:
   virtual ~SoPickLayerElement();

 public:
-  virtual void init(SoState * state);
+  void init(SoState * state) override;

   static void set(SoState * const state, SoNode * const node,
                   const int32_t layer);

@tritao

tritao commented Aug 12, 2026

Copy link
Copy Markdown
Collaborator

Another optional feature that we could add to have this be able to participate in Coin override semantics:

I don't think we really need it for FreeCAD purposes, but we could consider adding it just for completeness for upstreaming (untested).

diff --git a/include/Inventor/elements/SoOverrideElement.h b/include/Inventor/elements/SoOverrideElement.h
--- a/include/Inventor/elements/SoOverrideElement.h
+++ b/include/Inventor/elements/SoOverrideElement.h
@@ -68,7 +68,8 @@ public:
     TRANSPARENCY_TYPE = 0x00200000,
     NORMAL_VECTOR     = 0x00400000,
-    NORMAL_BINDING    = 0x00800000
+    NORMAL_BINDING    = 0x00800000,
+    PICK_LAYER        = 0x01000000
   };

   void init(SoState * state) override;
@@ -94,6 +95,7 @@ public:
   static SbBool getLineWidthOverride(SoState * const state);
   static SbBool getMaterialBindingOverride(SoState * const state);
   static SbBool getPickStyleOverride(SoState * const state);
+  static SbBool getPickLayerOverride(SoState * const state);
   static SbBool getPointSizeOverride(SoState * const state);
   static SbBool getPolygonOffsetOverride(SoState * const state);
   static SbBool getShapeHintsOverride(SoState * const state);
@@ -137,6 +139,9 @@ public:
   static void setPickStyleOverride(SoState * const state,
                                    SoNode * const node,
                                    const SbBool override);
+  static void setPickLayerOverride(SoState * const state,
+                                   SoNode * const node,
+                                   const SbBool override);
   static void setPointSizeOverride(SoState * const state,
                                    SoNode * const node,
                                    const SbBool override);

diff --git a/src/elements/SoOverrideElement.cpp b/src/elements/SoOverrideElement.cpp
--- a/src/elements/SoOverrideElement.cpp
+++ b/src/elements/SoOverrideElement.cpp
@@ -XXX,6 +XXX,14 @@ SoOverrideElement::getPickStyleOverride(SoState * const state)
   SO_GET_OVERRIDE(PICK_STYLE);
 }

+/*!
+  FIXME: write doc.
+*/
+SbBool
+SoOverrideElement::getPickLayerOverride(SoState * const state)
+{
+  SO_GET_OVERRIDE(PICK_LAYER);
+}
+
 /*
   ...
 */

@@ -XXX,6 +XXX,15 @@ SoOverrideElement::setPickStyleOverride(SoState * const state,
   SO_SET_OVERRIDE(PICK_STYLE);
 }

+/*!
+  FIXME: write doc.
+*/
+void
+SoOverrideElement::setPickLayerOverride(SoState * const state,
+                                        SoNode * const /* node */,
+                                        const SbBool override)
+{
+  SO_SET_OVERRIDE(PICK_LAYER);
+}
+
 /*
   ...
 */

diff --git a/src/nodes/SoPickStyle.cpp b/src/nodes/SoPickStyle.cpp
--- a/src/nodes/SoPickStyle.cpp
+++ b/src/nodes/SoPickStyle.cpp
@@ -XXX,10 +XXX,15 @@ SoPickStyle::doAction(SoAction * action)
       SoOverrideElement::setPickStyleOverride(action->getState(), this, TRUE);
     }
   }

-  if (!this->layer.isIgnored()) {
+  if (!this->layer.isIgnored()
+      && !SoOverrideElement::getPickLayerOverride(action->getState())) {
     SoPickLayerElement::set(action->getState(), this,
                             this->layer.getValue());
+    if (this->isOverride()) {
+      SoOverrideElement::setPickLayerOverride(action->getState(), this, TRUE);
+    }
   }
 }

@kadet1090

Copy link
Copy Markdown
Member

I'll push them today, thanks

@Lgt2x

Lgt2x commented Aug 15, 2026

Copy link
Copy Markdown
Member Author

Feel free to push on top of this branch and propagate the fix upstream

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants