phetsims / buoyancy

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

Changes in the Intro screen #96

Closed DianaTavares closed 2 months ago

DianaTavares commented 3 months ago

As a result of the 8/Feb/24 design meeting discussion:

image

image

zepumph commented 3 months ago

@AgustinVallejo and I are curious why these blocks are called 1A and 1B. @DianaTavares is this correct to you?

zepumph commented 3 months ago

Discussed the above, and I didn't realize that the number changes from the radio buttons (same mass/density/volume). Thanks!

zepumph commented 3 months ago

A total mess:

```diff Subject: [PATCH] comment out bug until we can fix things, https://github.com/phetsims/buoyancy/issues/98 --- Index: js/common/view/MassLabelNode.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/common/view/MassLabelNode.ts b/js/common/view/MassLabelNode.ts --- a/js/common/view/MassLabelNode.ts (revision 1b4d44668e35e30b27583c4e36c13cf80b213f9e) +++ b/js/common/view/MassLabelNode.ts (date 1707951189177) @@ -22,26 +22,6 @@ import LabelTexture from './LabelTexture.js'; import Tandem from '../../../../tandem/js/Tandem.js'; -// constants -const MASS_LABEL_SIZE = 32; -const createMassLabel = ( string: TReadOnlyProperty, fill: TPaint ) => { - const rectangle = new Rectangle( 0, 0, MASS_LABEL_SIZE, MASS_LABEL_SIZE, { - cornerRadius: DensityBuoyancyCommonConstants.CORNER_RADIUS, - fill: fill - } ); - const label = new Text( string, { - font: new PhetFont( { size: 24, weight: 'bold' } ), - fill: 'white', - center: rectangle.center, - maxWidth: 30 - } ); - label.localBoundsProperty.link( () => { - label.center = rectangle.center; - } ); - rectangle.addChild( label ); - return rectangle; -}; - const PRIMARY_LABEL_DEPENDENCIES = [ DensityBuoyancyCommonStrings.massLabel.primaryStringProperty, DensityBuoyancyCommonColors.labelPrimaryProperty ]; const SECONDARY_LABEL_DEPENDENCIES = [ DensityBuoyancyCommonStrings.massLabel.secondaryStringProperty, DensityBuoyancyCommonColors.labelSecondaryProperty ]; Index: js/common/view/MassView.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/common/view/MassView.ts b/js/common/view/MassView.ts --- a/js/common/view/MassView.ts (revision 1b4d44668e35e30b27583c4e36c13cf80b213f9e) +++ b/js/common/view/MassView.ts (date 1707951189182) @@ -14,15 +14,56 @@ import Mass, { MassTag } from '../model/Mass.js'; import Material from '../model/Material.js'; import DensityMaterials from './DensityMaterials.js'; -import MassLabelNode from './MassLabelNode.js'; +import MassLabelNode, { SECONDARY_LABEL } from './MassLabelNode.js'; import MaterialView from './MaterialView.js'; -import { InteractiveHighlighting, Path } from '../../../../scenery/js/imports.js'; +import { InteractiveHighlighting, Path, Rectangle, Text, TPaint } from '../../../../scenery/js/imports.js'; import { Shape } from '../../../../kite/js/imports.js'; +import LabelTexture from './LabelTexture.js'; +import Multilink from '../../../../axon/js/Multilink.js'; +import { TinyProperty } from '../../../../axon/js/imports.js'; +import DensityBuoyancyCommonColors from './DensityBuoyancyCommonColors.js'; +import TReadOnlyProperty from '../../../../axon/js/TReadOnlyProperty.js'; +import DensityBuoyancyCommonConstants from '../DensityBuoyancyCommonConstants.js'; +import PhetFont from '../../../../scenery-phet/js/PhetFont.js'; const TAG_SIZE = 0.03; export const TAG_OFFSET = 0.005; const TAG_SCALE = 0.0005; + +// constants +const MASS_LABEL_SIZE = 32; +const createMassTagNode = ( string: TReadOnlyProperty, fill: TPaint ) => { + const rectangle = new Rectangle( 0, 0, MASS_LABEL_SIZE, MASS_LABEL_SIZE, { + cornerRadius: DensityBuoyancyCommonConstants.CORNER_RADIUS, + fill: fill + } ); + const label = new Text( string, { + font: new PhetFont( { size: 24, weight: 'bold' } ), + fill: 'white', + center: rectangle.center, + maxWidth: 100 + } ); + label.localBoundsProperty.link( () => { + label.center = rectangle.center; + } ); + rectangle.addChild( label ); + return rectangle; +}; + +const label = new Text( string, { + font: new PhetFont( { size: 24, weight: 'bold' } ), + maxWidth: 100 +} ); +const rectangle = new Rectangle( 0, 0, label.width + 5, label.height + 3, { + cornerRadius: DensityBuoyancyCommonConstants.CORNER_RADIUS, + fill: 'white' +} ); +label.center = rectangle.center; +rectangle.addChild( label ); + +return new LabelTexture( rectangle ); + export default abstract class MassView extends THREE.Mesh { public readonly mass: Mass; @@ -78,7 +119,20 @@ this.tagHeight = TAG_SIZE; } else if ( mass.tag === MassTag.SECONDARY ) { + + const colorProperty = mass.tag === MassTag.PRIMARY ? DensityBuoyancyCommonColors.labelPrimaryProperty : + mass.tag === MassTag.SECONDARY ? DensityBuoyancyCommonColors.labelSecondaryProperty : + new TinyProperty( 'white' ); + const texture = new LabelTexture( createMass ); + + // @ts-expect-error + const multilink = Multilink.multilink( [ mass.nameProperty, colorProperty ], () => texture.update() ); + texture.disposedEmitter.addListener( () => multilink.dispose() ); + + return texture; + this.tagNodeTexture = MassLabelNode.getSecondaryTexture(); + debugger; this.tagMesh = new TextureQuad( this.tagNodeTexture, TAG_SIZE, TAG_SIZE, { depthTest: true } );
zepumph commented 3 months ago

I think we can almost consider this to be progress.

```diff Subject: [PATCH] opt out of webglnode --- Index: js/common/view/MassView.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/common/view/MassView.ts b/js/common/view/MassView.ts --- a/js/common/view/MassView.ts (revision 16fab19b150adff3ca4f7ddf0fc0b3797b32a1ae) +++ b/js/common/view/MassView.ts (date 1708042594766) @@ -16,13 +16,55 @@ import DensityMaterials from './DensityMaterials.js'; import MassLabelNode from './MassLabelNode.js'; import MaterialView from './MaterialView.js'; -import { InteractiveHighlighting, Path } from '../../../../scenery/js/imports.js'; +import { Color, InteractiveHighlighting, Path, Rectangle, Text } from '../../../../scenery/js/imports.js'; import { Shape } from '../../../../kite/js/imports.js'; +import LabelTexture from './LabelTexture.js'; +import { TinyProperty } from '../../../../axon/js/imports.js'; +import DensityBuoyancyCommonColors from './DensityBuoyancyCommonColors.js'; +import TReadOnlyProperty from '../../../../axon/js/TReadOnlyProperty.js'; +import DensityBuoyancyCommonConstants from '../DensityBuoyancyCommonConstants.js'; +import PhetFont from '../../../../scenery-phet/js/PhetFont.js'; +import DensityBuoyancyCommonStrings from '../../DensityBuoyancyCommonStrings.js'; const TAG_SIZE = 0.03; export const TAG_OFFSET = 0.005; const TAG_SCALE = 0.0005; +const MASS_LABEL_SIZE = 32; + +// Calculated by comparing the original label rectangle size when providing primary/secondary tags +const horiztonalMargin = 15; +const verticalMargin = 5; + +// constants +const tagFont = new PhetFont( { size: 24, weight: 'bold' } ); + +const createMassTagTexture = ( string: TReadOnlyProperty, fill: TReadOnlyProperty ) => { + + const label = new Text( string, { + font: tagFont, + fill: 'white', + maxWidth: 100 + } ); + + // const rectangle = new Rectangle( 0, 0, MASS_LABEL_SIZE, MASS_LABEL_SIZE, { + const rectangle = new Rectangle( 0, 0, label.width + horiztonalMargin, label.height + verticalMargin, { + cornerRadius: DensityBuoyancyCommonConstants.CORNER_RADIUS, + fill: fill + } ); + console.log( rectangle.width, rectangle.height ); + label.localBoundsProperty.link( () => { + label.center = rectangle.center; + } ); + rectangle.addChild( label ); + const texture = new LabelTexture( rectangle ); + + // const multilink = Multilink.multilink( [ string, fill ], () => texture.update() ); + // texture.disposedEmitter.addListener( () => multilink.dispose() ); + return texture; +}; + + export default abstract class MassView extends THREE.Mesh { public readonly mass: Mass; @@ -78,11 +120,20 @@ this.tagHeight = TAG_SIZE; } else if ( mass.tag === MassTag.SECONDARY ) { - this.tagNodeTexture = MassLabelNode.getSecondaryTexture(); - this.tagMesh = new TextureQuad( this.tagNodeTexture, TAG_SIZE, TAG_SIZE, { - depthTest: true - } ); - this.tagHeight = TAG_SIZE; + + const colorProperty = mass.tag === MassTag.PRIMARY ? DensityBuoyancyCommonColors.labelPrimaryProperty : + mass.tag === MassTag.SECONDARY ? DensityBuoyancyCommonColors.labelSecondaryProperty : + new TinyProperty( 'white' ); + + this.tagNodeTexture = createMassTagTexture( DensityBuoyancyCommonStrings.massLabel.secondaryStringProperty, colorProperty ); + const tagHeight = TAG_SCALE * this.tagNodeTexture._height; + this.tagMesh = new TextureQuad( this.tagNodeTexture, TAG_SCALE * this.tagNodeTexture._width, + tagHeight, { + depthTest: true + } ); + + // TODO: Getting closer!!! Start here? + this.tagHeight = tagHeight; } else if ( mass.tag !== MassTag.NONE ) { @@ -94,6 +145,7 @@ } ); this.tagHeight = TAG_SCALE * this.tagNodeTexture._height; + // TODO: Getting closer!!! Or maybe here? this.massNameListener = string => { this.tagNodeTexture!.dispose(); this.tagNodeTexture = MassLabelNode.getBasicLabelTexture( string );
zepumph commented 3 months ago

Good progress!!

```diff Subject: [PATCH] opt out of webglnode --- Index: mobius/js/TextureQuad.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/mobius/js/TextureQuad.ts b/mobius/js/TextureQuad.ts --- a/mobius/js/TextureQuad.ts (revision 40e6d2dec7b9509a42edc8a30f8fa7a7dbf74572) +++ b/mobius/js/TextureQuad.ts (date 1708098451925) @@ -31,6 +31,7 @@ const basicMaterial = new THREE.MeshBasicMaterial( merge( { transparent: true, depthTest: false, + // TODO: using "combineOptions" here says this isn't a known parameter. how? https://github.com/phetsims/density-buoyancy-common/issues/95 map: texture }, materialOptions ) ); Index: density-buoyancy-common/js/common/view/LabelTexture.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/density-buoyancy-common/js/common/view/LabelTexture.ts b/density-buoyancy-common/js/common/view/LabelTexture.ts --- a/density-buoyancy-common/js/common/view/LabelTexture.ts (revision 16fab19b150adff3ca4f7ddf0fc0b3797b32a1ae) +++ b/density-buoyancy-common/js/common/view/LabelTexture.ts (date 1708102970699) @@ -23,6 +23,8 @@ children: [ labelNode ], scale: 2 } ); + + // closest power of 2 is vital for GPU rendering const width = Utils.toPowerOf2( Math.ceil( containerNode.width ) ); const height = Utils.toPowerOf2( Math.ceil( containerNode.height ) ); Index: density-buoyancy-common/js/common/view/MassView.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/density-buoyancy-common/js/common/view/MassView.ts b/density-buoyancy-common/js/common/view/MassView.ts --- a/density-buoyancy-common/js/common/view/MassView.ts (revision 16fab19b150adff3ca4f7ddf0fc0b3797b32a1ae) +++ b/density-buoyancy-common/js/common/view/MassView.ts (date 1708102970695) @@ -16,13 +16,57 @@ import DensityMaterials from './DensityMaterials.js'; import MassLabelNode from './MassLabelNode.js'; import MaterialView from './MaterialView.js'; -import { InteractiveHighlighting, Path } from '../../../../scenery/js/imports.js'; +import { Color, InteractiveHighlighting, Path, Text } from '../../../../scenery/js/imports.js'; import { Shape } from '../../../../kite/js/imports.js'; +import LabelTexture from './LabelTexture.js'; +import { Multilink, TinyProperty } from '../../../../axon/js/imports.js'; +import DensityBuoyancyCommonColors from './DensityBuoyancyCommonColors.js'; +import TReadOnlyProperty from '../../../../axon/js/TReadOnlyProperty.js'; +import DensityBuoyancyCommonConstants from '../DensityBuoyancyCommonConstants.js'; +import PhetFont from '../../../../scenery-phet/js/PhetFont.js'; +import DensityBuoyancyCommonStrings from '../../DensityBuoyancyCommonStrings.js'; +import BackgroundNode from '../../../../scenery-phet/js/BackgroundNode.js'; const TAG_SIZE = 0.03; export const TAG_OFFSET = 0.005; const TAG_SCALE = 0.0005; +const MASS_LABEL_SIZE = 32; + +// Calculated by comparing the original label rectangle size when providing primary/secondary tags +const horiztonalMargin = 15; +const verticalMargin = 5; + +// constants +const tagFont = new PhetFont( { size: 24, weight: 'bold' } ); + +const createMassTagTexture = ( string: TReadOnlyProperty, fill: TReadOnlyProperty ) => { + + const label = new Text( string, { + font: tagFont, + fill: 'white', + maxWidth: 100 + } ); + + // TODO: Width must be rounded down to below 32 or 64 + const rectangle = new BackgroundNode( label, { + xMargin: horiztonalMargin / 2, + yMargin: verticalMargin / 2, + rectangleOptions: { + cornerRadius: DensityBuoyancyCommonConstants.CORNER_RADIUS, + fill: fill, + opacity: 1 + } + } ); + + const texture = new LabelTexture( rectangle ); + + const multilink = Multilink.multilink( [ string, fill ], () => texture.update() ); + texture.disposedEmitter.addListener( () => multilink.dispose() ); + return texture; +}; + + export default abstract class MassView extends THREE.Mesh { public readonly mass: Mass; @@ -31,7 +75,7 @@ private readonly positionListener: () => void; private tagNodeTexture: NodeTexture | null; - private readonly tagMesh: TextureQuad | null; + private tagMesh: TextureQuad | null; // Only set once private readonly massNameListener?: ( string: string ) => void; protected readonly tagHeight: number | null = null; @@ -78,29 +122,37 @@ this.tagHeight = TAG_SIZE; } else if ( mass.tag === MassTag.SECONDARY ) { - this.tagNodeTexture = MassLabelNode.getSecondaryTexture(); + + const colorProperty = mass.tag === MassTag.PRIMARY ? DensityBuoyancyCommonColors.labelPrimaryProperty : + mass.tag === MassTag.SECONDARY ? DensityBuoyancyCommonColors.labelSecondaryProperty : + new TinyProperty( 'white' ); + this.tagNodeTexture = createMassTagTexture( DensityBuoyancyCommonStrings.massLabel.secondaryStringProperty, colorProperty ); + const desiredHorizScale = TAG_SIZE / this.tagNodeTexture._width; + const desiredVertScale = TAG_SIZE / this.tagNodeTexture._height; + const tagWidth = desiredHorizScale * this.tagNodeTexture._width; + const tagHeight = desiredVertScale * this.tagNodeTexture._height; + this.tagMesh = new TextureQuad( this.tagNodeTexture, TAG_SIZE, TAG_SIZE, { depthTest: true } ); + this.tagHeight = TAG_SIZE; } else if ( mass.tag !== MassTag.NONE ) { - - const string = mass.nameProperty.value; - this.tagNodeTexture = MassLabelNode.getBasicLabelTexture( string ); + this.massNameListener = string => { + this.tagNodeTexture && this.tagNodeTexture.dispose(); + this.tagNodeTexture = MassLabelNode.getBasicLabelTexture( string ); - this.tagMesh = new TextureQuad( this.tagNodeTexture, TAG_SCALE * this.tagNodeTexture._width, TAG_SCALE * this.tagNodeTexture._height, { - depthTest: true - } ); - this.tagHeight = TAG_SCALE * this.tagNodeTexture._height; - - this.massNameListener = string => { - this.tagNodeTexture!.dispose(); - this.tagNodeTexture = MassLabelNode.getBasicLabelTexture( string ); - this.tagMesh!.updateTexture( this.tagNodeTexture, TAG_SCALE * this.tagNodeTexture._width, TAG_SCALE * this.tagNodeTexture._height ); - this.tagMesh!.visible = string !== ''; + if ( !this.tagMesh ) { + this.tagMesh = new TextureQuad( this.tagNodeTexture, TAG_SCALE * this.tagNodeTexture._width, TAG_SCALE * this.tagNodeTexture._height, { + depthTest: true + } ); + } + this.tagMesh.updateTexture( this.tagNodeTexture, TAG_SCALE * this.tagNodeTexture._width, TAG_SCALE * this.tagNodeTexture._height ); + this.tagMesh.visible = string !== ''; }; - this.mass.nameProperty.lazyLink( this.massNameListener ); + this.mass.nameProperty.link( this.massNameListener ); + this.tagHeight = TAG_SCALE * this.tagNodeTexture!._height; } if ( this.tagMesh ) {
zepumph commented 3 months ago

Close to commit, just need to clean up other usages, and trim the margins for extra wide tags

```diff Subject: [PATCH] Scenery to support TinyProperty for paintable things (and generally use isTReadOnlyProperty), https://github.com/phetsims/scenery/issues/1612 --- Index: js/common/view/MassView.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/common/view/MassView.ts b/js/common/view/MassView.ts --- a/js/common/view/MassView.ts (revision 8ac45e56681cc23ff9fe3938f6b86e5aaeb3c148) +++ b/js/common/view/MassView.ts (date 1708128546032) @@ -16,12 +16,48 @@ import DensityMaterials from './DensityMaterials.js'; import MassLabelNode from './MassLabelNode.js'; import MaterialView from './MaterialView.js'; -import { InteractiveHighlighting, Path } from '../../../../scenery/js/imports.js'; +import { Color, InteractiveHighlighting, Path, Text } from '../../../../scenery/js/imports.js'; import { Shape } from '../../../../kite/js/imports.js'; +import LabelTexture from './LabelTexture.js'; +import { Multilink, TinyProperty, UnknownMultilink } from '../../../../axon/js/imports.js'; +import DensityBuoyancyCommonColors from './DensityBuoyancyCommonColors.js'; +import TReadOnlyProperty from '../../../../axon/js/TReadOnlyProperty.js'; +import DensityBuoyancyCommonConstants from '../DensityBuoyancyCommonConstants.js'; +import PhetFont from '../../../../scenery-phet/js/PhetFont.js'; +import DensityBuoyancyCommonStrings from '../../DensityBuoyancyCommonStrings.js'; +import BackgroundNode from '../../../../scenery-phet/js/BackgroundNode.js'; const TAG_SIZE = 0.03; export const TAG_OFFSET = 0.005; -const TAG_SCALE = 0.0005; +const TAG_SCALE_NEW = 0.00046875; + +// Calculated by comparing the original label rectangle size when providing primary/secondary tags +const horiztonalMargin = 14; +const verticalMargin = 5; + +// constants +const tagFont = new PhetFont( { size: 24, weight: 'bold' } ); + +const createMassTagTexture = ( string: TReadOnlyProperty, fill: TReadOnlyProperty ): NodeTexture => { + const label = new Text( string, { + font: tagFont, + fill: Color.getLuminance( fill.value ) > ( 255 / 2 ) ? 'black' : 'white', // best guess? + maxWidth: 100 + } ); + + const rectangle = new BackgroundNode( label, { + xMargin: horiztonalMargin / 2, + yMargin: verticalMargin / 2, + rectangleOptions: { + cornerRadius: DensityBuoyancyCommonConstants.CORNER_RADIUS, + fill: fill, + opacity: 1 + } + } ); + + return new LabelTexture( rectangle ); +}; + export default abstract class MassView extends THREE.Mesh { @@ -31,9 +67,10 @@ private readonly positionListener: () => void; private tagNodeTexture: NodeTexture | null; - private readonly tagMesh: TextureQuad | null; - private readonly massNameListener?: ( string: string ) => void; + private tagMesh: TextureQuad | null; // Only set once + private readonly massTagMultilink?: UnknownMultilink; + // TODO: how is this used? Couldn't it potentially change as the string changes? https://github.com/phetsims/buoyancy/issues/96 protected readonly tagHeight: number | null = null; protected readonly tagOffsetProperty: Property = new Property( Vector3.ZERO ); @@ -67,40 +104,49 @@ this.mass.transformedEmitter.addListener( this.positionListener ); this.positionListener(); + // TODO: factor out as a new MassTag type, https://github.com/phetsims/buoyancy/issues/96 this.tagNodeTexture = null; this.tagMesh = null; if ( mass.tag === MassTag.PRIMARY ) { + // TODO: delete all the stuff in MassLabelNode, https://github.com/phetsims/buoyancy/issues/96 this.tagNodeTexture = MassLabelNode.getPrimaryTexture(); this.tagMesh = new TextureQuad( this.tagNodeTexture, TAG_SIZE, TAG_SIZE, { depthTest: true } ); this.tagHeight = TAG_SIZE; } - else if ( mass.tag === MassTag.SECONDARY ) { - this.tagNodeTexture = MassLabelNode.getSecondaryTexture(); - this.tagMesh = new TextureQuad( this.tagNodeTexture, TAG_SIZE, TAG_SIZE, { - depthTest: true - } ); - this.tagHeight = TAG_SIZE; - } else if ( mass.tag !== MassTag.NONE ) { - const string = mass.nameProperty.value; - this.tagNodeTexture = MassLabelNode.getBasicLabelTexture( string ); + const colorProperty = mass.tag === MassTag.PRIMARY ? DensityBuoyancyCommonColors.labelPrimaryProperty : + mass.tag === MassTag.SECONDARY ? DensityBuoyancyCommonColors.labelSecondaryProperty : + new TinyProperty( Color.white ); + const nameProperty = mass.tag === MassTag.PRIMARY ? DensityBuoyancyCommonStrings.massLabel.primaryStringProperty : + mass.tag === MassTag.SECONDARY ? DensityBuoyancyCommonStrings.massLabel.secondaryStringProperty : + mass.nameProperty; + + this.tagNodeTexture = createMassTagTexture( nameProperty, colorProperty ); - this.tagMesh = new TextureQuad( this.tagNodeTexture, TAG_SCALE * this.tagNodeTexture._width, TAG_SCALE * this.tagNodeTexture._height, { + this.tagHeight = TAG_SCALE_NEW * this.tagNodeTexture._height; + + const tagWidth = TAG_SCALE_NEW * this.tagNodeTexture._width; + const tagHeight = TAG_SCALE_NEW * this.tagNodeTexture._height; + + // TODO: Min as a square, and then otherwise we can tighten up the width margin, https://github.com/phetsims/buoyancy/issues/96 + this.tagMesh = new TextureQuad( this.tagNodeTexture, tagWidth, tagHeight, { depthTest: true } ); - this.tagHeight = TAG_SCALE * this.tagNodeTexture._height; + + this.massTagMultilink = new Multilink( [ nameProperty, colorProperty ], string => { + const texture = this.tagNodeTexture!; + texture.update(); - this.massNameListener = string => { - this.tagNodeTexture!.dispose(); - this.tagNodeTexture = MassLabelNode.getBasicLabelTexture( string ); - this.tagMesh!.updateTexture( this.tagNodeTexture, TAG_SCALE * this.tagNodeTexture._width, TAG_SCALE * this.tagNodeTexture._height ); + const tagWidth = TAG_SCALE_NEW * texture._width; + const tagHeight = TAG_SCALE_NEW * texture._height; + + this.tagMesh!.updateTexture( texture, tagWidth, tagHeight ); this.tagMesh!.visible = string !== ''; - }; - this.mass.nameProperty.lazyLink( this.massNameListener ); + } ); } if ( this.tagMesh ) { @@ -108,6 +154,7 @@ this.tagMesh.renderOrder = 1; this.tagOffsetProperty.link( offset => { + // TODO: why this magic number? Is it needed everywhere? https://github.com/phetsims/density-buoyancy-common/issues/95 this.tagMesh!.position.set( offset.x, offset.y, offset.z + 0.0001 ); } ); } @@ -131,8 +178,8 @@ this.focusablePath.dispose(); - if ( this.massNameListener ) { - this.mass.nameProperty.unlink( this.massNameListener ); + if ( this.massTagMultilink ) { + this.massTagMultilink.dispose(); } this.tagNodeTexture && this.tagNodeTexture.dispose();
zepumph commented 3 months ago

All the above work in patches for changing the mass tag color will be done up in https://github.com/phetsims/buoyancy/issues/102. All that is left here is:

Add the Density panel (close by default) like is shown in the following mockups

@AgustinVallejo would you like to take this on and let me know if you have any questions?

zepumph commented 2 months ago

All that is left here is the density panel for the intro screen.

zepumph commented 2 months ago

I think the best way to do this is to try to make DensityReadoutNode a more reusable component. I'll take a look. I think this also related to #112 because that view is set up kinda the same way (mass-> data), vs material->data which is directly gathered from the mass.materialProperty. I'll take a look (maybe in a separate issue). Tagging @AgustinVallejo so that he knows where all the work for the density readout is going.

zepumph commented 2 months ago

[ ] Add the Density panel (close by default) like is shown in the following mockups:

This will be much easy after we do https://github.com/phetsims/density-buoyancy-common/issues/103.

zepumph commented 2 months ago

https://github.com/phetsims/density-buoyancy-common/issues/103 is now far enough along to add the density panel on the right side.

UPDATE: I'm actually going to wait a bit longer on this one, as it has a very similar pattern to the explore screen, which is still getting settled.

zepumph commented 2 months ago

For added density/submerged panels in the intro screen:

zepumph commented 2 months ago

In terms of calculating how much width we have on the right, I tried this patch, but I'm running into a strange startup state for three js, since the dimension of the state is 0x0. I will need to reach out to JO

``` Subject: [PATCH] update important doc about Node.changedInstanceEmitter, https://github.com/phetsims/scenery/issues/1615 --- Index: js/common/view/DensityBuoyancyScreenView.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/common/view/DensityBuoyancyScreenView.ts b/js/common/view/DensityBuoyancyScreenView.ts --- a/js/common/view/DensityBuoyancyScreenView.ts (revision 614a62ffd57852efa44044fc0b8379d47f803099) +++ b/js/common/view/DensityBuoyancyScreenView.ts (date 1710457088177) @@ -777,6 +777,9 @@ * Projects a 3d model point to a 2d view point (in the screen view's coordinate frame). */ public modelToViewPoint( point: Vector3 ): Vector2 { + // TODO: how is it ok to get a view point when there isn't anything drawn? JO!!!?!? + assert && assert( this.sceneNode.stage.canvasHeight !== 0 && this.sceneNode.stage.canvasWidth !== 0, 'model to view where the three stage is not drawn' ); + // We'll want to transform global coordinates into screen coordinates here return this.parentToLocalPoint( animatedPanZoomSingleton.listener.matrixProperty.value.inverted().timesVector2( this.sceneNode.projectPoint( point ) ) ); } Index: js/buoyancy/view/BuoyancyIntroScreenView.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/buoyancy/view/BuoyancyIntroScreenView.ts b/js/buoyancy/view/BuoyancyIntroScreenView.ts --- a/js/buoyancy/view/BuoyancyIntroScreenView.ts (revision 614a62ffd57852efa44044fc0b8379d47f803099) +++ b/js/buoyancy/view/BuoyancyIntroScreenView.ts (date 1710457026141) @@ -127,13 +127,20 @@ margin: MARGIN } ) ); - - // Materials are set in densityBox.setMaterials() below + // TODO: but this will change all the time. We really want to somehow get an ideal when the visible bounds are the same as the layout bounds. + // const leftSideOfPoolViewPoint = this.modelToViewPoint( new Vector3( model.pool.bounds.maxX, model.pool.bounds.centerY, model.pool.bounds.maxZ ) ); + // const availableRightSpace = this.layoutBounds.right - leftSideOfPoolViewPoint.x; + // const maxRightContentWidth = availableRightSpace - 2 * MARGIN; + // console.log( maxRightContentWidth ); + // // Materials are set in densityBox.setMaterials() below const densityBox = new DensityAccordionBox( { - expandedProperty: model.densityExpandedProperty + expandedProperty: model.densityExpandedProperty, + contentWidthMax: 100 } ); - const submergedBox = new SubmergedAccordionBox( model.gravityProperty, model.liquidMaterialProperty ); + const submergedBox = new SubmergedAccordionBox( model.gravityProperty, model.liquidMaterialProperty, { + contentWidthMax: 100 + } ); // Adjust the visibility after, since we want to size the box's location for its "full" bounds // This instance lives for the lifetime of the simulation, so we don't need to remove this listener
zepumph commented 2 months ago

After discussing with @AgustinVallejo, we created https://github.com/phetsims/density-buoyancy-common/issues/104 to investigate modelToViewPoint, but for now I would like to proceed with making the max content width a Property for ReadoutAccordionBox.

zepumph commented 2 months ago

From #112, @DianaTavares had some ideas for this:

zepumph commented 2 months ago

Getting close:

```diff Subject: [PATCH] working on in! --- Index: js/buoyancy/view/ReadoutListAccordionBox.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/buoyancy/view/ReadoutListAccordionBox.ts b/js/buoyancy/view/ReadoutListAccordionBox.ts --- a/js/buoyancy/view/ReadoutListAccordionBox.ts (revision bb961484e90f772ccdc0d6bff038b3ea022f9b5a) +++ b/js/buoyancy/view/ReadoutListAccordionBox.ts (date 1711392567176) @@ -10,7 +10,7 @@ import TReadOnlyProperty from '../../../../axon/js/TReadOnlyProperty.js'; import PhetFont from '../../../../scenery-phet/js/PhetFont.js'; -import { HBox, RichText, RichTextOptions, Text, TextOptions, VBox } from '../../../../scenery/js/imports.js'; +import { HBox, RichText, RichTextOptions, Text, VBox } from '../../../../scenery/js/imports.js'; import densityBuoyancyCommon from '../../densityBuoyancyCommon.js'; import TinyEmitter from '../../../../axon/js/TinyEmitter.js'; import DensityBuoyancyCommonConstants from '../../common/DensityBuoyancyCommonConstants.js'; @@ -18,14 +18,20 @@ import AccordionBox, { AccordionBoxOptions } from '../../../../sun/js/AccordionBox.js'; import DensityBuoyancyCommonStrings from '../../DensityBuoyancyCommonStrings.js'; import PatternStringProperty from '../../../../axon/js/PatternStringProperty.js'; +import TinyProperty from '../../../../axon/js/TinyProperty.js'; const DEFAULT_FONT = new PhetFont( 14 ); const HBOX_SPACING = 5; const DEFAULT_CONTENT_WIDTH = ( 200 + HBOX_SPACING ) / 2; +const TEXT_OPTIONS = { + font: DEFAULT_FONT +}; + type SelfOptions = { + // Provide the ideal max content width for the accordion box content. This is used to apply maxWidths to the Texts of the readout. - contentWidthMax?: number; + contentWidthMax?: number | TReadOnlyProperty; readoutItems?: ReadoutItemOptions[]; }; @@ -49,22 +55,22 @@ export default abstract class ReadoutListAccordionBox extends AccordionBox { protected cleanupEmitter = new TinyEmitter(); - protected textOptions: TextOptions = {}; protected readonly readoutBox: VBox; - protected readonly contentWidthMax: number; + protected readonly contentWidthMaxProperty: TReadOnlyProperty; public constructor( titleStringProperty: TReadOnlyProperty, providedOptions?: ReadoutListAccordionBoxOptions ) { + const titleNode = new Text( titleStringProperty, { + font: DensityBuoyancyCommonConstants.TITLE_FONT + } ); + const options = optionize4, SelfOptions, AccordionBoxOptions>()( {}, DensityBuoyancyCommonConstants.ACCORDION_BOX_OPTIONS, { - titleNode: new Text( titleStringProperty, { - font: DensityBuoyancyCommonConstants.TITLE_FONT, - maxWidth: providedOptions?.contentWidthMax || DEFAULT_CONTENT_WIDTH - } ), + titleNode: titleNode, layoutOptions: { stretch: true }, contentWidthMax: DEFAULT_CONTENT_WIDTH, readoutItems: [] @@ -78,12 +84,13 @@ super( readoutBox, options ); this.readoutBox = readoutBox; - this.contentWidthMax = options.contentWidthMax; + this.contentWidthMaxProperty = typeof options.contentWidthMax === 'number' ? + new TinyProperty( options.contentWidthMax ) : + options.contentWidthMax; - this.textOptions = { - font: DEFAULT_FONT, - maxWidth: ( this.contentWidthMax - HBOX_SPACING ) / 2 - }; + const maxWidthListener = ( maxWidth: number ) => { titleNode.maxWidth = maxWidth * 0.9; }; + this.contentWidthMaxProperty.link( maxWidthListener ); + this.disposeEmitter.addListener( () => this.contentWidthMaxProperty.unlink( maxWidthListener ) ); } public setReadoutItems( readoutItems: ReadoutItemOptions[] ): void { @@ -100,12 +107,21 @@ DensityBuoyancyCommonStrings.nameColonPatternStringProperty, { name: nameProperty } ); - const labelText = new RichText( nameColonProperty, this.textOptions ); + + const labelText = new RichText( nameColonProperty, TEXT_OPTIONS ); const readoutFormat = readoutItem.readoutFormat ? readoutItem.readoutFormat : {}; const valueText = new RichText( readoutData.valueProperty, - combineOptions( {}, this.textOptions, readoutFormat ) ); + combineOptions( {}, TEXT_OPTIONS, readoutFormat ) ); + + const maxWidthListener = ( contentWidthMax: number ) => { + const maxWidth = ( contentWidthMax - HBOX_SPACING ) / 2; //!!!!!!! TODO: this doesn't account for the panel margins, so you need to in the intro screen proper. + labelText.maxWidth = maxWidth; + valueText.maxWidth = maxWidth; + }; + this.contentWidthMaxProperty.link( maxWidthListener ); this.cleanupEmitter.addListener( () => { + this.contentWidthMaxProperty.unlink( maxWidthListener ); valueText.dispose(); labelText.dispose(); nameColonProperty.dispose(); Index: js/buoyancy/view/BuoyancyIntroScreenView.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/buoyancy/view/BuoyancyIntroScreenView.ts b/js/buoyancy/view/BuoyancyIntroScreenView.ts --- a/js/buoyancy/view/BuoyancyIntroScreenView.ts (revision bb961484e90f772ccdc0d6bff038b3ea022f9b5a) +++ b/js/buoyancy/view/BuoyancyIntroScreenView.ts (date 1711401249408) @@ -24,6 +24,8 @@ import DensityAccordionBox from './DensityAccordionBox.js'; import SubmergedAccordionBox from './SubmergedAccordionBox.js'; import PatternStringProperty from '../../../../axon/js/PatternStringProperty.js'; +import Property from '../../../../axon/js/Property.js'; +import Bounds2 from '../../../../dot/js/Bounds2.js'; // constants const blockSetStringMap = { @@ -50,6 +52,11 @@ export default class BuoyancyIntroScreenView extends DensityBuoyancyScreenView { + // TODO:rename widthOfRightSideProperty? + // TODO: have a hard coded max for really wide aspect ratios + // + private readonly maxWidthProperty = new Property( 200 ); + public constructor( model: BuoyancyIntroModel, options: DensityBuoyancyScreenViewOptions ) { super( model, combineOptions( { @@ -131,10 +138,13 @@ // Materials are set in densityBox.setMaterials() below const densityBox = new DensityAccordionBox( { - expandedProperty: model.densityExpandedProperty + expandedProperty: model.densityExpandedProperty, + contentWidthMax: this.maxWidthProperty } ); - const submergedBox = new SubmergedAccordionBox( model.gravityProperty, model.liquidMaterialProperty ); + const submergedBox = new SubmergedAccordionBox( model.gravityProperty, model.liquidMaterialProperty, { + contentWidthMax: this.maxWidthProperty + } ); // Adjust the visibility after, since we want to size the box's location for its "full" bounds // This instance lives for the lifetime of the simulation, so we don't need to remove this listener @@ -176,6 +186,17 @@ this.addChild( this.popupLayer ); } + + public override layout( viewBounds: Bounds2 ): void { + super.layout( viewBounds ); + + // TODO: own function please + // TODO: fix margins + const rightSideOfPoolViewPoint = this.modelToViewPoint( new Vector3( this.model.pool.bounds.maxX, this.model.pool.bounds.centerY, this.model.pool.bounds.maxZ ) ); + const availableRightSpace = this.visibleBoundsProperty.value.right - rightSideOfPoolViewPoint.x; + console.log( this.visibleBoundsProperty.value.right, '-', rightSideOfPoolViewPoint.x ); + this.maxWidthProperty.value = availableRightSpace - 2 * MARGIN; + } } densityBuoyancyCommon.register( 'BuoyancyIntroScreenView', BuoyancyIntroScreenView ); \ No newline at end of file
zepumph commented 2 months ago

Ok. I believe that everything here is now done. Please give this a review, noting that since this issue was created, the % submerged panel was added over in https://github.com/phetsims/buoyancy/issues/112, but still has some work to be done (off by default from the preferences, etc).

Let me know what your think!

DianaTavares commented 2 months ago

Looks amazing, thanks!!!