Skip to content

CoilSegment getLocalShape computes almost 5,000,000 intersections #105

@samreid

Description

@samreid

During phetsims/faradays-electromagnetic-lab#103, @matthew-blackman and I were investigating the pointer areas for the CoilSegmentNode in phetsims/faradays-electromagnetic-lab#111 and we observed that the pointer areas did not match the stroked shape. So we tried to enable the "scenery helper" ctrl+shift+H to see what was being hit tested. In doing this and mousing over the coil, we observed that the sim locked up. When pressing "pause" in the dev tools, we saw that the intersection test was detecting nearly 5,000,000 intersections.

image

Here is the patch we used for testing that simplified the views to just focus on the CoilSegmentNode:

Details
Subject: [PATCH] Rename angleStabilizerProperty => angleStabilityProperty and related names, see https://github.com/phetsims/projectile-data-lab/issues/252
---
Index: js/common/view/PickupCoilNode.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/common/view/PickupCoilNode.ts b/js/common/view/PickupCoilNode.ts
--- a/js/common/view/PickupCoilNode.ts	(revision 038b14a29f5e7916bb29f76d43ca9f2b98529662)
+++ b/js/common/view/PickupCoilNode.ts	(date 1710944494414)
@@ -66,7 +66,7 @@
     } );
 
     // This Node's children are the foreground elements only.
-    options.children = [ coilNode, samplePointsNode, lightBulbNode, voltmeterNode ];
+    options.children = [ coilNode ];
 
     super( pickupCoil, options );
 
Index: js/common/view/ElectronsNode.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/common/view/ElectronsNode.ts b/js/common/view/ElectronsNode.ts
--- a/js/common/view/ElectronsNode.ts	(revision 038b14a29f5e7916bb29f76d43ca9f2b98529662)
+++ b/js/common/view/ElectronsNode.ts	(date 1710943813492)
@@ -49,7 +49,8 @@
       visibleProperty: coil.electronsVisibleProperty,
       sprites: [ sprite ], // the set of Sprites used to render this Node, must be set at instantiation
       spriteInstances: spriteInstances, // the set of SpriteInstances, one per compass needle in the grid
-      hitTestSprites: false
+      hitTestSprites: false,
+      pickable: false
     } );
 
     this.sprite = sprite;
Index: js/transformer/view/TransformerNode.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/transformer/view/TransformerNode.ts b/js/transformer/view/TransformerNode.ts
--- a/js/transformer/view/TransformerNode.ts	(revision 038b14a29f5e7916bb29f76d43ca9f2b98529662)
+++ b/js/transformer/view/TransformerNode.ts	(date 1710944429631)
@@ -33,7 +33,7 @@
     } );
 
     super( {
-      children: [ electromagnetNode, pickupCoilNode ],
+      children: [ pickupCoilNode ],
       tandem: tandem,
       phetioFeatured: true, // ... so that featured linked element will appear in 'Featured' tree.
       phetioVisiblePropertyInstrumented: false
Index: js/transformer/view/TransformerScreenView.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/transformer/view/TransformerScreenView.ts b/js/transformer/view/TransformerScreenView.ts
--- a/js/transformer/view/TransformerScreenView.ts	(revision 038b14a29f5e7916bb29f76d43ca9f2b98529662)
+++ b/js/transformer/view/TransformerScreenView.ts	(date 1710944429587)
@@ -75,18 +75,18 @@
     // Rendering order, from back to front.
     const screenViewRootNode = new Node( {
       children: [
-        transformerNode.pickupCoilNode.backgroundNode,
-        transformerNode.electromagnetNode.backgroundNode,
-        this.fieldNode,
-        pickupCoilAxisNode,
-        this.compassNode, // behind transformerNode, see https://github.com/phetsims/faradays-electromagnetic-lab/issues/10#issuecomment-1911160748
+        // transformerNode.pickupCoilNode.backgroundNode,
+        // transformerNode.electromagnetNode.backgroundNode,
+        // this.fieldNode,
+        // pickupCoilAxisNode,
+        // this.compassNode, // behind transformerNode, see https://github.com/phetsims/faradays-electromagnetic-lab/issues/10#issuecomment-1911160748
         transformerNode,
-        panels,
-        this.fieldMeterNode,
-        timeControlNode,
-        this.resetAllButton,
-        developerAccordionBox,
-        pickupCoilDebuggerPanel
+        // panels,
+        // this.fieldMeterNode,
+        // timeControlNode,
+        // this.resetAllButton,
+        // developerAccordionBox,
+        // pickupCoilDebuggerPanel
       ]
     } );
     this.addChild( screenViewRootNode );

We don't know whether this problem is isolated to the scenery helper, or if it would trigger in calling getLocalShape on the CoilSegmentNode directly. But ideally the scenery helper could be use to help with issues like this (it is working properly in other simulations such as Kepler's Laws).

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions