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

Electrons are missing from Screenshot. #116

Closed
pixelzoom opened this issue Mar 26, 2024 · 5 comments
Closed

Electrons are missing from Screenshot. #116

pixelzoom opened this issue Mar 26, 2024 · 5 comments
Assignees
Labels
type:bug Something isn't working

Comments

@pixelzoom
Copy link
Contributor

pixelzoom commented Mar 26, 2024

Reported by @kathy-phet on Slack. The electrons are not captured when you take a "Screenshot" via the PhET menu, see below. Microsoft Windows 11 Enterprise, Chrome 123.

This is probably a scenery problem with Sprites, see ElectronsNode.ts. But since the field visualization (FieldNode.ts) is also using Sprites, I don't know why it would be captured while the electrons are not.

Faraday's Electromagnetic Lab screenshot

@pixelzoom pixelzoom added the type:bug Something isn't working label Mar 26, 2024
@pixelzoom
Copy link
Contributor Author

I see this problem on macOS 14.4 with all browsers: Chrome 123, Safari 17.4, Firefox 123.

@pixelzoom
Copy link
Contributor Author

pixelzoom commented Mar 26, 2024

FEL Sprites are currently rendered using Canvas because of the problem in #109. But setting webgl: true in FELSim for does NOT fix the problem.

@jonathanolson
Copy link
Contributor

I see canvasBounds being set in FieldNode, but NOT in ElectronsNode. I believe the absence of this is preventing it from showing up in screenshots.

This seems like a large landmine I've laid. I think it is reasonable to say "if a Sprites instance draws something, it should have a non-negative-area bounds". I think it's also reasonable to add an assertSlow that says "every sprite drawn with non-zero alpha should overlap with the CanvasBounds" (it would be stronger to say contained inside, HOWEVER we might have sprites with a good amount of transparent padding).

@jonathanolson
Copy link
Contributor

Added canvasBounds in ElectronsNode (let me know if that's not good for performance), and assertions in Scenery that should get the majority of things. I'm going to opt out of the assertSlow for now, potentially to add in the future (but it isn't as trivial to add).

@pixelzoom
Copy link
Contributor Author

Thanks @jonathanolson. Performance seems fine, and electrons are now captured in Screenshot.

Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants