Light doesn't always activate when magnet flips

[KatieWoe]

Test device Samsung Operating System Win 11 Browser Chrome Problem description For https://github.com/phetsims/qa/issues/1091 On the second screen, the lightbulb may not light up in conditions when it should. Specifically, if you change the magnetic field by flipping the magnet's polarity, the bulb only sometimes lights. Steps to reproduce

1. On the second screen, have the lightbulb indicator up
2. Click the flip polarity button.
3. For more dramatic results, have the magnet within the coil.

Visuals

Troubleshooting information:

!!!!! DO NOT EDIT !!!!! Name: ‪Faraday's Electromagnetic Lab‬ URL: https://phet-dev.colorado.edu/html/faradays-electromagnetic-lab/1.0.0-dev.32/phet/faradays-electromagnetic-lab_all_phet.html Version: 1.0.0-dev.32 2024-05-30 15:49:49 UTC Features missing: applicationcache, applicationcache, touch Flags: pixelRatioScaling User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/125.0.0.0 Safari/537.36 Language: en-US Window: 1536x695 Pixel Ratio: 1.25/1 WebGL: WebGL 1.0 (OpenGL ES 2.0 Chromium) GLSL: WebGL GLSL ES 1.0 (OpenGL ES GLSL ES 1.0 Chromium) Vendor: WebKit (WebKit WebGL) Vertex: attribs: 16 varying: 31 uniform: 4096 Texture: size: 8192 imageUnits: 32 (vertex: 32, combined: 64) Max viewport: 8192x8192 OES_texture_float: true Dependencies JSON: {}

[KatieWoe]

I'm not seeing this on iPad, so it may have to do with screen resolution, or some other device specific issue.

[arouinfar]

I was able to easily reproduce this on macOS 14.5 and latest Chrome.

I think there's a rhythm to it. If I can press with the right tempo, the bulb will either never light or always light.

https://github.com/phetsims/faradays-electromagnetic-lab/assets/8419308/8990fab7-ad83-4d86-9352-487d7f2bdf83

it may have to do with screen resolution

I think @KatieWoe is on to something. It was very easy to reproduce when I had the sim window split screen (taller, narrower window). When I reduced the browser height to be shorter/wider, I couldn't reproduce at all. I was also not able to reproduce when sizing the window to the layout bounds using ?dev.

Troubleshooting Info

!!!!! DO NOT EDIT !!!!! Name: ‪Faraday's Electromagnetic Lab‬ URL: https://phet-dev.colorado.edu/html/faradays-electromagnetic-lab/1.0.0-dev.32/phet/faradays-electromagnetic-lab_all_phet.html?initialScreen=2 Version: 1.0.0-dev.32 2024-05-30 15:49:49 UTC Features missing: applicationcache, applicationcache, touch Flags: pixelRatioScaling User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/125.0.0.0 Safari/537.36 Language: en-US Window: 1514x1385 Pixel Ratio: 1.7999999523162842/1 WebGL: WebGL 1.0 (OpenGL ES 2.0 Chromium) GLSL: WebGL GLSL ES 1.0 (OpenGL ES GLSL ES 1.0 Chromium) Vendor: WebKit (WebKit WebGL) Vertex: attribs: 16 varying: 30 uniform: 1024 Texture: size: 16384 imageUnits: 16 (vertex: 16, combined: 32) Max viewport: 16384x16384 OES_texture_float: true Dependencies JSON: {}

[pixelzoom]

This is the same as https://github.com/phetsims/faradays-electromagnetic-lab/issues/9, which was a legacy bug. I'm not sure if https://github.com/phetsims/faradays-electromagnetic-lab/issues/9 just wasn't fixed at the time it was closed, or if something changed to make the problem come back.

When flipping the polarity, the light bulb only stays lit for 1 time step. The problem in #9 is that we were skipping a frame, and not seeing that time step being rendered. The first attempt to address this was to reimplement ConstantDtClock as a subclass of EventTimer, which did NOT fix the problem; see https://github.com/phetsims/faradays-electromagnetic-lab/issues/9#issuecomment-1942167712. Converting electrons to scenery.Sprites seemed to resolve the problem, probably by improving performance enough that we were skipping a frame less frequently; see https://github.com/phetsims/faradays-electromagnetic-lab/issues/9#issuecomment-1965522316.

[pixelzoom]

EventTimer will make multiple calls to step if dt is large enough. To see if that's what is happening in this case, I added this to ConstantDtClock:

  public override step( dt: number ): void {
    const periodBeforeNextEvent = 1 / ConstantDtClock.FRAMES_PER_SECOND;
    const steps = dt / periodBeforeNextEvent;
    if ( steps >= 2 ) {
      console.log( `dt=${dt} steps=${steps}` );
    }
    super.step( dt );
  }

... and this to LightBulb:

this._brightnessProperty.link( brightness => console.log( `brightness=${brightness} time=${Date.now()}` ) );

In the console, I see the light bulb's brightness go from 0 -> 1 -> 0, for example:

brightness=0 time=1718640618176
brightness=1 time=1718640618811
brightness=0 time=1718640618843

I do not see ConstantDtClock reporting multiple steps. This was my concern, since ConstantDtClock extends EventTimer.

[pixelzoom]

With concerns about getting multiple steps from EventTimer, I resurrected (and improved) the old implementation of ConstantDtClock, see below. This did not resolve the problem.

ConstantDtClockOld.ts

```ts // Copyright 2024, University of Colorado Boulder /** * ConstantDtClockOld implements a clock that steps at a constant rate, with a constant dt. * * In the Java version of this sim, we used a clock that steps 25 times per second, with constant dt = 1. * See FaradayModule.java: new SwingClock( 1000 / 25, 1 ) * Because so much of the code ported from Java relies on this, we implement something similar here. * * @author Chris Malley (PixelZoom, Inc.) */ import Emitter from '../../../../axon/js/Emitter.js'; import faradaysElectromagneticLab from '../../faradaysElectromagneticLab.js'; import NumberIO from '../../../../tandem/js/types/NumberIO.js'; export default class ConstantDtClockOld { // Constant framerate, inherited from the Java implementation. Increasing this value makes the sim run faster, // reducing it makes the sim run slower. Changing this value may have unintended/unknown consequences, so proceed // with caution. public static readonly FRAMES_PER_SECOND = 25; private static readonly SECONDS_PER_FRAME = 1 / ConstantDtClockOld.FRAMES_PER_SECOND; // Constant dt, inherited from the Java implementation. Each time the clock steps (about every 40 ms at 25 fps), // we'll notify listeners with this dt value. So while step is usually called with a value in seconds, this value // is not in seconds. Changing this value may have unintended/unknown consequences, so proceed with caution. public static readonly DT = 1; // Time since the clock last notified listeners. private elapsedTime: number; // For notifying listeners when the clock steps. The single parameter to emit is the constant dt value. private readonly emitter: Emitter<[ number ]>; public constructor() { this.elapsedTime = 0; this.emitter = new Emitter( { parameters: [ { name: 'dt', phetioType: NumberIO } ] } ); } /** * Adds a listener that will be notified when the clock ticks. * @param listener */ public addListener( listener: ( dt: number ) => void ): void { this.emitter.addListener( listener ); } /** * Accumulates dt until it reaches the constant dt, then emits once. * Unlike EventTimer, this will not result in more than 1 step if dt is large. * * @param dt - time change, in seconds */ public step( dt: number ): void { this.elapsedTime += dt; // When we've accumulated enough time to step ... if ( this.elapsedTime >= ConstantDtClockOld.SECONDS_PER_FRAME ) { // Advance one step. this.stepOnce(); // Apply the remainder to the next step. this.elapsedTime %= ConstantDtClockOld.SECONDS_PER_FRAME; } } /** * Advances the model by one step, with constant dt. In addition to being called by the step method, this is called * explicitly by the step button in time controls. */ public stepOnce(): void { this.emitter.emit( ConstantDtClockOld.DT ); } } faradaysElectromagneticLab.register( 'ConstantDtClockOld', ConstantDtClockOld ); ```

[pixelzoom]

I've spent ~3 hours on this and I'm no closer to resolving. I'll need help, preferrably from @jonathanolson, who I've contacted on Slack.

[samreid]

In several experiments, I'm seeing a "dropped frame" correlate with the unrendered brightness lines:

UPDATE: I do not see a reliable way to ask the browser if a frame was dropped. My experiments by logging the dts from joist showed that sometimes 0.023s dropped a frame and sometimes 0.023s did not drop a frame. So maybe a reasonable workaround would be to at least render the nonzero brightness for N frames, where N is greater than 1.

My exploratory patch for my reference:

```diff Subject: [PATCH] Move massValuesInitiallyDisplayed to a named option, see https://github.com/phetsims/density-buoyancy-common/issues/186 --- Index: faradays-electromagnetic-lab/js/common/model/LightBulb.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/faradays-electromagnetic-lab/js/common/model/LightBulb.ts b/faradays-electromagnetic-lab/js/common/model/LightBulb.ts --- a/faradays-electromagnetic-lab/js/common/model/LightBulb.ts (revision c27223ea366a95dfee9cb2f5a503f823aac1fb29) +++ b/faradays-electromagnetic-lab/js/common/model/LightBulb.ts (date 1718902956715) @@ -59,6 +59,10 @@ } ); this.brightnessProperty = this._brightnessProperty; + this._brightnessProperty.link( brightness => { + console.log( Date.now(), 'brightness', brightness ); + } ); + normalizedCurrentProperty.link( ( normalizedCurrent, previousNormalizedCurrent ) => { let brightness = 0; if ( previousNormalizedCurrent !== null && Index: joist/js/Sim.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/joist/js/Sim.ts b/joist/js/Sim.ts --- a/joist/js/Sim.ts (revision 5d3f17d2c6d9220ea0b5bc54bf934582fe77b24e) +++ b/joist/js/Sim.ts (date 1718904377711) @@ -1022,7 +1022,9 @@ // The animation frame timer runs every frame const currentTime = Date.now(); - animationFrameTimer.emit( getDT( this.lastAnimationFrameTime, currentTime ) ); + const dt = getDT( this.lastAnimationFrameTime, currentTime ); + console.log( dt ); + animationFrameTimer.emit( dt ); this.lastAnimationFrameTime = currentTime; if ( Tandem.PHET_IO_ENABLED ) { Index: scenery-phet/js/LightBulbNode.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/scenery-phet/js/LightBulbNode.ts b/scenery-phet/js/LightBulbNode.ts --- a/scenery-phet/js/LightBulbNode.ts (revision 313e4aa4b886068e04bbc37eca85873134b844ab) +++ b/scenery-phet/js/LightBulbNode.ts (date 1718902970196) @@ -100,8 +100,10 @@ assert && assert( brightness >= 0 && brightness <= 1 ); this.onNode.visible = ( brightness > 0 ); if ( this.onNode.visible ) { + console.log( Date.now(), 'opacity', Utils.linear( 0, 1, 0.3, 1, brightness ) ); this.onNode.opacity = Utils.linear( 0, 1, 0.3, 1, brightness ); } + console.log( Date.now(), 'brightness', brightness ); this.raysNode.setBrightness( brightness ); } }

[pixelzoom]

I tried this workaround in FELLightBulbNode.ts. It works if 1 frame is skipped, but apparently multiple frames are being skipped.

Subject: [PATCH] doc tweaks
---
Index: js/common/view/FELLightBulbNode.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/common/view/FELLightBulbNode.ts b/js/common/view/FELLightBulbNode.ts
--- a/js/common/view/FELLightBulbNode.ts    (revision 6491973eb7ade2ecfb4a0857a683b04ea2b802d8)
+++ b/js/common/view/FELLightBulbNode.ts    (date 1718915184965)
@@ -22,6 +22,7 @@
 import CurrentIndicator from '../model/CurrentIndicator.js';
 import PickRequired from '../../../../phet-core/js/types/PickRequired.js';
 import optionize from '../../../../phet-core/js/optionize.js';
+import NumberProperty from '../../../../axon/js/NumberProperty.js';

 type SelfOptions = {
   maxRayLength?: number; // passed to LightBulbNode
@@ -64,7 +65,20 @@
       bottom: baseNode.top + 14 // overlap enough to hide the bottom rounded corners
     } );

-    const bulbNode = new LightBulbNode( lightBulb.brightnessProperty, {
+    // Workaround for skipped frame, see https://github.com/phetsims/faradays-electromagnetic-lab/issues/180.
+    // When the polarity of the magnet is flipped, lightBulb.brightnessProperty is non-zero for exactly 1 frame.
+    // If the view skips a frame, this event will not be rendered. So this workaround looks for the brightnessProperty
+    // transition 0 -> !0 -> 0, and keeps the !0 value for an additional time step, so that it will be rendered.
+    const brightnessProperty = new NumberProperty( lightBulb.brightnessProperty.value );
+    let oldOldBrightness = 0; // value of brightnessProperty 2 changes ago.
+    lightBulb.brightnessProperty.lazyLink( ( newBrightness, oldBrightness ) => {
+      if ( !( oldOldBrightness === 0 && newBrightness !== 0 && oldBrightness === 0 ) ) {
+        brightnessProperty.value = newBrightness;
+      }
+      oldOldBrightness = oldBrightness;
+    } );
+
+    const bulbNode = new LightBulbNode( brightnessProperty, {
       bulbImageScale: 0.5,

       // From the Java version, see LightRaysGraphic.java

[pixelzoom]

I've scheduled a meeting for this afternoon with @samreid and @jonathanolson.

To summarize:

• Flipping the magnet results in lightBulb.brightnessProperty having a non-zero value for exactly 1 model step.
• The view is skipping 1 or more frames.
• When the view skips frames, the non-zero value of lightBulb.brightnessProperty is not rendered, so the user does not see the bulb "light".

I'd like to understand:

• When does the view skip a frame?
• Can we detect when the view skips a frame?
• Can we detect when a Node has been redrawn?
• What is an appropriate workaround?

[samreid]

```diff Subject: [PATCH] Limit the viscous force to prevent changing the velocity direction, see https://github.com/phetsims/density-buoyancy-common/issues/223 --- Index: js/common/view/FELLightBulbNode.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/common/view/FELLightBulbNode.ts b/js/common/view/FELLightBulbNode.ts --- a/js/common/view/FELLightBulbNode.ts (revision 6491973eb7ade2ecfb4a0857a683b04ea2b802d8) +++ b/js/common/view/FELLightBulbNode.ts (date 1718918739382) @@ -22,6 +22,7 @@ import CurrentIndicator from '../model/CurrentIndicator.js'; import PickRequired from '../../../../phet-core/js/types/PickRequired.js'; import optionize from '../../../../phet-core/js/optionize.js'; +import NumberProperty from '../../../../axon/js/NumberProperty.js'; type SelfOptions = { maxRayLength?: number; // passed to LightBulbNode @@ -31,7 +32,10 @@ export default class FELLightBulbNode extends Node { - public constructor( lightBulb: LightBulb, + private brightnessHistory: number[] = []; + private brightnessProperty: NumberProperty; + + public constructor( private readonly lightBulb: LightBulb, currentIndicatorProperty: TReadOnlyProperty, providedOptions: FELLightBulbNodeOptions ) { @@ -64,7 +68,10 @@ bottom: baseNode.top + 14 // overlap enough to hide the bottom rounded corners } ); - const bulbNode = new LightBulbNode( lightBulb.brightnessProperty, { + + const numberProperty = new NumberProperty( lightBulb.brightnessProperty.value ); + + const bulbNode = new LightBulbNode( numberProperty, { bulbImageScale: 0.5, // From the Java version, see LightRaysGraphic.java @@ -90,9 +97,21 @@ super( options ); + + this.brightnessProperty = numberProperty; + this.brightnessHistory.push( lightBulb.brightnessProperty.value ); + this.addLinkedElement( lightBulb ); } + public step(): void { + this.brightnessHistory.push( this.lightBulb.brightnessProperty.value ); + if ( this.brightnessHistory.length > 5 ) { + this.brightnessHistory.shift(); + } + this.brightnessProperty.value = Math.max( ...this.brightnessHistory ); + } + /** * Creates an icon for the light bulb. */ Index: js/pickup-coil/view/PickupCoilScreenView.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/pickup-coil/view/PickupCoilScreenView.ts b/js/pickup-coil/view/PickupCoilScreenView.ts --- a/js/pickup-coil/view/PickupCoilScreenView.ts (revision 6491973eb7ade2ecfb4a0857a683b04ea2b802d8) +++ b/js/pickup-coil/view/PickupCoilScreenView.ts (date 1718918660073) @@ -22,6 +22,7 @@ import FELPreferences from '../../common/model/FELPreferences.js'; export default class PickupCoilScreenView extends FELScreenView { + private readonly pickupCoilNode: PickupCoilNode; public constructor( model: PickupCoilScreenModel, preferences: FELPreferences, tandem: Tandem ) { @@ -62,11 +63,12 @@ tandem: tandem.createTandem( 'barMagnetNode' ) } ); - const pickupCoilNode = new PickupCoilNode( pickupCoil, { + this.pickupCoilNode = new PickupCoilNode( pickupCoil, { dragBoundsProperty: dragBoundsProperty, lockToAxisProperty: lockToAxisProperty, tandem: tandem.createTandem( 'pickupCoilNode' ) } ); + const pickupCoilNode = this.pickupCoilNode; const pickupCoilAxisNode = new PickupCoilAxisNode( lockToAxisProperty, pickupCoil.positionProperty, this.visibleBoundsProperty ); @@ -110,6 +112,12 @@ this.resetAllButton ]; } + + public override step( dt: number ):void { + super.step( dt ); + + this.pickupCoilNode.step(); + } } faradaysElectromagneticLab.register( 'PickupCoilScreenView', PickupCoilScreenView ); \ No newline at end of file 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 6491973eb7ade2ecfb4a0857a683b04ea2b802d8) +++ b/js/common/view/PickupCoilNode.ts (date 1718918599617) @@ -38,6 +38,7 @@ export default class PickupCoilNode extends FELMovableNode { public readonly backgroundNode: Node; // See CoilNode backgroundNode + private readonly myBulb: FELLightBulbNode; public constructor( pickupCoil: PickupCoil, providedOptions: PickupCoilNodeOptions ) { @@ -85,6 +86,8 @@ super( pickupCoil.positionProperty, options ); + this.myBulb = lightBulbNode; + this.addLinkedElement( pickupCoil ); this.backgroundNode = coilNode.backgroundNode; @@ -121,6 +124,10 @@ this.backgroundNode.cursor = cursor; } ); } + + public step():void{ + this.myBulb.step(); + } } faradaysElectromagneticLab.register( 'PickupCoilNode', PickupCoilNode ); \ No newline at end of file

[pixelzoom]

Notes from meeting with @samreid and @jonathanolson ...

• There is no browser API for detecting dropped frames.
• There is no way to detect if the browser dropped a frame from scenery/joist.
• Changing LightRaysNode to renderer: ‘canvas’ did not resolved the problem.
• @jonathanolson could not reproduce on MacBook M1 Pro.
• @samreid could reproduce on MacBook Air M1.
• I can reproduce on MacBook Pro Intel.
• @samreid suggested sampling and using the max, see patch in https://github.com/phetsims/faradays-electromagnetic-lab/issues/180#issuecomment-2181589100. That patch results in a bit of lag, and the length of time the bulb is lit is inconsistent. I'll pursue this direction.

[pixelzoom]

Evolution of @samreid's patch is attached below. There are still cases where I'm not seeing the light rays when pressing the "Flip Polarity" button. And there are cases where I'm now seeing light rays twice when pushing the "Flip Polarity" button once.

patch

```diff Subject: [PATCH] doc tweaks --- 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 6491973eb7ade2ecfb4a0857a683b04ea2b802d8) +++ b/js/common/view/PickupCoilNode.ts (date 1718920666742) @@ -38,6 +38,7 @@ export default class PickupCoilNode extends FELMovableNode { public readonly backgroundNode: Node; // See CoilNode backgroundNode + private readonly lightBulbNode: FELLightBulbNode; public constructor( pickupCoil: PickupCoil, providedOptions: PickupCoilNodeOptions ) { @@ -85,6 +86,8 @@ super( pickupCoil.positionProperty, options ); + this.lightBulbNode = lightBulbNode; + this.addLinkedElement( pickupCoil ); this.backgroundNode = coilNode.backgroundNode; @@ -121,6 +124,10 @@ this.backgroundNode.cursor = cursor; } ); } + + public step(): void { + this.lightBulbNode.step(); + } } faradaysElectromagneticLab.register( 'PickupCoilNode', PickupCoilNode ); \ No newline at end of file Index: js/common/view/FELLightBulbNode.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/common/view/FELLightBulbNode.ts b/js/common/view/FELLightBulbNode.ts --- a/js/common/view/FELLightBulbNode.ts (revision 6491973eb7ade2ecfb4a0857a683b04ea2b802d8) +++ b/js/common/view/FELLightBulbNode.ts (date 1718921789903) @@ -22,6 +22,9 @@ import CurrentIndicator from '../model/CurrentIndicator.js'; import PickRequired from '../../../../phet-core/js/types/PickRequired.js'; import optionize from '../../../../phet-core/js/optionize.js'; +import NumberProperty from '../../../../axon/js/NumberProperty.js'; + +const NUMBER_OF_BRIGHTNESS_SAMPLES = 5; type SelfOptions = { maxRayLength?: number; // passed to LightBulbNode @@ -31,6 +34,10 @@ export default class FELLightBulbNode extends Node { + private readonly lightBulb: LightBulb; + private readonly brightnessSamples: number[]; + private readonly brightnessProperty: NumberProperty; + public constructor( lightBulb: LightBulb, currentIndicatorProperty: TReadOnlyProperty, providedOptions: FELLightBulbNodeOptions ) { @@ -64,7 +71,9 @@ bottom: baseNode.top + 14 // overlap enough to hide the bottom rounded corners } ); - const bulbNode = new LightBulbNode( lightBulb.brightnessProperty, { + const brightnessProperty = new NumberProperty( lightBulb.brightnessProperty.value ); + + const bulbNode = new LightBulbNode( brightnessProperty, { bulbImageScale: 0.5, // From the Java version, see LightRaysGraphic.java @@ -90,9 +99,41 @@ super( options ); + this.lightBulb = lightBulb; + this.brightnessSamples = [ lightBulb.brightnessProperty.value ]; + this.brightnessProperty = brightnessProperty; + this.addLinkedElement( lightBulb ); } + public step(): void { + + const brightness = this.lightBulb.brightnessProperty.value; + + // Push the new sample, and pop the oldest sample. + this.brightnessSamples.push( brightness ); + if ( this.brightnessSamples.length > NUMBER_OF_BRIGHTNESS_SAMPLES ) { + this.brightnessSamples.shift(); + } + + // Count the number of samples that are non-zero. + let nonZeroCount = 0; + this.brightnessSamples.forEach( brightness => { + if ( brightness !== 0 ) { + nonZeroCount++; + } + } ); + + // If we only had 1 non-zero sample, then it's likely from flipping the bar magnet's polarity, so use that sample. + // Otherwise, use the most recent sample. + if ( nonZeroCount === 1 ) { + this.brightnessProperty.value = Math.max( ...this.brightnessSamples ); + } + else { + this.brightnessProperty.value = brightness; + } + } + /** * Creates an icon for the light bulb. */ Index: js/pickup-coil/view/PickupCoilScreenView.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/pickup-coil/view/PickupCoilScreenView.ts b/js/pickup-coil/view/PickupCoilScreenView.ts --- a/js/pickup-coil/view/PickupCoilScreenView.ts (revision 6491973eb7ade2ecfb4a0857a683b04ea2b802d8) +++ b/js/pickup-coil/view/PickupCoilScreenView.ts (date 1718920666734) @@ -22,6 +22,7 @@ import FELPreferences from '../../common/model/FELPreferences.js'; export default class PickupCoilScreenView extends FELScreenView { + private readonly pickupCoilNode: PickupCoilNode; public constructor( model: PickupCoilScreenModel, preferences: FELPreferences, tandem: Tandem ) { @@ -62,11 +63,12 @@ tandem: tandem.createTandem( 'barMagnetNode' ) } ); - const pickupCoilNode = new PickupCoilNode( pickupCoil, { + this.pickupCoilNode = new PickupCoilNode( pickupCoil, { dragBoundsProperty: dragBoundsProperty, lockToAxisProperty: lockToAxisProperty, tandem: tandem.createTandem( 'pickupCoilNode' ) } ); + const pickupCoilNode = this.pickupCoilNode; const pickupCoilAxisNode = new PickupCoilAxisNode( lockToAxisProperty, pickupCoil.positionProperty, this.visibleBoundsProperty ); @@ -110,6 +112,11 @@ this.resetAllButton ]; } + + public override step( dt: number ):void { + super.step( dt ); + this.pickupCoilNode.step(); + } } faradaysElectromagneticLab.register( 'PickupCoilScreenView', PickupCoilScreenView ); \ No newline at end of file ```

[pixelzoom]

To verify that scenery-phet LightRaysNode was not a performance problem, I reduced minRays and maxRays to 1. The problem persists.

Subject: [PATCH] doc tweaks
---
Index: js/common/view/FELLightBulbNode.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/common/view/FELLightBulbNode.ts b/js/common/view/FELLightBulbNode.ts
--- a/js/common/view/FELLightBulbNode.ts    (revision 6491973eb7ade2ecfb4a0857a683b04ea2b802d8)
+++ b/js/common/view/FELLightBulbNode.ts    (date 1718922134563)
@@ -70,8 +70,8 @@
       // From the Java version, see LightRaysGraphic.java
       lightRaysNodeOptions: {
         pickable: false,
-        minRays: 20,
-        maxRays: 20,
+        minRays: 1,
+        maxRays: 1,
         minRayLength: 0,
         maxRayLength: options.maxRayLength,
         longRayLineWidth: 2,

[pixelzoom]

Only because I hadn't done so yet, I tried running with ?webgl=false. The problem persists.

[pixelzoom]

I tried totally replacing LightBulbNode and LightRaysNode (FELLightBulbNode) with a simple Circle, see patch below. The problem persists. So the problem is not with the implementation of the light bulb view.

patch

```diff Subject: [PATCH] doc tweaks --- 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 6491973eb7ade2ecfb4a0857a683b04ea2b802d8) +++ b/js/common/view/PickupCoilNode.ts (date 1718923156937) @@ -15,7 +15,6 @@ import FELMovableNode, { FELMovableNodeOptions } from './FELMovableNode.js'; import { Node, NodeOptions } from '../../../../scenery/js/imports.js'; import PickupCoilSamplePointsNode from './PickupCoilSamplePointsNode.js'; -import FELLightBulbNode from './FELLightBulbNode.js'; import VoltmeterNode from './VoltmeterNode.js'; import optionize from '../../../../phet-core/js/optionize.js'; import PickRequired from '../../../../phet-core/js/types/PickRequired.js'; @@ -25,6 +24,7 @@ import StrictOmit from '../../../../phet-core/js/types/StrictOmit.js'; import { Shape } from '../../../../kite/js/imports.js'; import Multilink from '../../../../axon/js/Multilink.js'; +import CircleOfLightNode from './CircleOfLightNode.js'; type SelfOptions = { maxRayLength?: number; // passed to LightBulbNode @@ -56,7 +56,11 @@ const areaNode = new PickupCoilAreaNode( pickupCoil ); const samplePointsNode = new PickupCoilSamplePointsNode( pickupCoil ); - const lightBulbNode = new FELLightBulbNode( pickupCoil.lightBulb, pickupCoil.currentIndicatorProperty, { + // const lightBulbNode = new FELLightBulbNode( pickupCoil.lightBulb, pickupCoil.currentIndicatorProperty, { + // maxRayLength: options.maxRayLength, + // tandem: options.tandem.createTandem( 'lightBulbNode' ) + // } ); + const lightBulbNode = new CircleOfLightNode( pickupCoil.lightBulb, pickupCoil.currentIndicatorProperty, { maxRayLength: options.maxRayLength, tandem: options.tandem.createTandem( 'lightBulbNode' ) } ); Index: js/common/view/CircleOfLightNode.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/common/view/CircleOfLightNode.ts b/js/common/view/CircleOfLightNode.ts new file mode 100644 --- /dev/null (date 1718923210116) +++ b/js/common/view/CircleOfLightNode.ts (date 1718923210116) @@ -0,0 +1,55 @@ +// Copyright 2024, University of Colorado Boulder + +/** + * CircleOfLightNode is a temporary replacement for FELLightBulbNode, to test for performance problems. + * TODO https://github.com/phetsims/faradays-electromagnetic-lab/issues/180 + * + * @author Chris Malley (PixelZoom, Inc.) + */ + +import faradaysElectromagneticLab from '../../faradaysElectromagneticLab.js'; +import { Circle, CircleOptions, NodeOptions } from '../../../../scenery/js/imports.js'; +import LightBulb from '../model/LightBulb.js'; +import TReadOnlyProperty from '../../../../axon/js/TReadOnlyProperty.js'; +import CurrentIndicator from '../model/CurrentIndicator.js'; +import PickRequired from '../../../../phet-core/js/types/PickRequired.js'; +import DerivedProperty from '../../../../axon/js/DerivedProperty.js'; +import BooleanIO from '../../../../tandem/js/types/BooleanIO.js'; +import optionize from '../../../../phet-core/js/optionize.js'; +import FELColors from '../FELColors.js'; + +type SelfOptions = { + maxRayLength?: number; +}; + +type CircleOfLightNodeOptions = SelfOptions & PickRequired; + +export default class CircleOfLightNode extends Circle { + + public constructor( lightBulb: LightBulb, + currentIndicatorProperty: TReadOnlyProperty, + providedOptions: CircleOfLightNodeOptions ) { + + const options = optionize()( { + + // SelfOptions + maxRayLength: 500, + + // CircleOptions + fill: FELColors.lightRaysColorProperty, + opacity: 0.65, + visibleProperty: new DerivedProperty( [ currentIndicatorProperty ], indicator => ( indicator === lightBulb ), { + tandem: providedOptions.tandem.createTandem( 'visibleProperty' ), + phetioValueType: BooleanIO + } ) + }, providedOptions ); + + super( options ); + + lightBulb.brightnessProperty.link( brightness => { + this.radius = brightness * options.maxRayLength; + } ); + } +} + +faradaysElectromagneticLab.register( 'CircleOfLightNode', CircleOfLightNode ); \ No newline at end of file ```

[pixelzoom]

I tried totally disabing updates of FieldNode and CurrentNode, my 2 subclasses of Sprites. The problem persists.

Since I often see the flipPolarityButton stuck in the "pressed" state when frames are dropped, I wanted to rule out performance problems with sun buttons. In the patch below, I replace the TextPushButton with Text + PressListener. The problem persists.

patch

```diff Subject: [PATCH] doc tweaks --- Index: js/common/view/BarMagnetPanel.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/common/view/BarMagnetPanel.ts b/js/common/view/BarMagnetPanel.ts --- a/js/common/view/BarMagnetPanel.ts (revision 6491973eb7ade2ecfb4a0857a683b04ea2b802d8) +++ b/js/common/view/BarMagnetPanel.ts (date 1718923945873) @@ -10,7 +10,7 @@ import BarMagnet from '../model/BarMagnet.js'; import Panel, { PanelOptions } from '../../../../sun/js/Panel.js'; import Checkbox, { CheckboxOptions } from '../../../../sun/js/Checkbox.js'; -import { Node, Text, VBox, VBoxOptions } from '../../../../scenery/js/imports.js'; +import { Node, PressListener, Text, VBox, VBoxOptions } from '../../../../scenery/js/imports.js'; import TextPushButton from '../../../../sun/js/buttons/TextPushButton.js'; import { combineOptions, optionize4 } from '../../../../phet-core/js/optionize.js'; import FELConstants from '../FELConstants.js'; @@ -105,19 +105,18 @@ ( earthVisible, flipEarthString, flipPolarityString ) => earthVisible ? flipEarthString : flipPolarityString ); } - const flipPolarityButton = new TextPushButton( flipStringProperty, { + const flipPolarityButton = new Text( flipStringProperty, { font: FELConstants.CONTROL_FONT, - textNodeOptions: { - maxWidth: 180 - }, - listener: () => { + tandem: options.tandem.createTandem( 'flipPolarityButton' ) + } ); + contentChildren.push( flipPolarityButton ); + flipPolarityButton.cursor = 'pointer'; + flipPolarityButton.addInputListener( new PressListener( { + press: () => { barMagnet.flipPolarity(); compass.startMovingNow(); - }, - layoutOptions: { stretch: false }, - tandem: options.tandem.createTandem( 'flipPolarityButton' ) - } ); - contentChildren.push( flipPolarityButton ); + } + } ) ); } const content = new VBox( combineOptions( {}, FELConstants.VBOX_OPTIONS, { ```