search

phetsims / fourier-making-waves

"Fourier: Making Waves" is an educational simulation in HTML5, by PhET Interactive Simulations.
GNU General Public License v3.0
6 stars 3 forks source link

questions about Meaurement Tools #39

Closed pixelzoom closed 3 years ago

pixelzoom commented 3 years ago

Question about the Meaurement Tools in the Discrete screen ...

@arouinfar please advise.

screenshot_810
pixelzoom commented 3 years ago

Moving forward with this, doing what I think is correct...

  • Where should the tools be positoned initially? Java initial positions are shown in screenshot below. For domain=time, the Period tool is in the same initial position as the Wavelength tool.

Here are the initial tool positions:

Wavelength tool for Function of: space (x) and Function of: space (x) & time (t):

screenshot_819

Period tool for Function of: time (t):

screenshot_818

Period tool for Function of: space (x) & time (t):

screenshot_817
  • Should initial position be reset when Reset All is pressed? Java version does.

They are resettable.

  • How should dragging be constrained? Java version allows tools to be dragged anywhere in the "play area", as long as some (tiny!) part of the tool remained visible. This seems reasonable for HTML5 version, but I think more of the tool should be required to be inside window bounds.

Drag bounds for all tools are dynamically derived from the bounds of the browser window. If the browser window is resized such that a tool would be outside the drag bounds, any in-progress interaction is interrupted, and the tool is move inside the drag bounds. That said...

The caliper-like tools are constrained differently in the x and y dimensions. In the x dimension, at least 50 pixels of a tool must be inside the browser window's bounds. In the y dimension, the entire tool must be inside the browser window's bounds.

The clock-like (period) tool must be fully inside the browser window's bounds in both the x and y dimensions.

Over to @arouinfar to review in master.

pixelzoom commented 3 years ago

@arouinfar I made some changes to the tools (factored out duplicate code) and the initial position of the caliper-like tools is now off a little bit in master. I'm trying to track this down, but wanted to let you know. If you review this issue, please do so with the assumption that initial positions will be as shown in the above screenshots.

pixelzoom commented 3 years ago

@arouinfar Initial positions are fixed. So you can review master or 1.0.0-dev.12.

arouinfar commented 3 years ago

@pixelzoom the initial positions of the Wavelength tool and Period clock look great. I think something's up with the initial position of the caliper-like Period tool, though. When turning it on for the first time, it occludes the Amplitude readouts. image

Once after using the ResetAllButton, it respawned near the Harmonics plot, though offset to the left. image

The drag bounds all look reasonable in dev.12. I would be fine with limiting everything to the browser window's bounds like the clock-like period tool, but I don't have a strong opinion. Do whatever you think is best @pixelzoom.

pixelzoom commented 3 years ago

@arouinfar I can't reproduce the case where the caliper-like Period tool occludes the Amplitude readouts. Can you please provide steps to reproduce?

I did reproduce the other case, where the caliper-like Period tool is offset to the left:

  1. start sim
  2. go to Discrete screen
  3. select "Function of: time"
  4. Check the "Period" checkbox
pixelzoom commented 3 years ago

@arouinfar said:

... I would be fine with limiting everything to the browser window's bounds like the clock-like period tool, but I don't have a strong opinion. Do whatever you think is best @pixelzoom.

All tools are now constrained to be fully inside the browser window's bounds. I think this will make more sense to the user, and make it less likely that a tool will get "lost". Programmatically it's also more straightforward and will be easier to maintain. So win win.

arouinfar commented 3 years ago

@arouinfar I can't reproduce the case where the caliper-like Period tool occludes the Amplitude readouts. Can you please provide steps to reproduce?

Sure, it's the same steps you outlined for the offset:

start sim go to Discrete screen select "Function of: time" Check the "Period" checkbox

The caliper-like Period tool always shows up on top of the Amplitude controls for me when I follow this procedure. Here's my troubleshooting info:

Name: ‪Fourier: Making Waves‬ URL: https://phet-dev.colorado.edu/html/fourier-making-waves/1.0.0-dev.12/phet/fourier-making-waves_en_phet.html Version: 1.0.0-dev.12 2021-01-13 23:17:06 UTC Features missing: applicationcache, applicationcache, touch User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/87.0.4280.88 Safari/537.36 Language: en-US Window: 2560x1329 Pixel Ratio: 1/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: 80) Max viewport: 16384x16384 OES_texture_float: true Dependencies JSON: {}

pixelzoom commented 3 years ago

Wow, really weird. With macOS 11.1 + Chrome 87, I was not able to reproduce what @arouinfar described with the Period tool (covering the amplitude displays). After updating to Chrome 88, I can reproduce.

Also really weird is the fact that the Wavelength and Period tools are identical implementations (subclasses of CalipersNode), with identical initialization of initial position. But the Period tool is in the wrong initial position.

pixelzoom commented 3 years ago

More weirdness... On Safari, I consistently see what @arouinfar described -- the initial position of the Period tool covers the amplitude displays:

image

With Chrome 88, I saw that happen exactly once. And every reload after that, the Period tool is nowhere near the amplitude displays, it's here:

image

pixelzoom commented 3 years ago

If I change the default domain to "time", then the Wavelength tool is in the wrong initial position. So I'm guessing that a tool is not being initialized properly when it doesn't correspond to the domain -- probably something to do with the model-to-view transform as handled by chartTransform.

pixelzoom commented 3 years ago

In the above commit I had to change the drag bounds for the caliper-like tools. When you zoom in on the graphs, these tools (specifically for n=1) will become wider than the bounds of the browser window. So it's impossible to constrain these tools to be fully visible. The new behavior is that the left tip of the tool has to be within the width (x dimension) of the browser window, minus a margin of 40 pixels.

So for example, this is the furthest left that a caliper-like tool can be dragged:

screenshot_827

And this is the furthest right:

screenshot_828

@arouinfar seem OK to you?

pixelzoom commented 3 years ago

It might even be better to limit the x postions of these tools to the left and right edges of the charts. If you wanted to get them out of the way, you should uncheck the checkboxes, not move the tools drastically to the right. (I still think moving them the fully height of the browser window is OK.)

@arouinfar thoughts?

pixelzoom commented 3 years ago

Hmmm... What I proposed in the previous comment is going to involve some major changes to my scenegraph structure. So perhaps we should leave it the way it is.

pixelzoom commented 3 years ago

Initial position of Period tool is fixed in the above commit.

@arouinfar to summarize what's I need from you:

arouinfar commented 3 years ago

Verify that initial position of all tools is now correct.

Looks good!

Verify that the drag constraint change described in #39 (comment) is OK.

In the above commit I had to change the drag bounds for the caliper-like tools. When you zoom in on the graphs, these tools (specifically for n=1) will become wider than the bounds of the browser window. So it's impossible to constrain these tools to be fully visible. The new behavior is that the left tip of the tool has to be within the width (x dimension) of the browser window, minus a margin of 40 pixels.

Oh, good catch with the zoom level. I think the behavior in master feels really nice.