Impurity dots outside of wire

[ghost]

For phetsims/qa#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 ffcefb6.

[KatieWoe]

Close up image

[jessegreenberg]

Good eye, this is definitely new because the clip shape was changed for the wire in #165

[jessegreenberg]

@KatieWoe or @lmulhall-phet can you please see if this is fixed in master?

[ghost]

Looks fixed to me.

[KatieWoe]

Looks good on my end

[jessegreenberg]

OK thanks @KatieWoe and @lmulhall-phet, leaving open to make sure this gets into the release branch.

[KatieWoe]

@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.

[zepumph]

From #170 (comment) 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?

[ghost]

I'm seeing some slight clipping in master on Win10 + Chrome:

[ghost]

Found this in master on macOS + Chrome:

EDIT: This is a screenshot for ants.

[zepumph]

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,

[jessegreenberg]

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.

[jessegreenberg]

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.

[jessegreenberg]

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?

[ghost]

@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.

[KatieWoe]

I would agree. Slight clipping is preferable. The clipping is better since the issue was first raised.

[jessegreenberg]

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.

[jessegreenberg]

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.

[jessegreenberg]

With 9 segments on each side, it is plenty fast enough and almost a perfect shape

[jessegreenberg]

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?

[KatieWoe]

@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.

[jessegreenberg]

OK, great, thanks. In that case this can be merged into the release branch.

[jessegreenberg]

I believe that these are the commits that need to go into the release branch:
1889524
c029514
a3278ac

[zepumph]

We will not be pulling this into the 1.5 rc. If this has been confirmed fixed in master it can be closed.