iPad: WebGL support #316
Labels
Comments
|
@samreid in FEL & GasProps, we had to detect mobile Safari and set Edit: I asked for the underlying scenery issue to be prioritized on the |
|
This looks like a slightly different cause than in FEL / GasProps, since we're using three.js. This issue is my highest priority, I just had vacation, and it's a very tricky to reproduce thing for FEL. I believe this is likely related to the memory consumption of having a number of ThreeStages (in WebGL, they all need to have different memory and resources). Launching with I believe Buoyancy might be using more textures and data than Density. I think it will be necessary to create just a single ThreeIsometricNode, to pass back-and-forth between screens. Thus we'd want to score each screen's three.js content in a Three.Object3D, and add/remove it so it is just the content of the screen that is active. Additionally, the camera and projection matrices would presumably need to be updated. Alternatively, it would be possible to explore reducing the size of included resources (but that probably wouldn't help as much). @samreid thoughts? Should we collaborate on the suggested approach? |
|
Testing on iPad 7 running iPadOS 17.1.2, on a built version of buoyancy with ?fuzz&screens=1, it crashed at around:
Testing published density (all 3 screens) with fuzz, it crashes in:
So if a 2m 30s fuzz crash is on production, perhaps we just need to do that well or better? UPDATE: while tethered to safari, published density with all 3 screens fuzzed > 10 minutes with no crash. |
Closed
|
When crashing in a webview on tethered xcode, I get these errors: |
|
Attaching to the memory profiler in xcode debugger shows the memory keeps climbing. I wonder if we should fix the memory leaks before testing this further? 
|
|
After fixing #168, I built buoyancy_en.html, and tested it on my iPad 7. I used it for a full 6 minutes of interacting with every component ("manual" "fuzzing" as best I could), and it did not crash. We also saw that the behavior was corrected on the iphone after fixing memory leaks. However, fuzzing crashed it in 24 seconds in the 1st run and 25 seconds in the 2nd run. |
|
Tethering to safari is very flaky, even for one screen fuzzing, but I managed to get this timeline memory snapshot: 
|
|
Looks like every now and then there is a large memory spike and long GC time: 
|
|
Oh, it's just every 10 seconds when it takes a snapshot. |
|
Tethering to xcode, and fuzzing all screens of built buoyancy at fuzzRate=5, it seems there is a leak in the "other processes", which gained 400MB over 30 seconds: 
|
|
Using "Instruments" we see that VM: IOSurface is leaking rapidly: 
|
|
Here is the caller of those allocations: 
|
|
Here is the stack: 
|
|
In text |
|
Running with ?screens=1&fuzz&fuzzEvents=5 still shows the same leak, so likely reusing the same three.js canvas won't solve the problem: 
|
|
More discoveries:
|
|
Surprisingly, removing all Sprites from FEL still shows a lot of churn in Total Bytes, but stabilizes in persistent bytes 182MB: Subject: [PATCH] Standardize author annotations, see https://github.com/phetsims/density-buoyancy-common/issues/334
---
Index: js/pickup-coil/view/PickupCoilScreenView.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/pickup-coil/view/PickupCoilScreenView.ts b/js/pickup-coil/view/PickupCoilScreenView.ts
--- a/js/pickup-coil/view/PickupCoilScreenView.ts (revision bfc63e82a8bf2ee9233675f49661a985e6edde6f)
+++ b/js/pickup-coil/view/PickupCoilScreenView.ts (date 1723989150540)
@@ -70,7 +70,7 @@
const screenViewRootNode = new Node( {
children: [
pickupCoilNode.backgroundNode,
- this.fieldNode,
+ // this.fieldNode,
pickupCoilAxisNode,
this.compassNode, // behind barMagnetNode, see https://github.com/phetsims/faradays-electromagnetic-lab/issues/10#issuecomment-1911160748
barMagnetNode,
Index: js/electromagnet/view/ElectromagnetScreenView.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/electromagnet/view/ElectromagnetScreenView.ts b/js/electromagnet/view/ElectromagnetScreenView.ts
--- a/js/electromagnet/view/ElectromagnetScreenView.ts (revision bfc63e82a8bf2ee9233675f49661a985e6edde6f)
+++ b/js/electromagnet/view/ElectromagnetScreenView.ts (date 1723989150553)
@@ -69,7 +69,7 @@
const screenViewRootNode = new Node( {
children: [
electromagnetNode.backgroundNode,
- this.fieldNode,
+ // this.fieldNode,
this.compassNode, // behind electromagnetNode, see https://github.com/phetsims/faradays-electromagnetic-lab/issues/10#issuecomment-1911160748
electromagnetNode,
dcPowerSupplyPanel,
Index: js/common/view/CoilNode.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/common/view/CoilNode.ts b/js/common/view/CoilNode.ts
--- a/js/common/view/CoilNode.ts (revision bfc63e82a8bf2ee9233675f49661a985e6edde6f)
+++ b/js/common/view/CoilNode.ts (date 1723989172404)
@@ -25,7 +25,7 @@
import Bounds2 from '../../../../dot/js/Bounds2.js';
import CoilSegment from '../model/CoilSegment.js';
import CoilSegmentNode from './CoilSegmentNode.js';
-import CurrentNode from './CurrentNode.js';
+// import CurrentNode from './CurrentNode.js';
import Property from '../../../../axon/js/Property.js';
import Vector2 from '../../../../dot/js/Vector2.js';
import platform from '../../../../phet-core/js/platform.js';
@@ -122,13 +122,13 @@
}
// Render the current that moves through the coil.
- if ( options.renderCurrent ) {
- const foregroundCurrentNode = new CurrentNode( 'foreground', coil, this.foregroundCoilSegmentsParent.boundsProperty );
- this.foregroundNode.addChild( foregroundCurrentNode );
-
- const backgroundCurrentNode = new CurrentNode( 'background', coil, this.backgroundCoilSegmentsParent.boundsProperty );
- this.backgroundNode.addChild( backgroundCurrentNode );
- }
+ // if ( options.renderCurrent ) {
+ // const foregroundCurrentNode = new CurrentNode( 'foreground', coil, this.foregroundCoilSegmentsParent.boundsProperty );
+ // this.foregroundNode.addChild( foregroundCurrentNode );
+ //
+ // const backgroundCurrentNode = new CurrentNode( 'background', coil, this.backgroundCoilSegmentsParent.boundsProperty );
+ // this.backgroundNode.addChild( backgroundCurrentNode );
+ // }
// Render the segments that make up the coil's wire.
this.coilSegmentNodes = [];
Index: js/common/view/CurrentNode.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/common/view/CurrentNode.ts b/js/common/view/CurrentNode.ts
--- a/js/common/view/CurrentNode.ts (revision bfc63e82a8bf2ee9233675f49661a985e6edde6f)
+++ b/js/common/view/CurrentNode.ts (date 1723988689685)
@@ -1,202 +1,202 @@
-// Copyright 2024, University of Colorado Boulder
-
-/**
- * CurrentNode is the visual representation of current in a coil. Depending on the current convention selected,
- * it shows either electrons or imaginary positive charges. It shows charges for one layer (foreground or background)
- * of the coil, and hides charges that are in the other layer. Two instances of this class are needed to render all
- * charges in a coil. It uses scenery's Sprites feature for performance optimization.
- *
- * @author Chris Malley (PixelZoom, Inc.)
- */
-
-import faradaysElectromagneticLab from '../../faradaysElectromagneticLab.js';
-import { Color, Sprite, SpriteImage, SpriteInstance, SpriteInstanceTransformType, Sprites } from '../../../../scenery/js/imports.js';
-import Vector2 from '../../../../dot/js/Vector2.js';
-import FELColors from '../FELColors.js';
-import ChargedParticle from '../model/ChargedParticle.js';
-import Coil, { CoilLayer } from '../model/Coil.js';
-import Bounds2 from '../../../../dot/js/Bounds2.js';
-import Multilink from '../../../../axon/js/Multilink.js';
-import ElectronNode from './ElectronNode.js';
-import { CurrentFlow } from '../FELQueryParameters.js';
-import PositiveChargeNode from './PositiveChargeNode.js';
-import TReadOnlyProperty from '../../../../axon/js/TReadOnlyProperty.js';
-
-// Scale up by this much when creating Nodes, to improve resolution.
-const RESOLUTION_SCALE = 8;
-
-// Inverse scale to apply when rendering SpriteInstances.
-const INVERSE_SCALE = 1 / RESOLUTION_SCALE;
-
-const electronColorProperty = FELColors.electronColorProperty;
-const electronMinusColorProperty = FELColors.electronMinusColorProperty;
-const positiveChargeColorProperty = FELColors.positiveChargeColorProperty;
-const positiveChargePlusColorProperty = FELColors.positiveChargePlusColorProperty;
-
-export default class CurrentNode extends Sprites {
-
- // CurrentNode will show charges that are in this layer of the coil.
- private readonly coilLayer: CoilLayer;
-
- // The single Sprite used to render all charges.
- private readonly sprite: Sprite;
-
- // One SpriteInstance for each charge.
- private readonly spriteInstances: ChargedParticleSpriteInstance[];
-
- public constructor( coilLayer: CoilLayer, coil: Coil, canvasBoundsProperty: TReadOnlyProperty<Bounds2> ) {
-
- // Convert the Sprite used to represent current.
- const sprite = new Sprite( CurrentNode.getSpriteImage(
- coil.currentFlowProperty.value,
- electronColorProperty.value, electronMinusColorProperty.value,
- positiveChargeColorProperty.value, positiveChargePlusColorProperty.value
- ) );
-
- // To be populated by this.createSpriteInstances.
- const spriteInstances: ChargedParticleSpriteInstance[] = [];
-
- super( {
- isDisposable: false,
- visibleProperty: coil.currentVisibleProperty,
- sprites: [ sprite ], // the set of Sprites used to render this Node, must be set at instantiation
- spriteInstances: spriteInstances, // the set of SpriteInstances, one per charge in the coil
- hitTestSprites: false,
- pickable: false
- } );
-
- this.sprite = sprite;
- this.spriteInstances = spriteInstances;
- this.coilLayer = coilLayer;
-
- // When the set of charges changes, create a sprite instance for each charge.
- coil.chargedParticlesProperty.link( chargedParticle => this.createSpriteInstances( chargedParticle ) );
-
- canvasBoundsProperty.link( bounds => {
- this.canvasBounds = bounds;
- } );
-
- // When the charges have moved, update the sprite instances.
- coil.chargedParticlesMovedEmitter.addListener( () => this.updateSpriteInstances() );
-
- // Update the sprite and redraw.
- Multilink.multilink(
- [ coil.currentFlowProperty, electronColorProperty, electronMinusColorProperty, positiveChargeColorProperty, positiveChargePlusColorProperty ],
- ( currentFlow, electronColor, electronMinusColor, positiveChargeColor, positiveChargePlusColor ) => {
- sprite.imageProperty.value = CurrentNode.getSpriteImage( currentFlow, electronColor, electronMinusColor, positiveChargeColor, positiveChargePlusColor );
- this.invalidatePaint();
- } );
- }
-
- /**
- * Creates the SpriteInstances to match a set of charges. We're not bothering with SpriteInstance pooling because
- * this happens rarely, and the time to create new instances is not noticeable.
- */
- private createSpriteInstances( chargedParticles: ChargedParticle[] ): void {
-
- // Dispose of the SpriteInstances that we currently have.
- this.spriteInstances.forEach( spriteInstance => spriteInstance.dispose() );
- this.spriteInstances.length = 0;
-
- // Create new SpriteInstances for the new set of charges.
- chargedParticles.forEach( chargedParticle =>
- this.spriteInstances.push( new ChargedParticleSpriteInstance( chargedParticle, this.coilLayer, this.sprite ) ) );
-
- this.invalidatePaint();
- }
-
- /**
- * Updates the spriteInstances to match the charges.
- */
- private updateSpriteInstances(): void {
- this.spriteInstances.forEach( spriteInstance => spriteInstance.update() );
- this.invalidatePaint();
- }
-
- /**
- * Gets the SpriteImage used to visualize a charge.
- */
- private static getSpriteImage( currentFlow: CurrentFlow,
- electronColor: Color, electronMinusColor: Color,
- positiveChargeColor: Color, positiveChargePlusColor: Color ): SpriteImage {
-
- const node = ( currentFlow === 'electron' ) ?
- new ElectronNode( {
- color: electronColor,
- minusColor: electronMinusColor,
- scale: RESOLUTION_SCALE
- } ) :
- new PositiveChargeNode( {
- color: positiveChargeColor,
- plusColor: positiveChargePlusColor,
- scale: RESOLUTION_SCALE
- } );
-
- let spriteImage: SpriteImage | null = null;
- node.toCanvas( ( canvas, x, y, width, height ) => {
- spriteImage = new SpriteImage( canvas, new Vector2( ( x + node.centerX ), ( y + node.centerY ) ), {
-
- // Mipmapping was added to address pixelation reported in https://github.com/phetsims/faradays-electromagnetic-lab/issues/121
- mipmap: true,
- mipmapBias: -0.7 // Use a negative value to increase the displayed resolution. See Imageable.setMipmapBias.
- } );
- } );
-
- assert && assert( spriteImage );
- return spriteImage!;
- }
-}
-
-/**
- * ChargedParticleSpriteInstance corresponds to one ChargedParticle instance.
- */
-class ChargedParticleSpriteInstance extends SpriteInstance {
-
- private readonly chargedParticle: ChargedParticle;
- private readonly coilLayer: CoilLayer;
-
- public constructor( chargedParticle: ChargedParticle, coilLayer: CoilLayer, sprite: Sprite ) {
-
- super();
-
- this.chargedParticle = chargedParticle;
- this.coilLayer = coilLayer;
-
- // Fields in superclass SpriteInstance that must be set
- this.sprite = sprite;
- this.transformType = SpriteInstanceTransformType.TRANSLATION_AND_SCALE;
-
- this.update();
- }
-
- public dispose(): void {
- // Nothing to do currently. But instances of this class are allocated dynamically, so keep this method as a bit of defensive programming.
- }
-
- /**
- * Updates the matrix to match the charge's position and layering.
- */
- public update(): void {
-
- // Move to the charge's position (at the charge's center) and apply inverse scale.
- this.matrix.rowMajor(
- INVERSE_SCALE, 0, this.chargedParticle.x,
- 0, INVERSE_SCALE, this.chargedParticle.y,
- 0, 0, 1
- );
- assert && assert( this.matrix.isFinite(), 'matrix should be finite' );
-
- if ( this.chargedParticle.getLayer() === this.coilLayer ) {
-
- // Show foreground more prominently than background.
- this.alpha = ( this.coilLayer === 'foreground' ) ? 1 : 0.5;
- }
- else {
-
- // It the charge is not in this layer, hide it by setting its alpha to fully-transparent.
- this.alpha = 0;
- }
- }
-}
-
-faradaysElectromagneticLab.register( 'CurrentNode', CurrentNode );
\ No newline at end of file
+// // Copyright 2024, University of Colorado Boulder
+//
+// /**
+// * CurrentNode is the visual representation of current in a coil. Depending on the current convention selected,
+// * it shows either electrons or imaginary positive charges. It shows charges for one layer (foreground or background)
+// * of the coil, and hides charges that are in the other layer. Two instances of this class are needed to render all
+// * charges in a coil. It uses scenery's Sprites feature for performance optimization.
+// *
+// * @author Chris Malley (PixelZoom, Inc.)
+// */
+//
+// import faradaysElectromagneticLab from '../../faradaysElectromagneticLab.js';
+// import { Color, Sprite, SpriteImage, SpriteInstance, SpriteInstanceTransformType, Sprites } from '../../../../scenery/js/imports.js';
+// import Vector2 from '../../../../dot/js/Vector2.js';
+// import FELColors from '../FELColors.js';
+// import ChargedParticle from '../model/ChargedParticle.js';
+// import Coil, { CoilLayer } from '../model/Coil.js';
+// import Bounds2 from '../../../../dot/js/Bounds2.js';
+// import Multilink from '../../../../axon/js/Multilink.js';
+// import ElectronNode from './ElectronNode.js';
+// import { CurrentFlow } from '../FELQueryParameters.js';
+// import PositiveChargeNode from './PositiveChargeNode.js';
+// import TReadOnlyProperty from '../../../../axon/js/TReadOnlyProperty.js';
+//
+// // Scale up by this much when creating Nodes, to improve resolution.
+// const RESOLUTION_SCALE = 8;
+//
+// // Inverse scale to apply when rendering SpriteInstances.
+// const INVERSE_SCALE = 1 / RESOLUTION_SCALE;
+//
+// const electronColorProperty = FELColors.electronColorProperty;
+// const electronMinusColorProperty = FELColors.electronMinusColorProperty;
+// const positiveChargeColorProperty = FELColors.positiveChargeColorProperty;
+// const positiveChargePlusColorProperty = FELColors.positiveChargePlusColorProperty;
+//
+// export default class CurrentNode extends Sprites {
+//
+// // CurrentNode will show charges that are in this layer of the coil.
+// private readonly coilLayer: CoilLayer;
+//
+// // The single Sprite used to render all charges.
+// private readonly sprite: Sprite;
+//
+// // One SpriteInstance for each charge.
+// private readonly spriteInstances: ChargedParticleSpriteInstance[];
+//
+// public constructor( coilLayer: CoilLayer, coil: Coil, canvasBoundsProperty: TReadOnlyProperty<Bounds2> ) {
+//
+// // Convert the Sprite used to represent current.
+// const sprite = new Sprite( CurrentNode.getSpriteImage(
+// coil.currentFlowProperty.value,
+// electronColorProperty.value, electronMinusColorProperty.value,
+// positiveChargeColorProperty.value, positiveChargePlusColorProperty.value
+// ) );
+//
+// // To be populated by this.createSpriteInstances.
+// const spriteInstances: ChargedParticleSpriteInstance[] = [];
+//
+// super( {
+// isDisposable: false,
+// visibleProperty: coil.currentVisibleProperty,
+// sprites: [ sprite ], // the set of Sprites used to render this Node, must be set at instantiation
+// spriteInstances: spriteInstances, // the set of SpriteInstances, one per charge in the coil
+// hitTestSprites: false,
+// pickable: false
+// } );
+//
+// this.sprite = sprite;
+// this.spriteInstances = spriteInstances;
+// this.coilLayer = coilLayer;
+//
+// // When the set of charges changes, create a sprite instance for each charge.
+// coil.chargedParticlesProperty.link( chargedParticle => this.createSpriteInstances( chargedParticle ) );
+//
+// canvasBoundsProperty.link( bounds => {
+// this.canvasBounds = bounds;
+// } );
+//
+// // When the charges have moved, update the sprite instances.
+// coil.chargedParticlesMovedEmitter.addListener( () => this.updateSpriteInstances() );
+//
+// // Update the sprite and redraw.
+// Multilink.multilink(
+// [ coil.currentFlowProperty, electronColorProperty, electronMinusColorProperty, positiveChargeColorProperty, positiveChargePlusColorProperty ],
+// ( currentFlow, electronColor, electronMinusColor, positiveChargeColor, positiveChargePlusColor ) => {
+// sprite.imageProperty.value = CurrentNode.getSpriteImage( currentFlow, electronColor, electronMinusColor, positiveChargeColor, positiveChargePlusColor );
+// this.invalidatePaint();
+// } );
+// }
+//
+// /**
+// * Creates the SpriteInstances to match a set of charges. We're not bothering with SpriteInstance pooling because
+// * this happens rarely, and the time to create new instances is not noticeable.
+// */
+// private createSpriteInstances( chargedParticles: ChargedParticle[] ): void {
+//
+// // Dispose of the SpriteInstances that we currently have.
+// this.spriteInstances.forEach( spriteInstance => spriteInstance.dispose() );
+// this.spriteInstances.length = 0;
+//
+// // Create new SpriteInstances for the new set of charges.
+// chargedParticles.forEach( chargedParticle =>
+// this.spriteInstances.push( new ChargedParticleSpriteInstance( chargedParticle, this.coilLayer, this.sprite ) ) );
+//
+// this.invalidatePaint();
+// }
+//
+// /**
+// * Updates the spriteInstances to match the charges.
+// */
+// private updateSpriteInstances(): void {
+// this.spriteInstances.forEach( spriteInstance => spriteInstance.update() );
+// this.invalidatePaint();
+// }
+//
+// /**
+// * Gets the SpriteImage used to visualize a charge.
+// */
+// private static getSpriteImage( currentFlow: CurrentFlow,
+// electronColor: Color, electronMinusColor: Color,
+// positiveChargeColor: Color, positiveChargePlusColor: Color ): SpriteImage {
+//
+// const node = ( currentFlow === 'electron' ) ?
+// new ElectronNode( {
+// color: electronColor,
+// minusColor: electronMinusColor,
+// scale: RESOLUTION_SCALE
+// } ) :
+// new PositiveChargeNode( {
+// color: positiveChargeColor,
+// plusColor: positiveChargePlusColor,
+// scale: RESOLUTION_SCALE
+// } );
+//
+// let spriteImage: SpriteImage | null = null;
+// node.toCanvas( ( canvas, x, y, width, height ) => {
+// spriteImage = new SpriteImage( canvas, new Vector2( ( x + node.centerX ), ( y + node.centerY ) ), {
+//
+// // Mipmapping was added to address pixelation reported in https://github.com/phetsims/faradays-electromagnetic-lab/issues/121
+// mipmap: true,
+// mipmapBias: -0.7 // Use a negative value to increase the displayed resolution. See Imageable.setMipmapBias.
+// } );
+// } );
+//
+// assert && assert( spriteImage );
+// return spriteImage!;
+// }
+// }
+//
+// /**
+// * ChargedParticleSpriteInstance corresponds to one ChargedParticle instance.
+// */
+// class ChargedParticleSpriteInstance extends SpriteInstance {
+//
+// private readonly chargedParticle: ChargedParticle;
+// private readonly coilLayer: CoilLayer;
+//
+// public constructor( chargedParticle: ChargedParticle, coilLayer: CoilLayer, sprite: Sprite ) {
+//
+// super();
+//
+// this.chargedParticle = chargedParticle;
+// this.coilLayer = coilLayer;
+//
+// // Fields in superclass SpriteInstance that must be set
+// this.sprite = sprite;
+// this.transformType = SpriteInstanceTransformType.TRANSLATION_AND_SCALE;
+//
+// this.update();
+// }
+//
+// public dispose(): void {
+// // Nothing to do currently. But instances of this class are allocated dynamically, so keep this method as a bit of defensive programming.
+// }
+//
+// /**
+// * Updates the matrix to match the charge's position and layering.
+// */
+// public update(): void {
+//
+// // Move to the charge's position (at the charge's center) and apply inverse scale.
+// this.matrix.rowMajor(
+// INVERSE_SCALE, 0, this.chargedParticle.x,
+// 0, INVERSE_SCALE, this.chargedParticle.y,
+// 0, 0, 1
+// );
+// assert && assert( this.matrix.isFinite(), 'matrix should be finite' );
+//
+// if ( this.chargedParticle.getLayer() === this.coilLayer ) {
+//
+// // Show foreground more prominently than background.
+// this.alpha = ( this.coilLayer === 'foreground' ) ? 1 : 0.5;
+// }
+// else {
+//
+// // It the charge is not in this layer, hide it by setting its alpha to fully-transparent.
+// this.alpha = 0;
+// }
+// }
+// }
+//
+// faradaysElectromagneticLab.register( 'CurrentNode', CurrentNode );
\ No newline at end of file
Index: js/common/FELSim.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/common/FELSim.ts b/js/common/FELSim.ts
--- a/js/common/FELSim.ts (revision bfc63e82a8bf2ee9233675f49661a985e6edde6f)
+++ b/js/common/FELSim.ts (date 1723948010134)
@@ -35,7 +35,7 @@
// Disabling WebGL uses Canvas as the fallback for Sprites. The problem was not present on iOS, but we
// have no way to differentiate between Safari on iPadOS vs iOS, so WebGL is disabled on both platforms.
// See https://github.com/phetsims/faradays-electromagnetic-lab/issues/182.
- webgl: !platform.mobileSafari,
+ webgl: true,
// Remove ScreenViews that are not active, to minimize WebGL contexts, see https://github.com/phetsims/faradays-electromagnetic-lab/issues/153
detachInactiveScreenViews: true,
Index: js/common/view/FELScreenView.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/common/view/FELScreenView.ts b/js/common/view/FELScreenView.ts
--- a/js/common/view/FELScreenView.ts (revision bfc63e82a8bf2ee9233675f49661a985e6edde6f)
+++ b/js/common/view/FELScreenView.ts (date 1723989086846)
@@ -13,7 +13,6 @@
import faradaysElectromagneticLab from '../../faradaysElectromagneticLab.js';
import { Line, Node } from '../../../../scenery/js/imports.js';
import Multilink from '../../../../axon/js/Multilink.js';
-import FieldNode from '../../common/view/FieldNode.js';
import FieldMeterNode from '../../common/view/FieldMeterNode.js';
import CompassNode from '../../common/view/CompassNode.js';
import Magnet from '../model/Magnet.js';
@@ -55,7 +54,7 @@
export default class FELScreenView extends ScreenView {
// It is the subclass' responsibility to add these to the scene graph and pdomOrder.
- protected readonly fieldNode: Node;
+ // protected readonly fieldNode: Node;
protected readonly fieldMeterNode: Node;
protected readonly compassNode: Node;
protected readonly resetAllButton: Node;
@@ -82,7 +81,7 @@
options.rightPanels.top = this.layoutBounds.top + FELConstants.SCREEN_VIEW_Y_MARGIN;
} );
- this.fieldNode = new FieldNode( options.magnet, this.visibleBoundsProperty, options.tandem.createTandem( 'fieldNode' ) );
+ // this.fieldNode = new FieldNode( options.magnet, this.visibleBoundsProperty, options.tandem.createTandem( 'fieldNode' ) );
this.compassNode = new CompassNode( options.compass, {
Index: js/transformer/view/TransformerScreenView.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/transformer/view/TransformerScreenView.ts b/js/transformer/view/TransformerScreenView.ts
--- a/js/transformer/view/TransformerScreenView.ts (revision bfc63e82a8bf2ee9233675f49661a985e6edde6f)
+++ b/js/transformer/view/TransformerScreenView.ts (date 1723989150547)
@@ -85,7 +85,7 @@
children: [
transformerNode.pickupCoilNode.backgroundNode,
transformerNode.electromagnetNode.backgroundNode,
- this.fieldNode,
+ // this.fieldNode,
pickupCoilAxisNode,
this.compassNode, // behind transformerNode, see https://github.com/phetsims/faradays-electromagnetic-lab/issues/10#issuecomment-1911160748
transformerNode,
Index: js/generator/view/GeneratorScreenView.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/generator/view/GeneratorScreenView.ts b/js/generator/view/GeneratorScreenView.ts
--- a/js/generator/view/GeneratorScreenView.ts (revision bfc63e82a8bf2ee9233675f49661a985e6edde6f)
+++ b/js/generator/view/GeneratorScreenView.ts (date 1723989150565)
@@ -41,7 +41,7 @@
const screenViewRootNode = new Node( {
children: [
generatorNode.backgroundNode,
- this.fieldNode,
+ // this.fieldNode,
this.compassNode, // behind generatorNode, see https://github.com/phetsims/faradays-electromagnetic-lab/issues/10#issuecomment-1911160748
generatorNode,
rightPanels,
Index: js/bar-magnet/view/BarMagnetScreenView.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/bar-magnet/view/BarMagnetScreenView.ts b/js/bar-magnet/view/BarMagnetScreenView.ts
--- a/js/bar-magnet/view/BarMagnetScreenView.ts (revision bfc63e82a8bf2ee9233675f49661a985e6edde6f)
+++ b/js/bar-magnet/view/BarMagnetScreenView.ts (date 1723989150559)
@@ -73,7 +73,7 @@
// Rendering order, from back to front
const screenViewRootNode = new Node( {
children: [
- this.fieldNode,
+ // this.fieldNode,
this.compassNode, // behind barMagnetNode, see https://github.com/phetsims/faradays-electromagnetic-lab/issues/10#issuecomment-1911160748
barMagnetNode,
earthNode,
Index: js/faradays-electromagnetic-lab-main.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/faradays-electromagnetic-lab-main.ts b/js/faradays-electromagnetic-lab-main.ts
--- a/js/faradays-electromagnetic-lab-main.ts (revision bfc63e82a8bf2ee9233675f49661a985e6edde6f)
+++ b/js/faradays-electromagnetic-lab-main.ts (date 1723949842262)
@@ -18,6 +18,25 @@
import FELSim from './common/FELSim.js';
import FELPreferences from './common/model/FELPreferences.js';
+( function() {
+ // Store the original document.createElement method
+ const originalCreateElement = document.createElement;
+
+ // Override the document.createElement method
+ document.createElement = function( tagName ) {
+ console.log('document.createElement');
+ // Check if the tagName is 'canvas'
+ if ( tagName.toLowerCase() === 'canvas' ) {
+ console.log( 'A canvas element is being created.' );
+ }
+
+ // Call the original method
+ return originalCreateElement.apply( document, arguments );
+ };
+} )();
+
+console.log( 'startup' );
+
simLauncher.launch( () => {
const preferences = new FELPreferences();
const titleStringProperty = FaradaysElectromagneticLabStrings[ 'faradays-electromagnetic-lab' ].titleStringProperty;
Harness: import SwiftUI
import WebKit
struct ContentView: View {
var body: some View {
WebView(url: URL(string: "http://192.168.1.4/faradays-electromagnetic-lab/faradays-electromagnetic-lab_en.html?debugger&fuzz&screens=1&fuzzRate=5&disableModals")!)
.edgesIgnoringSafeArea(.all)
}
}
struct WebView: UIViewRepresentable {
let url: URL
func makeUIView(context: Context) -> WKWebView {
// Create the WKWebView configuration
let config = WKWebViewConfiguration()
// Add the JavaScript message handler to handle console.log
let contentController = WKUserContentController()
contentController.add(context.coordinator, name: "consoleHandler")
// Inject a script to override console.log
let script = """
window.console.log = (function(originalLogFunc) {
return function(text) {
originalLogFunc(text);
window.webkit.messageHandlers.consoleHandler.postMessage(text);
}
})(window.console.log);
"""
let userScript = WKUserScript(source: script, injectionTime: .atDocumentStart, forMainFrameOnly: false)
contentController.addUserScript(userScript)
config.userContentController = contentController
// Create the WKWebView with the configuration
return WKWebView(frame: .zero, configuration: config)
}
func updateUIView(_ uiView: WKWebView, context: Context) {
let request = URLRequest(url: url)
uiView.load(request)
}
// Create a coordinator to handle messages from JavaScript
func makeCoordinator() -> Coordinator {
Coordinator()
}
class Coordinator: NSObject, WKScriptMessageHandler {
func userContentController(_ userContentController: WKUserContentController, didReceive message: WKScriptMessage) {
if message.name == "consoleHandler", let logMessage = message.body as? String {
print("JavaScript log: \(logMessage)")
}
}
}
}
#Preview {
ContentView()
}Leak in VM: IOSurface 
UPDATE: Some of my conclusions above may be wrong since I have been looking at total MB instead of persistent MB. |
|
Similar to #316 (comment), we feel like the pixelRatio and aliasing are appropriate workarounds given how close we are to RC. Let's not eagerly go down the road of using a single renderer for all screens. If we need to investigate later, then we can. We'll wait to see how QA goes. |
|
@Nancy-Salpepi can you please test the links in [redacted link] to see how they do on iOS devices? We disabled antialiasing and the pixelRatio to save on memory, and it is working much better on iPad 7 and iPhone 15 Pro Max. An initial crash + reload on startup may be ok--we hypothesize this is clearing out some hidden memory. Can you run some fuzzing tests and some manual tests for those links on your iOS devices and let us know how it is going? If it is crashy, we have some new query parameters we can share with you to experiment with. UPDATE: Please start with buoyancy as a worst-case-scenario (for memory). If that is OK, then density and buoyancy-basics can be quicker spot checks. Also note we have seen a little bit of rendering aliasing and "crumbs" due to the aliasing and pixel ratio changes. |
|
Here is what I saw so far: For the iPad (9th generation):
For iPhone 12 Pro:
@samreid I'm in a meeting right now but if you want to give me the query parameters I can test with those later. |
|
Thanks! Can you visit https://johankj.github.io/devicePixelRatio/ on each of those devices and report its device pixel density? My iPad 7 is a 2 and my iPhone 15 Pro Max is a 3. Also for future fuzz/crash testing, I forgot to mention we added |
|
@Nancy-Salpepi thanks. Can you also run the failing tests with |
|
Device Pixel Density: |
|
using ?threeRendererPixelRatio=1&launchCounter&fuzz:
On my iPhone, with ?threeRendererPixelRatio=1: Wondering if crashes would still happen without screenshots on? @samreid you said something about possibly shutting them off during fuzzing? |
|
Screenshots are now shut off (do not appear in phet menu) during fuzzing (including in the recent test above). I'm not sure how to proceed here but will discuss with my subteam on Monday morning. Thanks for all the great testing! |
|
I'm able to take screenshots of the duck on iPhone 15 Pro Max using the buoyancy link on sparky. |
|
We reduced the pixel ratio for iOS, and would like QA to verify in RC. Please test that iPad and iPhones graphics look reasonable and run a reasonable amount of time before crashing. (Initial crash on startup is ok.) |
8 tasks
|
For phetsims/qa#1141
|
|
iPad Pro 11 inch 4th gen, latest OS, safari and Did 30 minutes of regular testing for Buoyancy and 10 minutes of fuzzing and did not get any crashes. |
|
On iPhone 12 iOS 18 I did a fuzz test for Buoyancy and it crashed and reloaded after about 2 minutes. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
In Faraday's Electromagnetic lab, mobile safari WebGL had to be disabled to prevent crashing, see phetsims/faradays-electromagnetic-lab#182 (comment)
In Gas Properties, the same thing happened, see phetsims/gas-properties#289 (comment).
Today I observed that neither unbuilt nor built buoyancy launched on the iPhone 15 Pro Max, see #315
@KatieWoe also identified rendering issues on the iPad, which is puzzling--since it means it must have run on iPad at least a bit. #230
@arouinfar can you please advise?
The text was updated successfully, but these errors were encountered: