Merge pull request #566 from adroitwhiz/usenearest-skin

Move useNearest from Drawable to Skin

src/BitmapSkin.js

@@ -33,13 +33,6 @@ class BitmapSkin extends Skin {
         super.dispose();
     }
 
-    /**
-     * @returns {boolean} true for a raster-style skin (like a BitmapSkin), false for vector-style (like SVGSkin).
-     */
-    get isRaster () {
-        return true;
-    }
-
     /**
      * @return {Array<number>} the "native" size, in texels, of this skin.
      */

src/Drawable.js

@@ -504,40 +504,6 @@ class Drawable {
         return this.skin.isTouchingLinear(getLocalPosition(this, vec));
     }
 
-    /**
-     * Should the drawable use NEAREST NEIGHBOR or LINEAR INTERPOLATION mode
-     * @param {?Array<Number>} scale Optionally, the screen-space scale of the drawable.
-     * @return {boolean} True if the drawable should use nearest-neighbor interpolation.
-     */
-    useNearest (scale = this.scale) {
-        // Raster skins (bitmaps) should always prefer nearest neighbor
-        if (this.skin.isRaster) {
-            return true;
-        }
-
-        // If the effect bits for mosaic, pixelate, whirl, or fisheye are set, use linear
-        if ((this.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 (this._direction % 90 !== 0) {
-            return false;
-        }
-
-        // If the scale of the skin is very close to 100 (0.99999 variance is okay I guess)
-        if (Math.abs(scale[0]) > 99 && Math.abs(scale[0]) < 101 &&
-            Math.abs(scale[1]) > 99 && Math.abs(scale[1]) < 101) {
-            return true;
-        }
-        return false;
-    }
-
     /**
      * Get the precise bounds for a Drawable.
      * This function applies the transform matrix to the known convex hull,
@@ -679,7 +645,7 @@ class Drawable {
         if (this.skin) {
             this.skin.updateSilhouette(this._scale);
 
-            if (this.useNearest()) {
+            if (this.skin.useNearest(this._scale, this)) {
                 this.isTouching = this._isTouchingNearest;
             } else {
                 this.isTouching = this._isTouchingLinear;
@@ -753,10 +719,10 @@ class Drawable {
             dst[3] = 0;
             return dst;
         }
-        
+
         const textColor =
         // commenting out to only use nearest for now
-        // drawable.useNearest() ?
+        // drawable.skin.useNearest(drawable._scale, drawable) ?
              drawable.skin._silhouette.colorAtNearest(localPosition, dst);
         // : drawable.skin._silhouette.colorAtLinear(localPosition, dst);
 

src/PenSkin.js

@@ -109,20 +109,19 @@ class PenSkin extends Skin {
         super.dispose();
     }
 
-    /**
-     * @returns {boolean} true for a raster-style skin (like a BitmapSkin), false for vector-style (like SVGSkin).
-     */
-    get isRaster () {
-        return true;
-    }
-
     /**
      * @return {Array<number>} the "native" size, in texels, of this skin. [width, height]
      */
     get size () {
         return this._size;
     }
 
+    useNearest (scale) {
+        // Use nearest-neighbor interpolation when scaling up the pen skin-- this matches Scratch 2.0.
+        // When scaling it down, use linear interpolation to avoid giving pen lines a "dashed" appearance.
+        return Math.max(scale[0], scale[1]) >= 100;
+    }
+
     /**
      * @param {Array<number>} scale The X and Y scaling factors to be used, as percentages of this skin's "native" size.
      * @return {WebGLTexture} The GL texture representation of this skin when drawing at the given size.

src/RenderWebGL.js

@@ -1901,7 +1901,9 @@ class RenderWebGL extends EventEmitter {
 
             if (uniforms.u_skin) {
                 twgl.setTextureParameters(
-                    gl, uniforms.u_skin, {minMag: drawable.useNearest(drawableScale) ? gl.NEAREST : gl.LINEAR}
+                    gl, uniforms.u_skin, {
+                        minMag: drawable.skin.useNearest(drawableScale, drawable) ? gl.NEAREST : gl.LINEAR
+                    }
                 );
             }
 

src/SVGSkin.js

@@ -2,6 +2,7 @@ const twgl = require('twgl.js');
 
 const Skin = require('./Skin');
 const SvgRenderer = require('scratch-svg-renderer').SVGRenderer;
+const ShaderManager = require('./ShaderManager');
 
 const MAX_TEXTURE_DIMENSION = 2048;
 
@@ -58,6 +59,35 @@ class SVGSkin extends Skin {
         return this._svgRenderer.size;
     }
 
+    useNearest (scale, drawable) {
+        // If the effect bits for mosaic, pixelate, whirl, or fisheye are set, use linear
+        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;
+        }
+
+        // Because SVG skins' bounding boxes are currently not pixel-aligned, the idea here is to hide blurriness
+        // by using nearest-neighbor scaling if one screen-space pixel is "close enough" to one texture pixel.
+        // If the scale of the skin is very close to 100 (0.99999 variance is okay I guess)
+        // TODO: Make this check more precise. We should use nearest if there's less than one pixel's difference
+        // between the screen-space and texture-space sizes of the skin. Mipmaps make this harder because there are
+        // multiple textures (and hence multiple texture spaces) and we need to know which one to choose.
+        if (Math.abs(scale[0]) > 99 && Math.abs(scale[0]) < 101 &&
+            Math.abs(scale[1]) > 99 && Math.abs(scale[1]) < 101) {
+            return true;
+        }
+        return false;
+    }
+
     /**
      * Create a MIP for a given scale.
      * @param {number} scale - The relative size of the MIP

src/Skin.js

@@ -59,13 +59,6 @@ class Skin extends EventEmitter {
         this._id = RenderConstants.ID_NONE;
     }
 
-    /**
-     * @returns {boolean} true for a raster-style skin (like a BitmapSkin), false for vector-style (like SVGSkin).
-     */
-    get isRaster () {
-        return false;
-    }
-
     /**
      * @return {int} the unique ID for this Skin.
      */
@@ -88,6 +81,19 @@ class Skin extends EventEmitter {
         return [0, 0];
     }
 
+    /**
+     * Should this skin's texture be filtered with nearest-neighbor or linear interpolation at the given scale?
+     * @param {?Array<Number>} scale The screen-space X and Y scaling factors at which this skin's texture will be
+     * displayed, as percentages (100 means 1 "native size" unit is 1 screen pixel; 200 means 2 screen pixels, etc).
+     * @param {Drawable} drawable The drawable that this skin's texture will be applied to.
+     * @return {boolean} True if this skin's texture, as returned by {@link getTexture}, should be filtered with
+     * nearest-neighbor interpolation.
+     */
+    // eslint-disable-next-line no-unused-vars
+    useNearest (scale, drawable) {
+        return true;
+    }
+
     /**
      * Get the center of the current bounding box
      * @return {Array<number>} the center of the current bounding box