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

Streamline image generation and checking #141

Closed samreid closed 3 weeks ago

samreid commented 1 month ago

Discovered in #48 it would be good to streamline the code that generates images. There is a lot of duplicated code in the existing implementation. I recommend the following:

samreid commented 1 month ago

Here is a proposal that addresses the recommendations above. I noted that it yielded a slightly different boat and bottle when I ran it. I wonder if they have changed or maybe it is platform dependent.

I'd like to check in with @zepumph before taking this idea to production.

Also, do we want the PNGs upside down? (they are currently upside down). If not, we can open a side issue for that.

```diff Subject: [PATCH] Factor out BackgroundEventTargetListener.ts, see https://github.com/phetsims/buoyancy/issues/104 --- Index: js/common/view/GeneratedImage.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/common/view/GeneratedImage.ts b/js/common/view/GeneratedImage.ts new file mode 100644 --- /dev/null (date 1713327870895) +++ b/js/common/view/GeneratedImage.ts (date 1713327870895) @@ -0,0 +1,52 @@ +// Copyright 2024, University of Colorado Boulder + +/** + * Many images in Density, Buoyancy, Buoyancy Basics and related simulations are generated programmatically using + * three.js. Then they are stored on the disk as PNGs to make loading faster and to reduce the resources used. + * This central point allows us to compare the generated images with the stored images to ensure that they are the same. + * + * @author Sam Reid (PhET Interactive Simulations) + */ + +import densityBuoyancyCommon from '../../densityBuoyancyCommon.js'; +import { Image } from '../../../../scenery/js/imports.js'; +import DensityBuoyancyCommonQueryParameters from '../DensityBuoyancyCommonQueryParameters.js'; +import Vector2 from '../../../../dot/js/Vector2.js'; + +export default class GeneratedImage { + public static getImage( name: string, png: HTMLImageElement, scale: Vector2, generate: () => { image: Image; url: string } ): Image { + if ( DensityBuoyancyCommonQueryParameters.generateIconImages ) { + + const generated = generate(); + + if ( generated.url !== png.src ) { + console.log( 'Image mismatch: ' + name ); + console.log( 'to update the PNG, run the following command:' ); + + const command = `const fs = require('fs'); + +// Your Base64 string +let base64String = "${generated.url}"; + +// Remove data:image/png;base64, if present +const matches = base64String.match(/^data:([A-Za-z-+/]+);base64,(.+)$/); +if (matches) { + base64String = matches[2]; +} + +// Convert Base64 string to binary data +const imageBuffer = Buffer.from(base64String, 'base64'); + +// Save the binary data as a PNG file +fs.writeFileSync('output.png', imageBuffer);`; + + console.log( command ); + } + } + return new Image( png, { + scale: scale + } ); + } +} + +densityBuoyancyCommon.register( 'GeneratedImage', GeneratedImage ); \ No newline at end of file Index: js/density/view/DensityIntroScreenView.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/density/view/DensityIntroScreenView.ts b/js/density/view/DensityIntroScreenView.ts --- a/js/density/view/DensityIntroScreenView.ts (revision 27e2f87bd1833b1bae11515777e63c3f6323329d) +++ b/js/density/view/DensityIntroScreenView.ts (date 1713326023117) @@ -146,7 +146,7 @@ const water = new THREE.Mesh( waterGeometry, waterMaterial ); water.position.copy( ThreeUtils.vectorToThree( new Vector3( 0, -0.5, 0 ) ) ); scene.add( water ); - } ); + } ).image; } } Index: js/buoyancy/view/BuoyancyApplicationsScreenView.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/buoyancy/view/BuoyancyApplicationsScreenView.ts b/js/buoyancy/view/BuoyancyApplicationsScreenView.ts --- a/js/buoyancy/view/BuoyancyApplicationsScreenView.ts (revision 27e2f87bd1833b1bae11515777e63c3f6323329d) +++ b/js/buoyancy/view/BuoyancyApplicationsScreenView.ts (date 1713327988691) @@ -42,6 +42,7 @@ import boat_icon_png from '../../../images/boat_icon_png.js'; import Vector2 from '../../../../dot/js/Vector2.js'; import SubmergedAccordionBox from './SubmergedAccordionBox.js'; +import GeneratedImage from '../../common/view/GeneratedImage.js'; // constants const MARGIN = DensityBuoyancyCommonConstants.MARGIN; @@ -355,39 +356,22 @@ this.positionResetSceneButton(); } - public static getBoatIcon(): Node { - if ( DensityBuoyancyCommonQueryParameters.generateIconImages ) { - if ( !ThreeUtils.isWebGLEnabled() ) { - return DensityBuoyancyScreenView.getFallbackIcon(); - } - - const angledIcon = DensityBuoyancyScreenView.getAngledIcon( 6, new Vector3( -0.03, 0, 0 ), scene => { - scene.add( BoatView.getBoatDrawingData().group ); - }, null ); - angledIcon.setScaleMagnitude( ICON_SCALE ); - return angledIcon; - } - else { - return new Image( boat_icon_png, { scale: ICON_IMAGE_SCALE } ); - } + private static getBoatIcon(): Node { + return GeneratedImage.getImage( 'boat_icon_png', boat_icon_png, ICON_IMAGE_SCALE, + () => DensityBuoyancyScreenView.getAngledIcon( + 6, + new Vector3( -0.03, 0, 0 ), + scene => scene.add( BoatView.getBoatDrawingData().group ), null ) + ); } - - public static getBottleIcon(): Node { - if ( DensityBuoyancyCommonQueryParameters.generateIconImages ) { - if ( !ThreeUtils.isWebGLEnabled() ) { - return DensityBuoyancyScreenView.getFallbackIcon(); - } - // Hard coded zoom and view-port vector help to center the icon. - const angledIcon = DensityBuoyancyScreenView.getAngledIcon( 3.4, new Vector3( -0.02, 0, 0 ), scene => { - scene.add( BottleView.getBottleDrawingData().group ); - }, null ); - angledIcon.setScaleMagnitude( ICON_SCALE ); - return angledIcon; - } - else { - return new Image( bottle_icon_png, { scale: ICON_IMAGE_SCALE } ); - } + private static getBottleIcon(): Node { + return GeneratedImage.getImage( 'bottle_icon_png', bottle_icon_png, ICON_IMAGE_SCALE, + () => DensityBuoyancyScreenView.getAngledIcon( + 3.4, + new Vector3( -0.02, 0, 0 ), + scene => scene.add( BottleView.getBottleDrawingData().group ), null ) + ); } } Index: js/density/view/DensityMysteryScreenView.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/density/view/DensityMysteryScreenView.ts b/js/density/view/DensityMysteryScreenView.ts --- a/js/density/view/DensityMysteryScreenView.ts (revision 27e2f87bd1833b1bae11515777e63c3f6323329d) +++ b/js/density/view/DensityMysteryScreenView.ts (date 1713326023123) @@ -177,7 +177,7 @@ scale.position.copy( ThreeUtils.vectorToThree( new Vector3( 0, -0.03, 0 ) ) ); scene.add( scale ); - } ); + } ).image; } } Index: js/density/view/DensityCompareScreenView.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/density/view/DensityCompareScreenView.ts b/js/density/view/DensityCompareScreenView.ts --- a/js/density/view/DensityCompareScreenView.ts (revision 27e2f87bd1833b1bae11515777e63c3f6323329d) +++ b/js/density/view/DensityCompareScreenView.ts (date 1713326023113) @@ -221,7 +221,7 @@ const water = new THREE.Mesh( waterGeometry, waterMaterial ); water.position.copy( ThreeUtils.vectorToThree( new Vector3( 0, -0.5, 0 ) ) ); scene.add( water ); - } ); + } ).image; } } Index: js/buoyancy/view/BuoyancyLabScreenView.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/buoyancy/view/BuoyancyLabScreenView.ts b/js/buoyancy/view/BuoyancyLabScreenView.ts --- a/js/buoyancy/view/BuoyancyLabScreenView.ts (revision 27e2f87bd1833b1bae11515777e63c3f6323329d) +++ b/js/buoyancy/view/BuoyancyLabScreenView.ts (date 1713326023121) @@ -211,7 +211,7 @@ scale.position.copy( ThreeUtils.vectorToThree( new Vector3( 0, 0.25, 0 ) ) ); scene.add( scale ); - }, null ); + }, null ).image; // } // else { // image = new Image( fluid_displaced_scale_icon_png ); 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 27e2f87bd1833b1bae11515777e63c3f6323329d) +++ b/js/common/view/DensityBuoyancyScreenView.ts (date 1713326881666) @@ -708,7 +708,10 @@ /** * Returns an icon for selection, given a scene setup callback. */ - protected static getAngledIcon( zoom: number, lookAt: Vector3, setupScene: ( scene: THREE.Scene ) => void, background: THREE.Color | null = new THREE.Color( 0xffffff ) ): Node { + protected static getAngledIcon( zoom: number, lookAt: Vector3, setupScene: ( scene: THREE.Scene ) => void, background: THREE.Color | null = new THREE.Color( 0xffffff ) ): { + image: Image, + url: string + } { const width = Screen.MINIMUM_HOME_SCREEN_ICON_SIZE.width; const height = Screen.MINIMUM_HOME_SCREEN_ICON_SIZE.height; @@ -746,10 +749,9 @@ stage.dispose(); - // Yes, we log this so we can regenerate them if we have changes - console.log( canvas.toDataURL() ); + const dataURL = canvas.toDataURL(); - const image = new Image( canvas.toDataURL(), { + const image = new Image( dataURL, { mipmap: true, initialWidth: canvas.width, initialHeight: canvas.height @@ -757,7 +759,8 @@ image.setScaleMagnitude( 1, -1 ); image.left = 0; image.top = 0; - return image; + + return { image: image, url: dataURL }; } /**
samreid commented 1 month ago

@zepumph and I discussed. We agreed:

```diff Subject: [PATCH] Make sure three.js coordinate transform is available during startup, see https://github.com/phetsims/density-buoyancy-common/issues/113 --- 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 252e2a1e9fa9f7caf688ba610bf4f3183a1aa51f) +++ b/js/common/view/DensityBuoyancyScreenView.ts (date 1713379596530) @@ -753,7 +753,8 @@ stage.dispose(); // Yes, we log this so we can regenerate them if we have changes - console.log( canvas.toDataURL() ); + console.log('upside down') + console.log( 'test: '+canvas.toDataURL() ); const image = new Image( canvas.toDataURL(), { mipmap: true, @@ -763,6 +764,13 @@ image.setScaleMagnitude( 1, -1 ); image.left = 0; image.top = 0; + + + const flippedDataURL = image.rasterized( { wrap: false } ).image; + console.log('upside up') + console.log( 'test2 '+flippedDataURL.src.substring(8) ); + + return image; }
samreid commented 4 weeks ago

In the preceding commit I manually flipped the images via Photoshop => Image => Image Rotation => Flip Canvas Vertical.

Next I flipped the camera upside down for creating the images like this:

```diff Subject: [PATCH] Vertically flip boat and bottle icon images, see https://github.com/phetsims/buoyancy/issues/141 --- Index: js/buoyancy/view/BuoyancyApplicationsScreenView.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/buoyancy/view/BuoyancyApplicationsScreenView.ts b/js/buoyancy/view/BuoyancyApplicationsScreenView.ts --- a/js/buoyancy/view/BuoyancyApplicationsScreenView.ts (revision 784c12808e75c92f47df7bc1375991e5d1efecde) +++ b/js/buoyancy/view/BuoyancyApplicationsScreenView.ts (date 1713486270357) @@ -357,9 +357,11 @@ public static getBoatIcon(): Node { if ( DensityBuoyancyCommonQueryParameters.generateIconImages && ThreeUtils.isWebGLEnabled() ) { + console.log( 'getting boat icon' ); const angledIcon = DensityBuoyancyScreenView.getAngledIcon( 6, new Vector3( -0.03, 0, 0 ), scene => { scene.add( BoatView.getBoatDrawingData().group ); }, null ); + console.log( 'got boat' ); angledIcon.setScaleMagnitude( ICON_SCALE ); return angledIcon; } 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 784c12808e75c92f47df7bc1375991e5d1efecde) +++ b/js/common/view/DensityBuoyancyScreenView.ts (date 1713486982157) @@ -756,6 +756,8 @@ stage.threeCamera.position.copy( ThreeUtils.vectorToThree( new Vector3( 0, 0.4, 1 ) ) ); stage.threeCamera.zoom = zoom; + // Modify the camera's up vector to flip the view vertically + stage.threeCamera.up.set( 0, -1, 0 ); stage.threeCamera.lookAt( ThreeUtils.vectorToThree( lookAt ) ); stage.threeCamera.updateProjectionMatrix(); @@ -764,6 +766,7 @@ stage.threeCamera.fov = 50; stage.threeCamera.aspect = width / height; stage.setDimensions( width, height ); + stage.threeCamera.updateProjectionMatrix(); stage.render( undefined ); @@ -772,17 +775,13 @@ stage.dispose(); // Yes, we log this so we can regenerate them if we have changes - console.log( canvas.toDataURL() ); + console.log( canvas.toDataURL() ); // <---- RIGHT HERE - const image = new Image( canvas.toDataURL(), { + return new Image( canvas.toDataURL(), { mipmap: true, initialWidth: canvas.width, initialHeight: canvas.height } ); - image.setScaleMagnitude( 1, -1 ); - image.left = 0; - image.top = 0; - return image; } } ```

The boat comes out upside up, but unfortunately it is facing right.

image

@zepumph is it OK to do Photoshop => Image => Image Rotation => Flip Canvas Vertical as needed, or should we flip/scale the canvas?

zepumph commented 4 weeks ago

This batch of code flips the image correctly, but returns the new canvas. I think we can just reuse the same canvas:


    // Create a temp canvas to store our data (because we need to clear the other box after rotation.
    const tempCanvas = document.createElement( 'canvas' );
    const tempCtx = tempCanvas.getContext( '2d' )!;

    tempCanvas.width = canvas.width;
    tempCanvas.height = canvas.height;

    tempCtx.translate( canvas.width, canvas.height );
    tempCtx.scale( 1, -1 );
    tempCtx.drawImage( canvas, canvas.width * -1, 0 );

    return tempCanvas;
zepumph commented 4 weeks ago

I believe that I have made the canvas right side up before using (either printing or as the SCENERY/Image). I prefer this to a manual step, and I was glad for the challenge of supporting this via canvas (a tech I'm not too used to).

Please review my commit. Can you take it from here? Do all images need to be regenerated?

zepumph commented 3 weeks ago

I talked things through with @samreid, and we were happy with the above improvements. Thanks for all your work here @samreid. Closing. We can pick things up over in https://github.com/phetsims/buoyancy/issues/48