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

Arrow scaling #27

Closed
jessegreenberg opened this issue Sep 6, 2017 · 17 comments
Closed

Arrow scaling #27

jessegreenberg opened this issue Sep 6, 2017 · 17 comments
Assignees

Comments

@jessegreenberg
Copy link
Contributor

From the design meeting, arrow scaling was too small at small forces. These notes are from the design doc.

Default should show force arrows obvious to the user (arrow about the size of GFL default).

Extremes can still be extreme -- okay if largest force goes off-screen and if smallest force isn’t visible.

@arouinfar by "default" did you want to change initial values for the distance/charges or should we just manipulate the arrow scaling?

@arouinfar
Copy link
Contributor

@jessegreenberg let's keep the default values the same, and manipulate the arrow scaling.

@arouinfar arouinfar assigned jessegreenberg and unassigned arouinfar Sep 6, 2017
@jessegreenberg
Copy link
Contributor Author

Sounds good @arouinfar. @mbarlow12 does this seem straight forward with our current approach to arrow scaling?

mbarlow12 added a commit to phetsims/inverse-square-law-common that referenced this issue Sep 8, 2017
@mbarlow12 mbarlow12 assigned arouinfar and unassigned mbarlow12 Sep 8, 2017
@mbarlow12
Copy link
Contributor

We added a second visual mapping for small force values. This adds some arrow modulation when the force is relatively small. You can still see a point where the mapping shifts to expected behavior.

@arouinfar could you have a look and confirm this is what was desired? Thanks.

@arouinfar
Copy link
Contributor

Definitely an improvement @mbarlow12. The Macro screen is feeling pretty good, but Atomic Scale could use some work. On Macro, the vector noticeably grows when increasing one of the charges or bringing them closer together. Ideally, the same would happen on the Atomic Scale screen, but there doesn't seem to be as much range.

The default vector size seems reasonable.
image

However, using the default charges, and minimizing the distance doesn't show much growth.
screen shot 2017-09-08 at 2 48 41 pm

Likewise, using the default distance, and maximizing the charges looks about the same as the default, even though the force is 2 orders of magnitude larger.
image

However, when the force increases yet another 2 orders of magnitude, the vector is visibly larger.
screen shot 2017-09-08 at 2 50 03 pm

@jessegreenberg
Copy link
Contributor Author

@arouinfar can you please review this version? (on branch, can't see on phettest)

http://www.colorado.edu/physics/phet/dev/html/coulombs-law/1.0.0-dev.6/coulombs-law_en.html

Being able to see 2 orders of magnitude in size difference around the initial values lead to massive arrows that can't be seen when exploring full parameter ranges. This is why we investigated a second function around small force values. What do you think about the behavior in this version?

@jessegreenberg
Copy link
Contributor Author

@arouinfar I don't mean to rush, but once #33 is done we could be ready for a dev test. Any thoughts for this issue?

@arouinfar
Copy link
Contributor

@jessegreenberg @mbarlow12 in general, the scaling in dev.7 is looking much nicer! The Macro screen looks great, and the size of the vectors seems very responsive. I'm still not completely satisfied with the behavior on the Atomic Scale screen, however.

The size of the force vector on startup seems appropriate -- quite small, but still visible.
image

Increasing the charges from +1/-1 to +10/-10, the vectors don't appear to grow very much, at all:
image

Likewise, moving the default charges closer together also does not yield much growth:
image

Ideally, the behavior would be:

  • At default values, the vector is visible
  • At default separation, increasing the magnitude of one charge results in noticeably larger vectors
  • Moving default charges closer together results in noticeably larger vectors

Currently, the Atomic Scale screen fails the latter two items.

@arouinfar arouinfar removed their assignment Dec 8, 2017
@jessegreenberg
Copy link
Contributor Author

@arouinfar, what do you think of behavior in https://www.colorado.edu/physics/phet/dev/html/coulombs-law/1.0.0-dev.6/coulombs-law_en.html?

In that dev version, we remove all "hollywood" effects that keeps arrows visible, so all three of the list items are covered. However, at larger forces the arrows go way off screen. Personally I think this is OK and prefer the behavior in dev.6.

@arouinfar
Copy link
Contributor

@jessegreenberg I was comparing the vector scaling in dev.5 and dev.7 -- I hadn't realized dev.6 and dev.7 were different. My bad!

I'm really liking the scaling in dev.6 -- the vectors feel much more responsive! I also think it's fine for the extremes to be extreme, as it's a better representation of inverse square behavior.

@arouinfar arouinfar removed their assignment Dec 9, 2017
@jessegreenberg
Copy link
Contributor Author

Sorry @arouinfar, I should have mentioned it when I provided a dev version to test.

@mbarlow12 could you please make master behave like 1.0.0-dev.6? I think in this version we removed the smallForceToArrowWidthFunction function in ISLCForceArrowNode.

@jessegreenberg
Copy link
Contributor Author

@arouinfar and I met to discuss on 12/14, and discussed that the scaling feels good in dev.6, but it would be nice if the scaling of the macro arrows at their default value was a little bigger. Ideally this should not change the size of the arrows on the atomic view.

@jessegreenberg
Copy link
Contributor Author

After discussing a little more, we decided to have a larger meeting to talk about how this should behave across CL, GFL, and GFL:B (all sims that use inverse-square-law-common).

@jessegreenberg
Copy link
Contributor Author

After a slack discussion with @arouinfar I removed the scaling around small arrows in master so that we can have a broader discussion about this in context with GFL and GFL:B sims soon. The old scaling should be preserved on branch augmented-arrows (in both this repo and ISLC) in case.

@jessegreenberg
Copy link
Contributor Author

No longer on hold, @ariel-phet and @arouinfar gave a +1 for the scaling in this sim. Next steps are to clean up TODOs added in e05fe2c and make sure that the pixel polish scaling of arrows around default values.

@jessegreenberg
Copy link
Contributor Author

I think that the arrow scaling values for coulombs-law were discussed with @arouinfar and @mbarlow12. @mbarlow12 is there anything more that needs to be done for this issue?

@arouinfar
Copy link
Contributor

@jessegreenberg looks like this should have been reassigned to me. @mbarlow12 published a new dev version after our meeting for me to review, and it slipped off the radar. I'll review this afternoon and report back.

@arouinfar arouinfar assigned arouinfar and unassigned mbarlow12 Jan 17, 2018
@arouinfar
Copy link
Contributor

@jessegreenberg @mbarlow12 looks great!

On Atomic Scale, we ended up increasing the default values to match Macro, which means the charge magnitudes no longer mirror the Hydrogen atom. I think this is an acceptable trade-off which allows for the desired behavior described in #27 (comment) while reducing the range of values for off-scale vectors.

I think the figures could be tweaked a bit, as there are ranges where the force increases quite a bit, but the figure is already maxed out. I'll open a separate issue for this, however.

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

No branches or pull requests

3 participants