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

Screenshots missing photons #255

Closed
Tracked by #885
Nancy-Salpepi opened this issue Jan 6, 2023 · 6 comments
Closed
Tracked by #885

Screenshots missing photons #255

Nancy-Salpepi opened this issue Jan 6, 2023 · 6 comments
Labels

Comments

@Nancy-Salpepi
Copy link

Test device
MacBook Air (m1 chip) and Dell

Operating System
MacOS 13.1 and Win10

Browser
safari and FF

Problem description
For phetsims/qa#871, the photons don't appear in screenshots

Visuals
Screenshot 2023-01-06 at 5 03 49 PM

Troubleshooting information: !!!!! DO NOT EDIT !!!!! Name: ‪Greenhouse Effect‬ URL: https://phet-dev.colorado.edu/html/greenhouse-effect/1.1.0-rc.1/phet/greenhouse-effect_all_phet.html Version: 1.1.0-rc.1 2022-12-23 00:15:29 UTC Features missing: applicationcache, applicationcache, touch Flags: pixelRatioScaling User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/16.2 Safari/605.1.15 Language: en-US Window: 1216x617 Pixel Ratio: 2/1 WebGL: WebGL 1.0 GLSL: WebGL GLSL ES 1.0 (1.0) Vendor: WebKit (WebKit WebGL) Vertex: attribs: 16 varying: 30 uniform: 1024 Texture: size: 16384 imageUnits: 16 (vertex: 16, combined: 32) Max viewport: 16384x16384 OES_texture_float: true Dependencies JSON: {}
@Nancy-Salpepi Nancy-Salpepi added the type:bug Something isn't working label Jan 6, 2023
@samreid
Copy link
Member

samreid commented Jan 11, 2023

I was able to reproduce the problem on my MacBook Air M1 on Chrome.

@samreid
Copy link
Member

samreid commented Jan 11, 2023

We just need to pass in the canvasBounds like so:

Subject: [PATCH] Ignore invisible nodes in move forward/backward, see https://github.com/phetsims/sun/issues/814
---
Index: js/common/PhotonSprites.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/common/PhotonSprites.ts b/js/common/PhotonSprites.ts
--- a/js/common/PhotonSprites.ts	(revision c0fd52d298c3a6c9503247c2b107e1ee6490a740)
+++ b/js/common/PhotonSprites.ts	(date 1673477063918)
@@ -25,6 +25,7 @@
 import Range from '../../../dot/js/Range.js';
 import LayersModel from './model/LayersModel.js';
 import greenhouseEffect from '../greenhouseEffect.js';
+import Bounds2 from '../../../dot/js/Bounds2.js';
 
 // constants
 const TARGET_PHOTON_IMAGE_WIDTH = 17; // empirically determined to match the design
@@ -62,7 +63,8 @@
       sprites: [ visiblePhotonSprite, infraredPhotonSprite ],
       spriteInstances: spriteInstances,
       renderer: 'webgl',
-      pickable: false
+      pickable: false,
+      canvasBounds: new Bounds2( 0, 0, 500, 500 )
     } );
 
     // Calculate the scale that will be used to render the photon images.

Maybe @jbphet can help specify that dimension.

samreid added a commit that referenced this issue Jan 12, 2023
@samreid
Copy link
Member

samreid commented Jan 12, 2023

I used the same strategy for specifying the canvasBounds as used in WaveLandscapeObservationWindow and it appears to work well. I also tested by eroding it by 10px and it behaved as expected, so it seems well-fitted to the window. Next I will commit it to the release branch as @jbphet has been doing for other issues.

samreid added a commit that referenced this issue Jan 12, 2023
(cherry picked from commit f459a3b)
@samreid
Copy link
Member

samreid commented Jan 12, 2023

OK I also pushed to the 1.1 branch. @jbphet will you please review?

@jbphet
Copy link
Contributor

jbphet commented Jan 13, 2023

Looks great, thanks @samreid! I'll have QA verify this in the next round of testing.

@Nancy-Salpepi
Copy link
Author

I see photons with mac + chrome/safari!
Closing.

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

No branches or pull requests

3 participants