Manage material colors now that materials are mutable

[samreid]

From TODOs in #256

[zepumph]

10 TODOs left

[zepumph]

• Also noting that it seems like for density compare (and likely buoyancy too), we should be able to change the underlying color that darkens/lightens based on density.
  UPDATE: @arouinfar and I do not want to do this afterall. We just need Density Mystery block colors customizable.

[AgustinVallejo]

Patch for linear color mapping

Subject: [PATCH] Linear color mapping
---
Index: js/common/model/Material.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/common/model/Material.ts b/js/common/model/Material.ts
--- a/js/common/model/Material.ts	(revision ca655a7534626154b25b1e5c4c1ad922195b2db0)
+++ b/js/common/model/Material.ts	(date 1721937114972)
@@ -133,14 +133,11 @@
 
   /**
    * Returns a lightness factor from 0-1 that can be used to map a density to a desired color.
-   * TODO: This has a poor dynamic range for the bottle inside material, but is used elsewhere. Should it be changed/split/improved? https://github.com/phetsims/density-buoyancy-common/issues/268
+   * TODO AV: This has a poor dynamic range for the bottle inside material, but is used elsewhere. Should it be changed/split/improved? https://github.com/phetsims/density-buoyancy-common/issues/268
    */
   public static getNormalizedLightness( density: number, densityRange: Range ): number {
-    const scaleFactor = 1000;
-    const scaleMax = Utils.log10( densityRange.max / scaleFactor ); // 1 for the default
-    const scaleMin = Utils.log10( densityRange.min / scaleFactor ); // -2 for the default
-    const scaleValue = Utils.log10( density / scaleFactor );
-    return Utils.clamp( Utils.linear( scaleMax, scaleMin, 0, 1, scaleValue ), 0, 1 );
+    const scaling = 0.8;
+    return 0.5 - scaling * ( densityRange.getNormalizedValue( density ) - 0.5 );
   }
 
   /**

[AgustinVallejo]

@zepumph and I have a big question after taking a tour of this issue: Regarding the color-density relationship, do we want a global lightness map for the whole sim suite, or something more custom for every screen's achievable densities??

Some potential design problems:

1. If we use a global map, i.e. minimally possible density to maximally possible density (0.0008 - 27) kg/L, the sliders in compare screens won't ever reach the borders, as they usually range on the smaller side of densities (0.1 - 3). So those blocks would look always whiteish.
2. If we use custom color maps, a dark block on one screen could have the same density as a white block on another screen, just because they don't share the same density range.

Some additional remarks:

• At their extremes, custom blocks can go from totally white to totally black. We can also choose softer limits.
• There's also color changing:
  • custom pool fluid. (CustomLiquidMaterial) (although this one seems fine for now :) )
  • custom bottle fluid (currently identical to the custom block) (CustomSolidMaterial)
  • compare screen blocks currently have their own color logic. (CompareBlockSetModel)

If we go with having custom color maps, how should they behave? Screen by screen basis? Sim by sim?

[zepumph]

Noting that the behavior right before this material change was actually in the MaterialMassVolumeControl, and made it so all blocks could go from white->black completely.

density-buoyancy-common/js/common/view/MaterialControlNode.ts

Lines 70 to 76 in e9d1887

	this.customDensityRange = new Range(
	options.minCustomMass / options.maxVolumeLiters * DensityBuoyancyCommonConstants.LITERS_IN_CUBIC_METER,
	// Prevent divide by zero errors (infinity) with a manual, tiny number
	options.maxCustomMass / ( options.minCustomVolumeLiters ) * DensityBuoyancyCommonConstants.LITERS_IN_CUBIC_METER
	);

UPDATE: This patch brings back the previous behavior above, as if we were attempting to go with (2) above instead of a global map:

Subject: [PATCH] remove gravity-force-lab, too old
---
Index: js/common/view/MaterialControlNode.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/common/view/MaterialControlNode.ts b/js/common/view/MaterialControlNode.ts
--- a/js/common/view/MaterialControlNode.ts	(revision ca655a7534626154b25b1e5c4c1ad922195b2db0)
+++ b/js/common/view/MaterialControlNode.ts	(date 1721940690565)
@@ -19,6 +19,7 @@
 import densityBuoyancyCommon from '../../densityBuoyancyCommon.js';
 import MaterialProperty from '../model/MaterialProperty.js';
 import Utils from '../../../../dot/js/Utils.js';
+import Range from '../../../../dot/js/Range.js';
 
 type SelfMaterialControlNodeOptions = {
 
@@ -76,6 +77,18 @@
       };
     };
 
+    // TODO: workable, but buggy with reset since the rangeProperty initialized to a min of .8
+    if ( customMaterials.length > 0 ) {
+      const customDensityRange = new Range(
+        options.minCustomMass / options.maxVolumeLiters * DensityBuoyancyCommonConstants.LITERS_IN_CUBIC_METER,
+
+        // Prevent divide by zero errors (infinity) with a manual, tiny number
+        options.maxCustomMass / ( options.minCustomVolumeLiters ) * DensityBuoyancyCommonConstants.LITERS_IN_CUBIC_METER
+      );
+      materialProperty.customMaterial.densityProperty.rangeProperty.value = customDensityRange;
+
+    }
+
     // When switching to custom, set the custom density to the previous material's density (clamped just in case).
     // However, when switching from a mystery material, do not change the custom value. This prevents clever students from discovering
     // the mystery values by using the UI instead of by computing them, see https://github.com/phetsims/buoyancy/issues/54

[AgustinVallejo]

Reminder: Try both of the above patches at once, to see how the linear mapping behaves for the other custom colors.

[zepumph]

Given how much @AgustinVallejo and I liked the patch above, I'd like to experiment with calculating the range in CompareBlockSetModel as well.

[zepumph]

The compare case went really nicely, and makes a lot of sense for that screen. After thinking through each case above, I believe that seeing white/black for custom block materials fully on each screen is preferable. I will commit the above changes, and we can discuss sync tomorrow morning.

[zepumph]

When applying the patch in #268 (comment), I ran into a bug where the current log-based lightness function returns NaN when the range is [0,20000] and the density is 0, so I'll proceed with #268 (comment) too.

[zepumph]

• The max density of the custom material in the bottle should not go up to 200KG/L. Oops.

[zepumph]

From design meeting:

1. What colors need phet-io customization

   • Mystery materials (solid) should be able to change their color globally.
     • N/A for gold/pyrite since they have a texture not a color. Don't add anything to these.
     • NOT FOR mystery fluids!! This is because of contrast concerns that would apply to all pools on all screens.
   • customizing colors of fluids( custom and mystery and others): We don't want to change these colors.
   • Blocks:
     • custom material black/white:
       • Code review: This should act like fluid color interpolation, with two color profile properties instead of just a gray scale logic
         • Ask designers to review the actual colors for white/black once they are in the color profile.
       • We do not think this should be instrumented or customizable. You are stuck with white/black.

2. Behavior of light/dark logic for custom materials

   • On the compare screen, revert last night's commit and use a hard coded range per screen that doesn't care about outliers.

[zepumph]

We may want to go back to a log-based lightness function, as was used in density:

density-buoyancy-common/js/common/model/Material.ts

Lines 148 to 151 in 7bfaab3

	*/
	public static getCustomLightness( density: number ): number {
	return Utils.roundSymmetric( Utils.clamp( Utils.linear( 1, -2, 0, 255, Utils.log10( density / 1000 ) ), 0, 255 ) );
	}

[zepumph]

The max density of the custom material in the bottle should not go up to 200KG/L. Oops.

Alright. I fixed a couple bugs in cases where there was already a density slider and a range for a custom denstiy (blocks in B:B:explore and Bottle.materialInside).

[zepumph]

@AgustinVallejo and I got to an algorithm for color mapping in the compare screen that we liked. It involved linear mapping instead of log + exponential mapping, originally added back in phetsims/density#84.

The rest of the TODOs are over to @AgustinVallejo. Thanks everyone!

[AgustinVallejo]

On friday I spoke with @arouinfar and she likes the new dynamic range. Also, in the above commit I made colorProperty read only implementing a function called createColorProperty, overriden in the children of Material. But I'm not totally sure if that's the right way to go, specially handling null cases. Would like to review with @samreid tomorrow.