Skip to content
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

Performance testing and sign-off #99

Closed
pixelzoom opened this issue Mar 13, 2024 · 16 comments
Closed

Performance testing and sign-off #99

pixelzoom opened this issue Mar 13, 2024 · 16 comments

Comments

@pixelzoom
Copy link
Contributor

pixelzoom commented Mar 13, 2024

Specifically from the CRC:

  • Play with sim, identify any obvious performance issues. Examples: animation that slows down with large numbers of objects; animation that pauses or "hitches" during garbage collection.
  • If the sim uses WebGL, does it have a fallback? Does the fallback perform reasonably well? (run with query
    parameter webgl=false)
  • Are UI components sufficiently responsive? (especially continuous UI components, such as sliders)

No performance issues were noted in dev-lite test phetsims/qa#1051, but that testing was on macOS and Win11, so did not include any performance-challenged devices (Chromebook, older iPad).

@arouinfar and I will review. If you have access to a performance-challenged device, that would be ideal.

@pixelzoom
Copy link
Contributor Author

The oldest device that I have access to is an iPad6 (circa 2018) running iPadOS 17.1.2. Performance seemed fine, everything responsive, animation smooth.

@pixelzoom pixelzoom removed their assignment Mar 14, 2024
@arouinfar
Copy link
Contributor

Thanks @pixelzoom. I should be on campus tomorrow, so I can test on Chromebook and whatever performance-limited iPad QA recommends.

@pixelzoom
Copy link
Contributor Author

pixelzoom commented Mar 18, 2024

@Nancy-Salpepi volunteer to test on a performance-challenged device. A Chromebook would be great -- please indicate which device(s) you use for testing, and what the performance is like on each device. If there are cases where performance is less than great, please describe whether usability is still OK, compromised, or unusable. Thanks!

Things to check:

  • Bar Magnet screen: Smoothness/responsiveness for magnetic field (grid of compass needles) as you drag the bar magnet.
  • Bar Magnet screen: Kinematics of compass as it is being dragged, and as it comes to rest.
  • Electromagnet screen: Animation of AC power supply waveform.
  • Electromagnet screen: Animation of electrons with AC power supply selected.
  • Generator screen: Animation of everything when the water faucet is turned on.
  • All screens: Responsiveness when dragging objects.
  • All screens: Watch for any brief pauses or "hitches" in animation that is indicative of garbage collection.

@Nancy-Salpepi
Copy link

Nancy-Salpepi commented Mar 18, 2024

Device: Lenovo 100e Chromebook ChromeOS version 121.0.6167.188:
On the Transformer and Electromagnet screens, when I move the mouse over the battery's coils, the electrons will briefly stop moving when the pointer arrow changes to the hand (it is more obvious on the Transformer screen). I don't see this issue with my macBook Air.

electronsStop.mov

I also see extremely brief pauses periodically on the Generator screen. Most people probably wouldn't even notice it, but if I am looking at the bar magnet I see it.

I went through the list above and didn't see any other issues.

@pixelzoom
Copy link
Contributor Author

pixelzoom commented Mar 18, 2024

Thanks @Nancy-Salpepi. What are your thoughts on:

If there are cases where performance is less than great, please describe whether usability is still OK, compromised, or unusable.

And a couple of questions about the pausing electron animation:

  1. Does the electron animation also pause when you move the pointer off the electromagnet coil, and the cursor changes back to an arrow?
  2. Does the electron animation also pause when you move the mouse over the pickup coil in the Transformer screen?

@pixelzoom
Copy link
Contributor Author

I asked @jonathanolson about the electron animation in Slack#DM. He said:

Not sure, potentially a performance thing? Perhaps kite's shape-hit-testing failing out? Probably would want to reproduce with a debugger

@Nancy-Salpepi
Copy link

If there are cases where performance is less than great, please describe whether usability is still OK, compromised, or unusable.

Usability is still OK.

Does the electron animation also pause when you move the pointer off the electromagnet coil, and the cursor changes back to an arrow?

Yes it does. Sorry that wasn't clear in my video.

Does the electron animation also pause when you move the mouse over the pickup coil in the Transformer screen?

Not that I can see.

Let me know if there is anything else!

@Nancy-Salpepi
Copy link

Actually--changing my answer to that last question. The electron animation on the Battery stops when I move my pointer over and then away from the pickup coil.

electronsStop2.mov

@pixelzoom
Copy link
Contributor Author

Thanks @Nancy-Salpepi.

@pixelzoom pixelzoom assigned pixelzoom and unassigned Nancy-Salpepi Mar 19, 2024
@arouinfar
Copy link
Contributor

Thanks for testing @Nancy-Salpepi!

@pixelzoom I think it's worth some additional investigation to see if we can prevent the current animation from stuttering.

@arouinfar arouinfar removed their assignment Mar 19, 2024
@pixelzoom
Copy link
Contributor Author

@jonathanolson We would like to investigate why the cursor change is causing the sim to pause. I don't know where to start, and I cannot reproduce the problem on any of my devices. Thoughts on how to proceed?

@KatieWoe
Copy link

I checked this on a slightly better chromebook. While I did see this, it was slight enough I don't think I would have noticed if not looking for it.

@jonathanolson
Copy link
Contributor

image

I'm seeing style recalculation based on the cursor. I think we've run into bits of this before. It looks like some potential mitigations would actually make it worse (https://gist.github.com/paulirish/5d52fb081b3570c81e3a?permalink_comment_id=3799355#gistcomment-3799355 and document.body.style.cursor).

There is a chance creating a single "overlay" DOM element could help (if we put the cursor on that), however that blocks a number of use cases (and prevents us from using DOM-Scenery nodes with interaction in the future), so I'm hesitant to even go down that route.

I'm going to check into a few approaches that would also enable the DOM-Scenery nodes in a better way (if we remove SVG elements from hit-testing, cursor changes might not take so long in style recalculation).

@jonathanolson
Copy link
Contributor

I don't see a good/easy way to proceed to increase performance in that case, besides generally simplifying the content of the simulation (reducing nodes).

@jonathanolson jonathanolson removed their assignment Mar 28, 2024
@pixelzoom
Copy link
Contributor Author

pixelzoom commented Mar 29, 2024

The electrons are already pickable: false, so there's no optimization available there.

We could use a simpler shape for the background layer's pointer areas, allowing the user to grab the coil between the wires. I can't think of any cases where that would prevent the user from grabbing something, because there's nothing interactive behind the coil's background layer.

Doing the same (using a simpler shape for pointer areas) for the foreground layer would mean that things "inside the coil" could not be grabbed between the wires. That would be inconvient for a few cases -- like this one, where you would not be able to grab the compass:

screenshot_3152

All of that said... My inclination is to live with the slight pause when the cursor changes.

@arouinfar Let's discuss.

@arouinfar
Copy link
Contributor

Thanks for the options @pixelzoom.

All of that said... My inclination is to live with the slight pause when the cursor changes.

Given @KatieWoe's assessment in #99 (comment), I'm fine with leaving it as-is. Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants