Revert "Draw pen lines via fragment shader"

[fsih]

Reverts #438
Bryce found some issues with pen that we're investigating

[adroitwhiz]

Keep me updated! What test project is that?

[fsih]

I think this is go to random position? I don't think it was doing that when I tested it

[adroitwhiz]

This looks more like fencing behavior to me. I'm not sure what problem @BryceLTaylor is referring to in the above screenshot, but I can reproduce something similar-looking on this project:

Of course, the sprite's rotation center is waaaay off, so it's to be expected that the top and left sides of the stage aren't getting touched:

It doesn't seem like the pen skin is getting incorrectly moved or the lines aren't being drawn in the right place-- all the lines are connected, and they never go off the right or bottom edges of the stage.

The only other issue I see in that screenshot is that some of the pen lines look "dashed":

but that's a problem with nearest-neighbor interpolation that existed long before my PR was merged.

[fsih]

That makes sense about the fencing behavior.

Do you happen to know what might cause the "problem with nearest-neighbor interpolation"? I agree it seems unlikely it's your PR.

[adroitwhiz]

Sorry--by that I meant that the problem is nearest-neighbor interpolation. The PenSkin is considered a "raster skin", and so is scaled using nearest-neighbor interpolation, much like BitmapSkins. In "smallish stage mode", nearest-neighbor interpolation causes certain rows and columns of pixels to disappear entirely, creating the appearance of dashed lines.

I can think of three ways to fix this:

• The first way is to set PenSkin.isRaster to false. This will, however, make the pen layer appear blurry at larger sizes (currently it appears pixelated, which matches 2.0).
• The second way is to modify useNearest to use nearest-neighbor interpolation only when scaling up raster skins, and to use linear interpolation when scaling them down. I think this actually more closely matches 2.0's behavior, but would change the way bitmap skins are currently displayed.
• The third way would be to add a special case to useNearest for when the drawable's skin is a PenSkin, or give each Skin its own useNearest implementation that Drawable.useNearest defers to, and implement the "nearest neighbor when scaling up" behavior only for PenSkins.

[BryceLTaylor]

This is the project on production that was having the fencing issue: https://scratch.mit.edu/projects/371983393/
I made it on staging first where it exhibited the fencing bug, then I downloaded it from there and uploaded it to production. The project was not changed between the two versions and it shows no issues on production.

It also demonstrated the skinny lines appearing to be dashed problem, which even if it's caused by existing issues, we should not implement anything that reveals it.

[adroitwhiz]

Are you sure the fencing bug is related to my PR? I ran your test project under the "pen shader" branch and found no such issue:

(Firefox 73.0.1 on Linux; what browser did you test on?)

Regarding pen lines appearing "dashed", IMO they appear only slightly worse in my PR--here's what they look like currently:

I'd prefer to avoid blocking this on a nearest-neighbor fix, as I'm concerned it would take another few months to get that merged in (which is completely understandable given your current workload, but something I'd rather not hold up this PR on). Once a Scratch Team member can provide input on which of these three options should be implemented in order to fix the scaling artifacts, I'd be happy to create another PR.

[adroitwhiz]

Alright @BryceLTaylor, I think I may have found the source of the fencing issue.

If you draw an off-center costume, the rotation center and bounds are updated as expected and fencing occurs.

If, however, you then delete the contents of the costume, fencing incorrectly continues to be applied using the previous costume contents until you reload:

Thus, you ran into this bug if you performed the following set of steps:

1. Create the pen project
2. Delete the contents of "costume1", leaving costume1's previous rotation center intact
3. Observe the fencing bug
4. Reload, fixing the bug
   4a. But, if you created the project in staging, and reloaded on production, it will falsely appear that staging caused the bug!