-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
iPad: WebGL support #316
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. |
When crashing in a webview on tethered xcode, I get these errors:
|
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. |
Oh, it's just every 10 seconds when it takes a snapshot. |
In text
|
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. |
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.) |
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. |
For rc.2 I fuzz tested for 10 minutes on an iPad 9th generation with iOS 18 and saw no crashes. |
Things looked ok after a minute or two of fuzzing on my iPhone (iOS 18). Since it was the only one that crashed last time, I think this is ok. |
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: