Move useNearest from Drawable to Skin #566
Conversation
adroitwhiz
force-pushed
the
usenearest-skin
branch
from
77df7e3 to
94e681a
Compare
|
Could you resolve conflicts? |
adroitwhiz
force-pushed
the
usenearest-skin
branch
from
94e681a to
0af5331
Compare
|
Rebased to latest develop |
adroitwhiz
force-pushed
the
usenearest-skin
branch
3 times, most recently
from
71b1e0f to
116db7b
Compare
adroitwhiz
added
pr - good to merge
pr - changes requested
and removed
pr - good to merge
labels
|
For some reason this is making vector sprites look blurry |
adroitwhiz
force-pushed
the
usenearest-skin
branch
from
116db7b to
cc2bc3d
Compare
|
Sorry this fell off our radar :/ |
This makes the code messier but I'm not sure what else to do since the texture filtering method to be used depends on the drawable's properties (e.g. transform, enabled effects). We still need to pass in the scale separately because in the main rendering path, we multiply it by the screen-space scale factor.
adroitwhiz
force-pushed
the
usenearest-skin
branch
from
cc2bc3d to
26d2677
Compare
| if ((drawable.enabledEffects & ( | ||
| ShaderManager.EFFECT_INFO.fisheye.mask | | ||
| ShaderManager.EFFECT_INFO.whirl.mask | | ||
| ShaderManager.EFFECT_INFO.pixelate.mask | | ||
| ShaderManager.EFFECT_INFO.mosaic.mask | ||
| )) !== 0) { | ||
| return false; | ||
| } | ||
|
|
||
| // We can't use nearest neighbor unless we are a multiple of 90 rotation | ||
| if (drawable._direction % 90 !== 0) { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
What do you think of keeping the drawable.useNearest function, having these lines stay there, and then having it return skin.useNearest? Then both drawable and skin get to have a say in whether we useNearest
There was a problem hiding this comment.
These lines are very SVGSkin-specific--it kinda sucks that SVGSkin depends on its parent drawable to know whether it should use nearest-neighbor interpolation but I'm not sure what else can be done.
There was a problem hiding this comment.
👍 thanks!
There are some weird things going on here -- such as https://github.com/LLK/scratch-render/pull/566/files#r510419973 -- but after some thinking and discussing I think I agree that the weirdness is sort of inherent to the way it's all set up and isn't a problem introduced by this change. Maybe we can find a way to clean it up later, but I'm happy to move forward.
This PR depends on #555
Resolves
Resolves #565
Proposed Changes
This PR moves the
useNearestmethod from theDrawableclass to theSkinclass. The default behavior is to always use nearest-neighbor interpolation, butSkinsubclasses can override the method to provide their own behavior, and two currently do:SVGSkin.useNearesthas absorbed the complex logic previously present inDrawable.useNearest(checking if certain effects are set, the sprite is rotated, and the scale is close to 100%).PenSkin.useNearestwill use nearest-neighbor interpolation when being scaled up, and linear interpolation when being scaled down. This prevents pen lines from appearing "dashed" when aPenSkinis displayed at <100% scale.Reason for Changes
I believe this is the clearest way to implement the desired interpolation behavior for
PenSkins.Since textures belong to a
Skin, it makes sense that deciding the proper way to interpolate a texture should be the responsibility of thatSkin.