Tools jump to non-existent points of interest in light scene

[Nancy-Salpepi]

Test device
MacBook Air (m1 chip)

Operating System
12.3.1

Browser
Chrome

Problem description
For phetsims/qa#796 and related to #395

In lens screen, with the light and the concave lens: using the 'J' key to jump to points of interest takes the rulers and position markers to non-existent points of interest. It seems like they are moving to where the virtual image and second point of virtual image would be if it was a framed object.

Steps to reproduce

1. Select the concave lens in the Lens screen
2. Select the light
3. Tab to the horizontal ruler and press space to select
4. Press the 'J' key several times
5. Repeat steps 3-4 with the vertical ruler and position markers

Visuals

lighthorizontalruler.mp4

Troubleshooting information: !!!!! DO NOT EDIT !!!!! Name: ‪Geometric Optics‬ URL: https://phet-dev.colorado.edu/html/geometric-optics/1.1.1-rc.1/phet/geometric-optics_all_phet.html Version: 1.1.1-rc.1 2022-04-11 14:16:55 UTC Features missing: applicationcache, applicationcache, touch Flags: pixelRatioScaling User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/100.0.4896.75 Safari/537.36 Language: en-US Window: 1430x690 Pixel Ratio: 2/1 WebGL: WebGL 1.0 (OpenGL ES 2.0 Chromium) GLSL: WebGL GLSL ES 1.0 (OpenGL ES GLSL ES 1.0 Chromium) Vendor: WebKit (WebKit WebGL) Vertex: attribs: 16 varying: 31 uniform: 1024 Texture: size: 16384 imageUnits: 16 (vertex: 16, combined: 32) Max viewport: 16384x16384 OES_texture_float: true Dependencies JSON: {}

[arouinfar]

Really nice find @Nancy-Salpepi.

I only see this happing with the light/concave lens combo. The markers also jump to the same x-coordinates as the horizontal ruler, but reveal a non-zero y-coordinate. I have no idea what these points of interest are meant to represent, but if you add the second light and move the lights around, the marker always touches the central ray.

As weird as this looks, I really would like to punt on it until 1.2. It is critical to publish the maintenance release to fix #424 as soon as possible because it is a very visible bug that users are going to notice.

[pixelzoom]

This "jump" feature is so complicated and fragile. I hope someone finds it useful, but I'll be surprised if most people can figure out what it does without scaffolding. That said...

Agreed, let's defer this to 1.2 release.

[pixelzoom]

The problem occurs with all tools. As @Nancy-Salpepi surmised, these are the optical image points, where an optical image would be formed. No optical image is formed in the Light scene, but these points are nevertheless interesting -- sometimes. When the optic is converging (convex lens) these points are were the rays converge, on the right side of the lens, and they are useful measurement points -- see screenshot below. When the optic is diverging (concave lens), the rays do not converge, and these points are NOT useful.

I'll investigate ignoring these points for the Light scene when the lens is concave.

[pixelzoom]

I also realized that the positions of the light spots are not really "interesting" for the concave lens. The diameter of the spot is huge, so there's no distint "spot" on the screen. And the position doesn't aligned with the center marginal ray (see screenshot below) -- I'm not sure if that's correct or incorrect, and it's not easy to determine.

So... I've also exlcuded the positions of the light spots for the concave lens.

[pixelzoom]

Another problem (?), related to convex lens. There's currently nothing that knows whether a light spot intersects the project screen. A light spot is always drawn, and it is simply clipped (via scenery) to the projection screen. So if the center of a light spot is not on the projection screen, jumping to that light spot can look like the 2 examples below -- note the position marker. The position marker is at the position of the light spot in the vertical plane of the screen.

@arouinfar do you think this needs to be addressed?

[pixelzoom]

@arouinfar while you're replying.... Please review the changes that I've made so far. For concave lens in the Light scene, I've removed (1) the optical image positions and (2) the light spot positions from the jump points.

[pixelzoom]

I thought of a way to address #426 (comment). It involves several changes, but nothing particularly tricky:

• Add a constant to ProjectionScreen that indicates how tall the screen is at its center-x. const averageScreenHeight looks like it's what we need, change it to public readonly height: number;

• Add visibleProperty to LightSpot, derived from postionProperty, diameterProperty, the projection screen's positionProperty, the projection screen's height, and the opticalImage visibleProperty (for animation). Something like this in LightSpot.ts:

this.visibleProperty = new DerivedPropery( 
  [ this.positionProperty, this.diameterProperty, projectionScreen.positionProperty, opticalImage.visibleProperty ],
  ( postion, diameter, projectionScreenPosition, opticalImageVisible ) => 
    opticalImageVisible &&
    position.y >= projectionScreenPosition.y - projectionScreen.height / 2 - diameter / 2 &&
    position.y < = projectionScreenPosition.y + projectionScreen.height / 2 + diameter / 2,  {
...
} );
 
* Change `visibleProperty` for each LightSpotNode to be simply be something like this in LightSpotNode.ts:

```js
visibleProperty: lightSpot1.visibleProperty

• Make a change something like this in LightSceneNode, for the visibleProperty of each LightSpotNode instance:

-      visibleProperty: DerivedProperty.and( [ lightPropagationEnabledProperty, scene.opticalImage1.visibleProperty ], {
+      visibleProperty: DerivedProperty.and( [ lightSpot1.visibleProperty, lightPropagationEnabledProperty ], {
...
-      visibleProperty: DerivedProperty.and(
        [ lightPropagationEnabledProperty, scene.opticalImage2.visibleProperty, visibleProperties.secondPointVisibleProperty ], {
+      visibleProperty: DerivedProperty.and(
        [ lightSpot2.visibleProperty, lightPropagationEnabledProperty, visibleProperties.secondPointVisibleProperty ], {

[pixelzoom]

Well, that went smoother than expected... I did not exactly follow the plan outlined in #426 (comment). Instead of LightSpot visibleProperty, I added intersectsProjectionScreenProperty (which is PhET-iO instrumented). If any part of a light spot intersects the projection screen, then the associated LightSpotNode will be visible, and the light spot's positionProperty (its center point) will be a jump point for tools.

@arouinfar ready for review in master.

[arouinfar]

Thanks @pixelzoom. Here are the notable changes:

1. Concave lens: optical image positions are skipped
2. Concave lens: light spot position is skipped
3. Convex lens: light spot skipped if fully off-screen
4. Convex lens: if light spot is fully off-screen, the optical image position is also skipped

It's not clear if (4) is intentional or not. In general, the optical image isn't particularly interesting for the Light scene. I am fine with skipping it altogether. It seems odd to jump there only when the lens is convex and the light spot is visible on the screen.

[pixelzoom]

Re:

... In general, the optical image isn't particularly interesting for the Light scene. I am fine with skipping it altogether. It seems odd to jump there only when the lens is convex and the light spot is visible on the screen.

@arouinfar and I met on Zoom, and decided to delete the optical image positions from the Light scene jump points. Done in the above commit, and we both verified. Labeling as fixed-awaiting-deploy until the next 1.2 RC.

[KatieWoe]

Things are looking good in dev.1