Closed ghost closed 5 years ago
Close up image
Good eye, this is definitely new because the clip shape was changed for the wire in #165
@KatieWoe or @lmulhall-phet can you please see if this is fixed in master?
Looks fixed to me.
Looks good on my end
OK thanks @KatieWoe and @lmulhall-phet, leaving open to make sure this gets into the release branch.
@zepumph For phetsims/QA/issues/183. It seems like this seems to happen to some small extent still. Win 10 Chrome Edit: I notice that it seems to happen most near the corners of the wire.
From https://github.com/phetsims/resistance-in-a-wire/issues/170#issuecomment-415901065 it makes it seem like this issue is completely fixed in master. @KatieWoe can you confirm that you can't find this "small" version of the bug in master too?
I'm seeing some slight clipping in master on Win10 + Chrome:
Found this in master on macOS + Chrome:
EDIT: This is a screenshot for ants.
I agree, but I think it is mighty close. @jessegreenberg this may just be adjusting the clip area ever so slightly. A few more comments about what I found in master.
Here is a dot a the edge where it is curved more, and it is a bit off.
Where as this dot is pretty much perfect on the end. I'm not sure it needs any tweaking, but it is on the more gentle part of the curve
Here is another where I cannot decide if it is outside the wire or not,
Edit: I notice that it seems to happen most near the corners of the wire.
That seems to be the most important thing. The clip shape is scaled down by the radius of the dot to prevent this, but maybe that isn't enough for the corners.
I am able to reproduce this issue around the rounded corners of the wire if I extend the length of the wire just long enough with the keyboard.
If the clip shape gets too small the dots start to look like they disappear too early, which IMO is worse than them extending beyond the edge of corners of the wire by a few pixels. I adjusted the scale by a bit to get a happy medium, @KatieWoe and @lmulhall-phet can you play with master and see if this is better?
@jessegreenberg, I took a look at it in master:
Notice how the center-left dot disappears too early.
Having a little bit of clipping isn't ideal, but I think it's better than having the dots disappear too early.
I would agree. Slight clipping is preferable. The clipping is better since the issue was first raised.
Ok, one more thing we can try is to approximate the arc around the ends of the wire. Using Shape.ellipticalArc
was performance intensive, but maybe an arc shape with fewer segments will work out faster.
Using segments to approximate the clip shape seems to be working well. I tested in JAWS 2018 on my slower laptop in FF 62 and it sounds nice, I can't reproduce #165 or #164. The clip shape looks something like this, though we can increase the number of segments to get a cleaner shape.
With 9 segments on each side, it is plenty fast enough and almost a perfect shape
The "approximation" will be worst around the corners of the wire. So the current result is that dots that are right at the corners may have a sliver chopped off between the dot and edge of the wire that looks something like this:
But I think this is totally reasonable since we have clean clipping for the rest of the wire, better performance, and no dots that are removed too early.
@KatieWoe @lmulhall-phet can you please review master and comment if you agree or disagree?
@jessegreenberg I agree, it looks much better. I noticed the same thing you did in the corners, but on a very high resolution screen I had to squint to notice it.
OK, great, thanks. In that case this can be merged into the release branch.
I believe that these are the commits that need to go into the release branch: https://github.com/phetsims/resistance-in-a-wire/commit/1889524314f79fec312219217a8cc286171d670a https://github.com/phetsims/resistance-in-a-wire/commit/c029514323a0bd7730882a4e86e6ad10bd6ea8c8 https://github.com/phetsims/resistance-in-a-wire/commit/a3278ac15a64d84780417bc740d27c57e04cbb3a
We will not be pulling this into the 1.5 rc. If this has been confirmed fixed in master it can be closed.
For https://github.com/phetsims/QA/issues/179. On macOS 10.13.6 + Chrome 68.0.3440.106, I'm seeing this:
I can't remember if this was an issue in older versions of the sim before @jessegreenberg implemented ffcefb6ad36f94f606c878e51b0f8ff86f10f7f3.