Light bulb rays flicker

[matthew-blackman]

Discovered as part of #103. As the brightness of the bulb changes, the rays undergo a nice stretching effect. This makes it easy to see the change in brightness of the bulb. However I noticed that while this is happening, there is also a flicker effect that is caused by the arrangement of rays changing. This makes the brightness more difficult to visually track (see below).

bulb.mp4

If it's not too much cost, I think it would be a nice improvement to this feature if the rays only stretched, and did not redraw with different positions. This would make the brightness oscillation of the AC transformer very smooth and clear to the user.

[pixelzoom]

This would be a change to scenery-phet LightBulbNode. It would also affect EFAC and ABS (ConductivityTesterNode).

[matthew-blackman]

I worked on a patch that adds an option to LightRaysNode. The resulting bulb behavior in FEL looks significantly better to me, especially for seeing the change in brightness when it oscillates. @arouinfar can you confirm that this is an improvement? Here are the before and after behavior:

Before:

Screen.Recording.2024-03-26.at.2.00.32.PM.mov

After:

Screen.Recording.2024-03-26.at.2.00.00.PM.mov

Here is the patch - @pixelzoom can you review and see if this is a reasonable addition to LightRaysNode?

Subject: [PATCH] Add REVIEW comments - see https://github.com/phetsims/faradays-electromagnetic-lab/issues/122
---
Index: scenery-phet/js/LightRaysNode.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/scenery-phet/js/LightRaysNode.ts b/scenery-phet/js/LightRaysNode.ts
--- a/scenery-phet/js/LightRaysNode.ts	(revision f42720a49e7aaf3f79a6e8dc0d736c2bf8bd7188)
+++ b/scenery-phet/js/LightRaysNode.ts	(date 1711476117352)
@@ -24,6 +24,7 @@
   longRayLineWidth?: number; // for long rays
   mediumRayLineWidth?: number; // for medium-length rays
   shortRayLineWidth?: number; // for short rays
+  constantNumRays?: boolean; // if true, the number of rays is always equal to minRays
 };
 
 export type LightRaysNodeOptions = SelfOptions & PathOptions;
@@ -38,6 +39,7 @@
   private readonly longRayLineWidth: number;
   private readonly mediumRayLineWidth: number;
   private readonly shortRayLineWidth: number;
+  private readonly constantNumRays: boolean;
 
   public constructor( bulbRadius: number, provideOptions?: LightRaysNodeOptions ) {
 
@@ -53,6 +55,7 @@
       longRayLineWidth: 1.5, // for long rays
       mediumRayLineWidth: 1, // for medium-length rays
       shortRayLineWidth: 0.5, // for short rays
+      constantNumRays: false,
 
       // PathOptions
       stroke: 'yellow'
@@ -68,6 +71,7 @@
     this.mediumRayLineWidth = options.mediumRayLineWidth;
     this.longRayLineWidth = options.longRayLineWidth;
     this.shortRayLineWidth = options.shortRayLineWidth;
+    this.constantNumRays = options.constantNumRays;
 
     // Ensures there are well-defined bounds at initialization
     this.setBrightness( 0 );
@@ -84,7 +88,7 @@
     assert && assert( brightness >= 0 && brightness <= 1 );
 
     // number of rays is a function of brightness
-    const numberOfRays = ( brightness === 0 ) ? 0 : this.minRays + Utils.roundSymmetric( brightness * ( this.maxRays - this.minRays ) );
+    const numberOfRays = this.constantNumRays ? this.minRays : ( brightness === 0 ) ? 0 : this.minRays + Utils.roundSymmetric( brightness * ( this.maxRays - this.minRays ) );
 
     // ray length is a function of brightness
     const rayLength = this.minRayLength + ( brightness * ( this.maxRayLength - this.minRayLength ) );
Index: faradays-electromagnetic-lab/js/common/view/FELLightBulbNode.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/faradays-electromagnetic-lab/js/common/view/FELLightBulbNode.ts b/faradays-electromagnetic-lab/js/common/view/FELLightBulbNode.ts
--- a/faradays-electromagnetic-lab/js/common/view/FELLightBulbNode.ts	(revision b51d775c7e721228c2169d40135b1f754abf6046)
+++ b/faradays-electromagnetic-lab/js/common/view/FELLightBulbNode.ts	(date 1711476079260)
@@ -44,11 +44,12 @@
 
       // From the Java version, see LightRaysGraphic.java
       lightRaysNodeOptions: {
-        minRays: 8,
+        constantNumRays: true,
+        minRays: 12,
         maxRays: 60,
         minRayLength: 0,
         maxRayLength: 350,
-        longRayLineWidth: 3,
+        longRayLineWidth: 2,
         mediumRayLineWidth: 2,
         shortRayLineWidth: 1,
         stroke: FELColors.lightRaysColorProperty

[pixelzoom]

@matthew-blackman @samreid and I looked at this one together. The same effect can be achived in FELLightBulbNode, without making changes to common-code LightBulbNode:

    const bulbNode = new LightBulbNode( lightBulb.brightnessProperty, {
      bulbImageScale: 0.5,

      // From the Java version, see LightRaysGraphic.java
      lightRaysNodeOptions: {
+       minRays: 20,
+       maxRays: 20,
        minRayLength: 0,
        maxRayLength: 350,
+       longRayLineWidth: 2,

[pixelzoom]

@arouinfar and I review this via Zoom. I implemented what's shown in #120 (comment). Test drive in main or 1.0.0-dev.20.

This obviously still needs calibration in #66, but looks like a nice improvement to me.

Over to @arouinfar for review. Close if OK.

[arouinfar]

Thanks for the suggestion @matthew-blackman! I agree @pixelzoom, this is a nice improvement. Closing.