phetsims / density

"Density" is an educational simulation in HTML5, by PhET Interactive Simulations.
GNU General Public License v3.0
0 stars 6 forks source link

Same volume yellow block has wrong properties and color #138

Closed KatieWoe closed 2 years ago

KatieWoe commented 2 years ago

Device Dell OS Win 11 Browser Chrome Problem Description For https://github.com/phetsims/qa/issues/820. The property density.compareScreen.model.blockSets.sameVolume.yellowBlock.materialProperty does not seem to exist under the studio tree. The properties under the yellow block in the same volume scene of the compare screen are also different that the other blocks in the scene and screen. The yellow is also much greener than the yellow used in the other scenes. This is also seen in the standalone version Visuals

yellowwrong wrong
arouinfar commented 2 years ago

I confirmed that density.compareScreen.model.blockSets.sameVolume.yellowBlock.materialProperty exists, but for some reason it's not phetioFeatured: true. I will address that in #137.

The color of sameVolume.yellowBlock definitely looks a bit off when compared to the other scenes. @jonathanolson can you clarify how the gradient is applied to the block color on the Compare Screen? Is it simply proportional to density, or is there some normalization based on the scene?

KatieWoe commented 2 years ago

Noting that this odd color was present in the published version, but wasn't noticed before.

jonathanolson commented 2 years ago

Yes, this looks as expected (since the yellow block is much denser than the others, it will appear darker).

The code that determines the appearance is:

        const lightness = Material.getCustomLightness( density ); // 0-255

        const modifier = 0.1;
        const rawValue = ( lightness / 128 - 1 ) * ( 1 - modifier ) + modifier;
        const power = 0.7;
        const modifiedColor = color.colorUtilsBrightness( Math.sign( rawValue ) * Math.pow( Math.abs( rawValue ), power ) );

        return Material.createCustomMaterial( {
          density: density,
          customColor: new Property( modifiedColor, { tandem: Tandem.OPT_OUT } )
        } );

(with the provided color - yellow, and density) - It blends the color with the density.

The utility function for density => lightness in general is

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

Thanks for the clarification @jonathanolson.

It seems like if we want the yellow block to look a bit more yellow, we should swap its density with one of the other blocks. @DianaTavares what would you like to do?

DianaTavares commented 2 years ago

I understand that the only problem wiht this block is that its name (yellow) is not so yellow in the view, right?

We agree that the darkness of the block depends on its density, and one block of this compare screen needs a high density to make comparisons more evident; then, the density needs to be the way it is. In the initial state of this scene, we have 2 kg, 4 kg, 6 kg, and 8 kg blocks, and the 8 kg is yellow. I don't think is a good idea to associate all yellow blocks in comparison to a similar density in all screens because that can give the wrong idea of association of color with density.

My opinion is that the block density is correct, and its color is correct.

KatieWoe commented 2 years ago

Alright. Closing