iPad: WebGL support

[samreid]

In Faraday's Electromagnetic lab, mobile safari WebGL had to be disabled to prevent crashing, see https://github.com/phetsims/faradays-electromagnetic-lab/issues/182#issuecomment-2168713207

In Gas Properties, the same thing happened, see https://github.com/phetsims/gas-properties/issues/289#issuecomment-2246277100.

Today I observed that neither unbuilt nor built buoyancy launched on the iPhone 15 Pro Max, see https://github.com/phetsims/density-buoyancy-common/issues/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. https://github.com/phetsims/density-buoyancy-common/issues/230

@arouinfar can you please advise?

[arouinfar]

@samreid in FEL & GasProps, we had to detect mobile Safari and set webgl=false. There is a common code issue tracking a more general fix in https://github.com/phetsims/scenery/issues/1649. Unfortunately, it doesn't appear to be prioritized and JO doesn't have access to the tools he needs to make progress on the issue.

Edit: I asked for the underlying scenery issue to be prioritized on the #planning Slack channel.

[jonathanolson]

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 ?screens=1 allows Buoyancy to launch on my iPhone, where it fails to launch without that query parameter.

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?

[samreid]

Testing on iPad 7 running iPadOS 17.1.2, on a built version of buoyancy with ?fuzz&screens=1, it crashed at around:

• 5 minutes.
• 5m 47s
• 6m 10s

Testing published density (all 3 screens) with fuzz, it crashes in:

• 2m 30s.

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.

[samreid]

When crashing in a webview on tethered xcode, I get these errors:

0x10f000a00 - [PID=601] WebProcessProxy::didClose: (web process 601 crash)
0x10f000a00 - [PID=601] WebProcessProxy::processDidTerminateOrFailedToLaunch: reason=Crash
0x10e01c720 - ProcessAssertion: Failed to acquire RBS Background assertion 'XPCConnectionTerminationWatchdog' for process because PID 0 is invalid
0x10e01c720 - ProcessAssertion::acquireSync Failed to acquire RBS assertion 'XPCConnectionTerminationWatchdog' for process with PID=0, error: (null)
0x10202e018 - [pageProxyID=6, webPageID=7, PID=601] WebPageProxy::processDidTerminate: (pid 601), reason=Crash
0x10202e018 - [pageProxyID=6, webPageID=7, PID=601] WebPageProxy::dispatchProcessDidTerminate: reason=Crash

[samreid]

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?

[samreid]

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.

[samreid]

Tethering to safari is very flaky, even for one screen fuzzing, but I managed to get this timeline memory snapshot:

[samreid]

Looks like every now and then there is a large memory spike and long GC time:

[samreid]

Oh, it's just every 10 seconds when it takes a snapshot.

[samreid]

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:

[samreid]

Using "Instruments" we see that VM: IOSurface is leaking rapidly:

[samreid]

Here is the caller of those allocations:

[samreid]

Here is the stack:

[samreid]

In text

IOSurfaceClientLookupFromMachPort   
-[IOSurface initWithMachPort:]  
WebCore::IOSurface::createFromSendRight(WTF::MachSendRight const&&) 
decltype(auto) std::__1::__variant_detail::__visitation::__base::__dispatcher<1ul>::__dispatch[abi:v160006]<std::__1::__variant_detail::__visitation::__variant::__value_visitor<WTF::Visitor<WebKit::RemoteLayerBackingStoreProperties::layerContentsBufferFromBackendHandle(std::__1::variant<WebKit::ShareableBitmapHandle, WTF::MachSendRight>&&, WebKit::RemoteLayerBackingStoreProperties::LayerContentsType)::$_1, WebKit::RemoteLayerBackingStoreProperties::layerContentsBufferFromBackendHandle(std::__1::variant<WebKit::ShareableBitmapHandle, WTF::MachSendRight>&&, WebKit::RemoteLayerBackingStoreProperties::LayerContentsType)::$_2>>&&, std::__1::__variant_detail::__base<(std::__1::__variant_detail::_Trait)1, WebKit::ShareableBitmapHandle, WTF::MachSendRight>&>(std::__1::__variant_detail::__visitation::__variant::__value_visitor<WTF::Visitor<WebKit::RemoteLayerBackingStoreProperties::layerContentsBufferFromBackendHandle(std::__1::variant<WebKit::ShareableBitmapHandle, WTF::MachSendRight>&&, WebKit::RemoteLayerBackingStoreProperties::LayerContentsType)::$_1, WebKit::RemoteLayerBackingStoreProperties::layerContentsBufferFromBackendHandle(std::__1::variant<WebKit::ShareableBitmapHandle, WTF::MachSendRight>&&, WebKit::RemoteLayerBackingStoreProperties::LayerContentsType)::$_2>>&&, std::__1::__variant_detail::__base<(std::__1::__variant_detail::_Trait)1, WebKit::ShareableBitmapHandle, WTF::MachSendRight>&)   
WebKit::RemoteLayerBackingStoreProperties::layerContentsBufferFromBackendHandle(std::__1::variant<WebKit::ShareableBitmapHandle, WTF::MachSendRight>&&, WebKit::RemoteLayerBackingStoreProperties::LayerContentsType)   
WebKit::RemoteLayerTreePropertyApplier::applyPropertiesToLayer(CALayer*, WebKit::RemoteLayerTreeNode*, WebKit::RemoteLayerTreeHost*, WebKit::RemoteLayerTreeTransaction::LayerProperties const&, WebKit::RemoteLayerBackingStoreProperties::LayerContentsType)  
WebKit::RemoteLayerTreePropertyApplier::applyProperties(WebKit::RemoteLayerTreeNode&, WebKit::RemoteLayerTreeHost*, WebKit::RemoteLayerTreeTransaction::LayerProperties const&, WTF::HashMap<WebCore::ProcessQualified<WTF::ObjectIdentifierGeneric<WebCore::PlatformLayerIdentifierType, WTF::ObjectIdentifierMainThreadAccessTraits>>, std::__1::unique_ptr<WebKit::RemoteLayerTreeNode, std::__1::default_delete<WebKit::RemoteLayerTreeNode>>, WTF::DefaultHash<WebCore::ProcessQualified<WTF::ObjectIdentifierGeneric<WebCore::PlatformLayerIdentifierType, WTF::ObjectIdentifierMainThreadAccessTraits>>>, WTF::HashTraits<WebCore::ProcessQualified<WTF::ObjectIdentifierGeneric<WebCore::PlatformLayerIdentifierType, WTF::ObjectIdentifierMainThreadAccessTraits>>>, WTF::HashTraits<std::__1::unique_ptr<WebKit::RemoteLayerTreeNode, std::__1::default_delete<WebKit::RemoteLayerTreeNode>>>, WTF::HashTableTraits> const&, WebKit::RemoteLayerBackingStoreProperties::LayerContentsType)    
WebKit::RemoteLayerTreeHost::updateLayerTree(WebKit::RemoteLayerTreeTransaction const&, float)  
WebKit::RemoteLayerTreeDrawingAreaProxy::commitLayerTree(IPC::Connection&, WTF::Vector<std::__1::pair<WebKit::RemoteLayerTreeTransaction, WebKit::RemoteScrollingCoordinatorTransaction>, 0ul, WTF::CrashOnOverflow, 16ul, WTF::FastMalloc> const&) 
WebKit::RemoteLayerTreeDrawingAreaProxy::didReceiveMessage(IPC::Connection&, IPC::Decoder&) 
IPC::MessageReceiverMap::dispatchMessage(IPC::Connection&, IPC::Decoder&)   
WebKit::WebProcessProxy::didReceiveMessage(IPC::Connection&, IPC::Decoder&) 
IPC::Connection::dispatchMessage(std::__1::unique_ptr<IPC::Decoder, std::__1::default_delete<IPC::Decoder>>)    
IPC::Connection::dispatchIncomingMessages() 
WTF::RunLoop::performWork() 
WTF::RunLoop::performWork(void*)    
__CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__  
__CFRunLoopDoSource0    
__CFRunLoopDoSources0   
__CFRunLoopRun  
CFRunLoopRunSpecific    
GSEventRunModal 
-[UIApplication _run]   
UIApplicationMain   
0x1c6f81760 
0x1c6f81610 
0x1c6c7fae8 
0x1042b553f 
main    
start   

[samreid]

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:

[samreid]

More discoveries:

• This leak does not appear on iPad10 simulator
• The leak disappears when putting ?webgl=false

[samreid]

Surprisingly, removing all Sprites from FEL still shows a lot of churn in Total Bytes, but stabilizes in persistent bytes 182MB:

```diff 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 ) { - - // 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 ) { +// +// // 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:

```swift 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.

[samreid]

Disable cache in wkwebview:

```swift import SwiftUI import WebKit // Custom WebView that disables caching and propagates console.log struct WebView: UIViewRepresentable { let url: URL func makeUIView(context: Context) -> WKWebView { // Create the WKWebView configuration let config = makeConfiguration() // 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 let webView = WKWebView(frame: .zero, configuration: config) let request = URLRequest(url: url, cachePolicy: .reloadIgnoringLocalCacheData, timeoutInterval: 10.0) webView.load(request) return webView } func updateUIView(_ webView: WKWebView, context: Context) { let request = URLRequest(url: url, cachePolicy: .reloadIgnoringLocalCacheData, timeoutInterval: 10.0) webView.load(request) } private func makeConfiguration() -> WKWebViewConfiguration { let configuration = WKWebViewConfiguration() configuration.websiteDataStore = .nonPersistent() // This will disable cache by using a non-persistent data store return configuration } // 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)") } } } } 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&fuzzRate=100&cacheBreak")!) .edgesIgnoringSafeArea(.all) } } #Preview { ContentView() }

[samreid]

Here is a minimal patch that shows that changing screen.view.setVisible( visible ); to screen.view.setVisible( true ); in Sim.ts avoids the VM: IOSurface leak in FEL, on my tethered ipad in Instruments. This means the ScreenView.setVisible(true)/setVisible(false) is cascading to the leak.

```diff Subject: [PATCH] 123 --- Index: joist/js/Sim.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/joist/js/Sim.ts b/joist/js/Sim.ts --- a/joist/js/Sim.ts (revision 232f3a3f3d327ac83522aaf45150bfffa07dcba2) +++ b/joist/js/Sim.ts (date 1724023128244) @@ -297,6 +297,8 @@ options.preferencesModel = new PreferencesModel(); } + console.log('detachInactiveScreenViews', options.detachInactiveScreenViews); + // Some options are used by sim and SimDisplay. Promote webgl to top level sim option out of API ease, but it is // passed to the SimDisplay. const simDisplayOptions: SimDisplayOptions = { @@ -782,7 +784,13 @@ this.display.simulationRoot.insertChild( 0, screen.view ); } } + + // LEAK VM: IOSURFACE screen.view.setVisible( visible ); + + // NO LEAK + screen.view.setVisible( true ); + if ( !visible ) { if ( this.detachInactiveScreenViews && this.display.simulationRoot.hasChild( screen.view ) ) { this.display.simulationRoot.removeChild( screen.view ); Index: faradays-electromagnetic-lab/js/common/FELSim.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/faradays-electromagnetic-lab/js/common/FELSim.ts b/faradays-electromagnetic-lab/js/common/FELSim.ts --- a/faradays-electromagnetic-lab/js/common/FELSim.ts (revision bfc63e82a8bf2ee9233675f49661a985e6edde6f) +++ b/faradays-electromagnetic-lab/js/common/FELSim.ts (date 1724021893647) @@ -35,10 +35,10 @@ // 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, + // detachInactiveScreenViews: false, credits: FELConstants.CREDITS, phetioDesigned: true, Index: faradays-electromagnetic-lab/js/faradays-electromagnetic-lab-main.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/faradays-electromagnetic-lab/js/faradays-electromagnetic-lab-main.ts b/faradays-electromagnetic-lab/js/faradays-electromagnetic-lab-main.ts --- a/faradays-electromagnetic-lab/js/faradays-electromagnetic-lab-main.ts (revision bfc63e82a8bf2ee9233675f49661a985e6edde6f) +++ b/faradays-electromagnetic-lab/js/faradays-electromagnetic-lab-main.ts (date 1724022179042) @@ -8,13 +8,9 @@ import simLauncher from '../../joist/js/simLauncher.js'; import Tandem from '../../tandem/js/Tandem.js'; -import PickupCoilScreen from './pickup-coil/PickupCoilScreen.js'; import FaradaysElectromagneticLabStrings from './FaradaysElectromagneticLabStrings.js'; import './common/FELQueryParameters.js'; import BarMagnetScreen from './bar-magnet/BarMagnetScreen.js'; -import ElectromagnetScreen from './electromagnet/ElectromagnetScreen.js'; -import TransformerScreen from './transformer/TransformerScreen.js'; -import GeneratorScreen from './generator/GeneratorScreen.js'; import FELSim from './common/FELSim.js'; import FELPreferences from './common/model/FELPreferences.js'; @@ -23,10 +19,11 @@ const titleStringProperty = FaradaysElectromagneticLabStrings[ 'faradays-electromagnetic-lab' ].titleStringProperty; const screens = [ new BarMagnetScreen( preferences, Tandem.ROOT.createTandem( 'barMagnetScreen' ) ), - new PickupCoilScreen( preferences, Tandem.ROOT.createTandem( 'pickupCoilScreen' ) ), - new ElectromagnetScreen( preferences, Tandem.ROOT.createTandem( 'electromagnetScreen' ) ), - new TransformerScreen( preferences, Tandem.ROOT.createTandem( 'transformerScreen' ) ), - new GeneratorScreen( preferences, Tandem.ROOT.createTandem( 'generatorScreen' ) ) + new BarMagnetScreen( preferences, Tandem.ROOT.createTandem( 'barMagnet2Screen' ) ) + // new PickupCoilScreen( preferences, Tandem.ROOT.createTandem( 'pickupCoilScreen' ) ), + // new ElectromagnetScreen( preferences, Tandem.ROOT.createTandem( 'electromagnetScreen' ) ), + // new TransformerScreen( preferences, Tandem.ROOT.createTandem( 'transformerScreen' ) ), + // new GeneratorScreen( preferences, Tandem.ROOT.createTandem( 'generatorScreen' ) ) ]; const sim = new FELSim( titleStringProperty, screens, preferences ); sim.start();

[samreid]

By the way, this is FEL in main right now, which does exhibit the leak (even though there is no webgl):

[samreid]

Another minimal patch with no leak, by showing all screens at once:

```diff Subject: [PATCH] 123 --- Index: js/Sim.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/Sim.ts b/js/Sim.ts --- a/js/Sim.ts (revision 232f3a3f3d327ac83522aaf45150bfffa07dcba2) +++ b/js/Sim.ts (date 1724023765136) @@ -750,9 +750,9 @@ _.each( screens, screen => { screen.view.layerSplit = true; - if ( !this.detachInactiveScreenViews ) { + // if ( !this.detachInactiveScreenViews ) { this.display.simulationRoot.addChild( screen.view ); - } + // } } ); this.display.simulationRoot.addChild( this.navigationBar ); @@ -776,19 +776,19 @@ // Make the selected screen visible and active, other screens invisible and inactive. // screen.isActiveProperty should change only while the screen is invisible, https://github.com/phetsims/joist/issues/418 if ( visible ) { - screen.activeProperty.set( visible ); + // screen.activeProperty.set( visible ); - if ( this.detachInactiveScreenViews && !this.display.simulationRoot.hasChild( screen.view ) ) { - this.display.simulationRoot.insertChild( 0, screen.view ); - } + // if ( this.detachInactiveScreenViews && !this.display.simulationRoot.hasChild( screen.view ) ) { + // this.display.simulationRoot.insertChild( 0, screen.view ); + // } } - screen.view.setVisible( visible ); + screen.view.setVisible( true ); if ( !visible ) { - if ( this.detachInactiveScreenViews && this.display.simulationRoot.hasChild( screen.view ) ) { - this.display.simulationRoot.removeChild( screen.view ); - } + // if ( this.detachInactiveScreenViews && this.display.simulationRoot.hasChild( screen.view ) ) { + // this.display.simulationRoot.removeChild( screen.view ); + // } - screen.activeProperty.set( visible ); + // screen.activeProperty.set( visible ); } } ); this.updateBackground();

[samreid]

Running buoyancy with ?webgl=false shows only 5 VM IOSurfaces. Surprisingly, changing the warning message to use canvas, starts leaking IOSurfaces:

```diff Subject: [PATCH] 123 --- Index: mobius/js/ThreeUtils.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/mobius/js/ThreeUtils.ts b/mobius/js/ThreeUtils.ts --- a/mobius/js/ThreeUtils.ts (revision 985e0f3732e48c9680675991d42b5a2c0646b4ef) +++ b/mobius/js/ThreeUtils.ts (date 1724029062151) @@ -246,6 +246,7 @@ */ showWebGLWarning( screenView: ScreenView ): void { const warningNode = new HBox( { + renderer: 'canvas', children: [ new Path( exclamationTriangleSolidShape, { fill: '#E87600', // "safety orange", according to Wikipedia Index: joist/js/ScreenView.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/joist/js/ScreenView.ts b/joist/js/ScreenView.ts --- a/joist/js/ScreenView.ts (revision 232f3a3f3d327ac83522aaf45150bfffa07dcba2) +++ b/joist/js/ScreenView.ts (date 1724027360249) @@ -77,7 +77,7 @@ // Node options layerSplit: true, // so we're not in the same layer as the navbar, etc. - excludeInvisible: true, // so we don't keep invisible screens in the SVG tree + excludeInvisible: false, // so we don't keep invisible screens in the SVG tree // phet-io tandem: Tandem.REQUIRED, Index: scenery/js/display/CanvasBlock.js IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/scenery/js/display/CanvasBlock.js b/scenery/js/display/CanvasBlock.js --- a/scenery/js/display/CanvasBlock.js (revision 556af6920d89fa2b018c2693904fd62ac621f53c) +++ b/scenery/js/display/CanvasBlock.js (date 1724028811572) @@ -170,6 +170,8 @@ return false; } + return false; + sceneryLog && sceneryLog.CanvasBlock && sceneryLog.CanvasBlock( `update #${this.id}` ); sceneryLog && sceneryLog.CanvasBlock && sceneryLog.push(); Index: scenery/js/display/WebGLBlock.js IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/scenery/js/display/WebGLBlock.js b/scenery/js/display/WebGLBlock.js --- a/scenery/js/display/WebGLBlock.js (revision 556af6920d89fa2b018c2693904fd62ac621f53c) +++ b/scenery/js/display/WebGLBlock.js (date 1724028811547) @@ -156,9 +156,9 @@ this.canvas.addEventListener( 'webglcontextlost', this.contextLostListener, false ); this.canvas.addEventListener( 'webglcontextrestored', this.contextRestoreListener, false ); - this.domElement.appendChild( this.canvas ); + // this.domElement.appendChild( this.canvas ); - this.setupContext( gl ); + // this.setupContext( gl ); } sceneryLog && sceneryLog.WebGLBlock && sceneryLog.pop(); ```

[samreid]

If you get the dom element size small enough, there is no leaking of IOSurfaces:

```diff Subject: [PATCH] 123 --- Index: mobius/js/ThreeUtils.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/mobius/js/ThreeUtils.ts b/mobius/js/ThreeUtils.ts --- a/mobius/js/ThreeUtils.ts (revision 985e0f3732e48c9680675991d42b5a2c0646b4ef) +++ b/mobius/js/ThreeUtils.ts (date 1724029774729) @@ -246,6 +246,7 @@ */ showWebGLWarning( screenView: ScreenView ): void { const warningNode = new HBox( { + renderer: 'canvas', children: [ new Path( exclamationTriangleSolidShape, { fill: '#E87600', // "safety orange", according to Wikipedia @@ -266,14 +267,14 @@ warningNode.mouseArea = warningNode.touchArea = warningNode.localBounds; - warningNode.addInputListener( { - up: function() { - const joistGlobal = _.get( window, 'phet.joist', null ); // returns null if global isn't found - const locale = joistGlobal ? joistGlobal.sim.locale : 'en'; - - openPopup( `https://phet.colorado.edu/webgl-disabled-page?simLocale=${locale}` ); - } - } ); + // warningNode.addInputListener( { + // up: function() { + // const joistGlobal = _.get( window, 'phet.joist', null ); // returns null if global isn't found + // const locale = joistGlobal ? joistGlobal.sim.locale : 'en'; + // + // openPopup( `https://phet.colorado.edu/webgl-disabled-page?simLocale=${locale}` ); + // } + // } ); } }; Index: scenery/js/display/Display.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/scenery/js/display/Display.ts b/scenery/js/display/Display.ts --- a/scenery/js/display/Display.ts (revision 556af6920d89fa2b018c2693904fd62ac621f53c) +++ b/scenery/js/display/Display.ts (date 1724039092346) @@ -746,12 +746,12 @@ if ( this.size.width !== this._currentSize.width ) { sizeDirty = true; this._currentSize.width = this.size.width; - this._domElement.style.width = `${this.size.width}px`; + this._domElement.style.width = `600px`; } if ( this.size.height !== this._currentSize.height ) { sizeDirty = true; this._currentSize.height = this.size.height; - this._domElement.style.height = `${this.size.height}px`; + this._domElement.style.height = `600px`; } if ( sizeDirty && !this._allowSceneOverflow ) { // to prevent overflow, we add a CSS clip

[samreid]

Self-contained example leaks 311 IOSurfaces within 30 seconds:

```html

```

UPDATE: Same test shows no leak on iOS simulator

[samreid]

Leaked 20GB in 34 minutes in buoyancy. When will it crash?

[samreid]

It could be that there were crashes in the above trace but the instrumentation just kept going, but I did watch carefully for 5 minutes on a new run to see it go >4GB and it did not crash.

I used the built buoyancy sim for several minutes and it did not crash. I'm not sure how we will confidently proceed here.

Let's talk at standup about what else should be done before QA test.

[samreid]

At today's standup, we discussed that we have been able to run on iPad pretty well and aren't too concerned about more work on it before QA. We agreed the proposed refactoring to use a single webgl context sounds like it would be very difficult and we don't want to do it. When we QA test, we would like to request testing on iPhones. Closing.

[Nancy-Salpepi]

I have an iPad 9th generation with iOS 17.6.1. I started testing Buoyancy in https://github.com/phetsims/qa/issues/1136 and experienced 2 crashes within 5 minutes of testing. Unfortunately, it happens randomly--one time while switching screens, another time while on the Compare screen--so there aren't steps that I can give you to reproduce but interactive highlights was on in both cases. I have noticed that the crash happens pretty easily when adding a query parameter(e.g. ?stringTest=double). The sim crashes before it even finishes loading.

[Nancy-Salpepi]

On my iPhone 12 Pro with iOS 17.5.1, the sim immediately crashes and I get this message:

[Nancy-Salpepi]

Another easy way to get my ipad to crash is to go to the Compare, Lab, or Shapes screen and try and take a screenshot using the tool from the PhET Menu. EDIT: Turn on interactive highlights first

[samreid]

@Nancy-Salpepi can you please see how much it crashes if running with ?screens=1 and ?screens=1,2

Can it be crashed without interactive highlights?

[Nancy-Salpepi]

can you please see how much it crashes if running with ?screens=1 and ?screens=1,2

--I didn't get a crash without interactive highlights on with the above query parameters. With ?screens=4 and a styrofoam duck, taking a screenshot with the tool in the PhET menu causes a crash every time. Aside from that, I didn't get a crash.

--Sometimes the sim crashes when I am modifying the url. https://github.com/user-attachments/assets/fb97732a-219c-4337-ab93-40b23e0ed780

Can it be crashed without interactive highlights?

--It repeatedly crashes on my iPhone. It never even opens at all. --on the iPad, it has only crashed twice without interactive highlights on over a large span of time.

I did tether the iPad to my mac and webGL warning messages do pop up when the sim loads (but nothing further when it crashes):

I also asked @Matthew-Moore240 and @Ashton-Morris to take a look on their iPhones since they have a different model.

Let me know if there is anything else I can do.

[Matthew-Moore240]

No crashes for me but the sim does have various UI issues. Pictures are under details.

 

[Ashton-Morris]

iPhone 15 pro max.

Latest iOS

At first it didn't crash and the UI looked fine. But after reloading it a few times it did crash with the same message as above with Nancy.

At one point it asked me if I wanted to lower privacy restrictions-in safari when using the sim to improve performance (this was before the and after crash).

It worked well for a long time and then crashed again after reloading it. Tried it on regular and private browsing in safari mobile.

[Matthew-Moore240]

iPhone 15 pro max.

Latest iOS

At first it didn't crash and the UI looked fine. But after reloading it a few times it did crash with the same message as above with Nancy.

At one point it asked me if I wanted to lower privacy restrictions-in safari when using the sim to improve performance (this was before the and after crash).

It worked well for a long time and then crashed again after reloading it. Tried it on regular and private browsing in safari mobile.

I also experienced a crash after reloading the page a couple of times. I had forgotten to turn off an app that controls dark mode in the browser, which was causing the weird graphical issues. It still crashes after a few reloads but the sim looks normal now. Apologies!

[Nancy-Salpepi]

I didn't experience any crashes with ?webgl=false, including 10 minutes of fuzzing.

[samreid]

Nice! Can you please run another test. Delete ?webgl=false (so webgl is back on, and we will see the blocks). And try a test with supportsInteractiveDescription=false instead? Thanks!!

[Nancy-Salpepi]

With supportsInteractiveDescription=false the sim crashed twice in 5 minutes while fuzzing. Aside from that, I was only able to get it to crash when taking a screenshots during ~25 minutes of testing.

[samreid]

@Nancy-Salpepi volunteered to test supportsInteractiveDescription=false on the iPhone, thanks!

Should we test https://phet.colorado.edu/sims/html/density/latest/density_all.html (density 1.1) for crashing behavior, so we can narrow down when it was introduced?

[zepumph]

• This isn't that helpful, but I wanted to share that fuzzing on my Google pixel7a (android phone) didn't crash for about 20 minutes.
• I also used @samreid's suggestion of using the "memory" checkbox on the performance tab, which shows this sawtooth trend as the js heap increases to >1000MB in 200 seconds:

[Nancy-Salpepi]

OK! here is the latest update from me:

• On my iPhone 12 Pro I fuzz tested Density and Buoyancy Basics for 10 minutes each and had no crashes.
• On my iPhone 12 Pro I added ?supportsInteractiveDescription=false to the Buoyancy url and it still crashes repeatedly.

[samreid]

@Nancy-Salpepi these tests are very helpful. Let us test the hypothesis that the complex geometries (boat, bottle, duck, etc) may be related to the problem. Can you test this next?

• iPhone 12 Pro 10-minute fuzz testing buoyancy with ?screens=1,2,3

[Nancy-Salpepi]

On my iPhone: Buoyancy opens with ?screens=1,2,3, ?screens=1,2,3,4, ?screens=1,2,3,5, and ?screens=4,5

10 minutes of fuzz testing with ?screens=1,2,3 produced no crashes 10 minutes of fuzz testing with ?screens=4,5 produced no crashes 10 minutes of fuzz testing with ?screens=1,2,4 produced no crashes

[samreid]

I added a script so I can tell how many times the sim has crashed, otherwise you have to watch very carefully to see when the crash occurs:

```diff Subject: [PATCH] Convert to *.ts, see https://github.com/phetsims/density-buoyancy-common/issues/377 --- Index: chipper/js/initialize-globals.js IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/chipper/js/initialize-globals.js b/chipper/js/initialize-globals.js --- a/chipper/js/initialize-globals.js (revision a3f46af15234cd27121cdebefef316470004ff96) +++ b/chipper/js/initialize-globals.js (date 1724968070968) @@ -151,6 +151,10 @@ public: true }, + crashCounter: { + type: 'flag' + }, + /** * enables debugger commands in certain cases like thrown errors and failed tests. */ Index: joist/js/Sim.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/joist/js/Sim.ts b/joist/js/Sim.ts --- a/joist/js/Sim.ts (revision 8d73b67cd74cf759647afd6c0a95f8c82dd13955) +++ b/joist/js/Sim.ts (date 1724978117472) @@ -74,6 +74,7 @@ import isSettingPhetioStateProperty from '../../tandem/js/isSettingPhetioStateProperty.js'; import StringIO from '../../tandem/js/types/StringIO.js'; import dotRandom from '../../dot/js/dotRandom.js'; +import TinyProperty from '../../axon/js/TinyProperty.js'; // constants const PROGRESS_BAR_WIDTH = 273; @@ -262,6 +263,24 @@ */ public constructor( simNameProperty: TReadOnlyProperty, allSimScreens: AnyScreen[], providedOptions?: SimOptions ) { + if ( phet.chipper.queryParameters.crashCounter ) { + + // get the crash count from local storage, or set it to 1. + const storedRunCountString = window.localStorage.getItem( 'phet.runCount' ); + + let newRunCount: number | null = null; + if ( storedRunCountString ) { + const storedRunCount = parseInt( storedRunCountString, 10 ); + newRunCount = storedRunCount + 1; + } + else { + newRunCount = 1; + } + window.localStorage.setItem( 'phet.runCount', newRunCount.toFixed( 0 ) ); + simNameProperty = new TinyProperty( simNameProperty.value + ' (Run ' + newRunCount + ')' ); + + } + window.phetSplashScreenDownloadComplete(); assert && assert( allSimScreens.length >= 1, 'at least one screen is required' ); ```

In this patch, I saw many many crashes on my iPad and iPhone. Here is a table that notes the run number as a function of time. Successive run numbers indicate crashes. This is testing a new build from main (includes recent grab drag handler keys, which have a memory leak):

iPad fuzzing built version
6:43pm run 1 starting crash at 6:45pm. Correctly says "run 2"
6:45pm run 2
6:50pm run 3
6:52pm run 4
6:56pm run 5
6:58pm run 6 - NOTE: it is crashing around once every 3 minutes
7:00pm run 7
7:03pm run 8
7:07pm run 9
7:14pm run 10
7:17pm run 11

iphone 15 pro max full built just crashes on startup.
Setting screens=1,2,3 it can launch.
screens=1,2,3,4 crashes on startup
screens=1,2,3,5 fuzzes

screens=4 (shapes)
6:56pm run 3
6:58pm run 4 NOTE: crashed with only one screen, but it was the shapes screen
7:00pm run 4
7:03pm run 5
7:07pm run 7
7:10pm run 8
7:17pm run 9

However, in chrome, it just looks like a standard memory leak:

Let's make sure the memory leak is stopped before running more tests here. My results here are all for a build on main, not for the dev test.

[samreid]

@Nancy-Salpepi I wanted to double check that the tests you reported above are for the built dev test https://phet-dev.colorado.edu/html/buoyancy/1.2.0-dev.4/phet/buoyancy_all_phet.html?fuzz (or maybe _en?) and not on phettest. Let me know either way.

[Nancy-Salpepi]

Yes. They were for the dev version. Let me know if there is something else you need me to check or recheck....or if you wanted me to do some memory testing with the dev versions.

[samreid]

Patch that adds a query parameter ?threeRendererPixelRatio:

```diff Subject: --- Index: chipper/js/initialize-globals.js IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/chipper/js/initialize-globals.js b/chipper/js/initialize-globals.js --- a/chipper/js/initialize-globals.js (revision 3f1c5bac01380a2a4e758218388cd2ac567bd98a) +++ b/chipper/js/initialize-globals.js (date 1725474504480) @@ -463,6 +463,11 @@ */ preserveDrawingBuffer: { type: 'flag' }, + threeRendererPixelRatio: { + type: 'number', + defaultValue: window.devicePixelRatio || 1 + }, + /** * If true, the full screen button won't be shown in the phet menu */ Index: mobius/js/ThreeStage.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/mobius/js/ThreeStage.ts b/mobius/js/ThreeStage.ts --- a/mobius/js/ThreeStage.ts (revision f0c1b31e7f905a4872fa3a6daeb52c691c930fc7) +++ b/mobius/js/ThreeStage.ts (date 1725475015612) @@ -20,6 +20,7 @@ import mobius from './mobius.js'; import TReadOnlyProperty from '../../axon/js/TReadOnlyProperty.js'; import TEmitter from '../../axon/js/TEmitter.js'; +import platform from '../../phet-core/js/platform.js'; // hard-coded gamma (assuming the exponential part of the sRGB curve as a simplification) const GAMMA = 2.2; @@ -74,7 +75,7 @@ if ( ThreeUtils.isWebGLEnabled() ) { try { this.threeRenderer = new THREE.WebGLRenderer( { - antialias: true, + antialias: !platform.mobileSafari, alpha: true, preserveDrawingBuffer: phet.chipper.queryParameters.preserveDrawingBuffer } ); @@ -84,7 +85,7 @@ console.log( e ); } } - this.threeRenderer && this.threeRenderer.setPixelRatio( window.devicePixelRatio || 1 ); + this.threeRenderer && this.threeRenderer.setPixelRatio( phet.chipper.queryParameters.threeRendererPixelRatio ); // Dialog shown on context loss, constructed lazily because Dialog requires sim bounds during construction this.contextLossDialog = null; ```

[samreid]

Patch that adds an option to antialias the three renderer:

```diff Subject: [PATCH] Add ?launchCounter, see https://github.com/phetsims/joist/issues/980 --- Index: density-buoyancy-common/js/common/view/DensityBuoyancyScreenView.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/density-buoyancy-common/js/common/view/DensityBuoyancyScreenView.ts b/density-buoyancy-common/js/common/view/DensityBuoyancyScreenView.ts --- a/density-buoyancy-common/js/common/view/DensityBuoyancyScreenView.ts (revision 99c36c581108166c099568dd9514128bd338fb2d) +++ b/density-buoyancy-common/js/common/view/DensityBuoyancyScreenView.ts (date 1725476000275) @@ -58,6 +58,7 @@ import PoolMesh from './mesh/PoolMesh.js'; import BarrierMesh from './mesh/BarrierMesh.js'; import FluidMesh from './mesh/FluidMesh.js'; +import platform from '../../../../phet-core/js/platform.js'; // constants const MARGIN = DensityBuoyancyCommonConstants.MARGIN_SMALL; @@ -118,7 +119,10 @@ }, // So the sky background will show through - backgroundColorProperty: new ColorProperty( Color.TRANSPARENT ) + backgroundColorProperty: new ColorProperty( Color.TRANSPARENT ), + + // Reduce memory usage on mobile safari to prevent crashing + threeRendererAntialias: !platform.mobileSafari }, cameraLookAt: DensityBuoyancyCommonConstants.DENSITY_CAMERA_LOOK_AT, cameraZoom: 1.75 * scaleIncrease, @@ -178,7 +182,7 @@ // TODO: Don't we want to clear out the cursor if we don't have a pointer? https://github.com/phetsims/density-buoyancy-common/issues/363 if ( newPointer ) { - + // When the mouse hovers over a mass, show the cursor hand const massUnderPointerEntry = this.getMassViewUnderPointer( newPointer ); if ( newPointer instanceof Mouse ) { Index: mobius/js/ThreeStage.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/mobius/js/ThreeStage.ts b/mobius/js/ThreeStage.ts --- a/mobius/js/ThreeStage.ts (revision f0c1b31e7f905a4872fa3a6daeb52c691c930fc7) +++ b/mobius/js/ThreeStage.ts (date 1725476658687) @@ -30,6 +30,8 @@ // The initial camera position cameraPosition?: Vector3; + + threeRendererAntialias?: boolean; }; export default class ThreeStage { @@ -56,7 +58,8 @@ const options = optionize()( { backgroundColorProperty: new Property( Color.BLACK ), - cameraPosition: new Vector3( 0, 0, 10 ) + cameraPosition: new Vector3( 0, 0, 10 ), + threeRendererAntialias: true }, providedOptions ); this.activeScale = 1; @@ -74,7 +77,7 @@ if ( ThreeUtils.isWebGLEnabled() ) { try { this.threeRenderer = new THREE.WebGLRenderer( { - antialias: true, + antialias: options.threeRendererAntialias, alpha: true, preserveDrawingBuffer: phet.chipper.queryParameters.preserveDrawingBuffer } ); ```

The iPhone 15 Pro Max crashes repeatedly on startup with antialias=true, but both iPad 7 and iPhone have been fuzzing with antialias=false for > 15 minutes with no crashes.

I'll request review from @zepumph, particularly about whether we want to use the nested options pattern or not. After that's committed, we could request testing (on a built version) from QA. Or could do that in next dev or rc test. Or if we want more tuning parameters, we could add threeRendererAntialias and threeRendererPixelScale to the query parameters.

[samreid]

I sent this to @jonathanolson and @zepumph in slack:

We found that by disabling antialiasing on mobile safari, we were able to fuzz 15+ minutes with no crashing on the iPad and iPhone, so that seemed good. We also saw that on the iPhone 15 Pro Max the pixel density is 3, so on mobile safari we also reduced the pixel ratio to 0.9 * devicePixelRatio. It still looks good enough on iPad and iPhone. We additionally created query parameters in MobiusQueryParameters. now it has

• threeRendererPreserveDrawingBuffer
• threeRendererPixelRatio
• threeRendererAntialias

I also took several (10+) screenshots on the iPad and it did not crash.

We think we are ready to see how it fares on QA devices. If QA shows consistent crashing, we can request additional testing with the query parameters to see how it affects things.

I also hypothesized that mobile safari sometimes has a hidden memory state, and that one crash somehow "clears" it, so that once it crashes (perhaps on first launch) then it autoreloads and has plenty of memory to continue.

I’m convinced that using a single webgl canvas would be a good memory savings, but I’m also concerned that could be time consuming, and/or a source of complexity or bugs. But that’s probably just uncertainty since I don’t know how it would be implemented or the tradeoffs.

The next time we begin a fresh a webgl-intensive simulation I recommend we investigate memory limits and crashing behavior on devices early on. Developing a new simulation with a shared canvas may be easier than retrofitting one?

[samreid]

Here's a screenshot from the iPad 7 with aliasing and 0.9x pixel ratio:

By the way, we also observed that with these changes, the frame rate when dragging a block on the iPad is significantly higher. It went from roughly 25fps to roughly 60fps and feels much snappier. Dragging boat or bottle remains slow though.