From 1208324d4d4ccee8d6a9a9a27712672f824dc37f Mon Sep 17 00:00:00 2001 From: juliandescottes Date: Sun, 9 Aug 2015 12:37:03 +0200 Subject: [PATCH] Copy paste bug : add unit tests for FrameUtils with null value --- src/js/app.js | 4 +- .../controller/preview/PreviewController.js | 3 - src/js/rendering/CanvasRenderer.js | 8 -- src/js/selection/BaseSelection.js | 8 +- src/js/selection/SelectionManager.js | 21 ++- src/js/utils/FrameUtils.js | 30 +++-- src/js/utils/PixelUtils.js | 2 +- test/js/testutils/TestUtils.js | 22 +++ test/js/utils/FrameUtilsTest.js | 125 +++++++++++------- 9 files changed, 131 insertions(+), 92 deletions(-) diff --git a/src/js/app.js b/src/js/app.js index 23ea52a8..b8dbd171 100644 --- a/src/js/app.js +++ b/src/js/app.js @@ -192,9 +192,7 @@ getFirstFrameAsPng : function () { var firstFrame = this.piskelController.getFrameAt(0); - var canvasRenderer = new pskl.rendering.CanvasRenderer(firstFrame, 1); - canvasRenderer.drawTransparentAs('rgba(0,0,0,0)'); - var firstFrameCanvas = canvasRenderer.render(); + var firstFrameCanvas = pskl.utils.FrameUtils.toImage(firstFrame); return firstFrameCanvas.toDataURL('image/png'); }, diff --git a/src/js/controller/preview/PreviewController.js b/src/js/controller/preview/PreviewController.js index 1c12ccdc..5526839c 100644 --- a/src/js/controller/preview/PreviewController.js +++ b/src/js/controller/preview/PreviewController.js @@ -25,9 +25,6 @@ }; ns.PreviewController.prototype.init = function () { - // the oninput event won't work on IE10 unfortunately, but at least will provide a - // consistent behavior across all other browsers that support the input type range - // see https://bugzilla.mozilla.org/show_bug.cgi?id=853670 this.fpsRangeInput.on('input change', this.onFPSSliderChange.bind(this)); document.querySelector('.right-column').style.width = Constants.ANIMATED_PREVIEW_WIDTH + 'px'; diff --git a/src/js/rendering/CanvasRenderer.js b/src/js/rendering/CanvasRenderer.js index 8e38ecdd..9dfe9e07 100644 --- a/src/js/rendering/CanvasRenderer.js +++ b/src/js/rendering/CanvasRenderer.js @@ -31,14 +31,6 @@ return scaledCanvas; }; - ns.CanvasRenderer.prototype.renderPixel_ = function (color, x, y, context) { - if (color == Constants.TRANSPARENT_COLOR) { - color = this.transparentColor_; - } - context.fillStyle = color; - context.fillRect(x, y, 1, 1); - }; - ns.CanvasRenderer.prototype.createCanvas_ = function (zoom) { zoom = zoom || 1; var width = this.frame.getWidth() * zoom; diff --git a/src/js/selection/BaseSelection.js b/src/js/selection/BaseSelection.js index 1551a922..0907b5dd 100644 --- a/src/js/selection/BaseSelection.js +++ b/src/js/selection/BaseSelection.js @@ -24,13 +24,9 @@ }; ns.BaseSelection.prototype.fillSelectionFromFrame = function (targetFrame) { - // on copy trim the selection if out of bounds - // this.pixels = this.pixels.filter(function (pixel) { - // return targetFrame.containsPixel(pixel.col, pixel.row); - // }); - this.pixels.forEach(function (pixel) { - pixel.color = targetFrame.getPixel(pixel.col, pixel.row); + var color = targetFrame.getPixel(pixel.col, pixel.row); + pixel.color = color || Constants.TRANSPARENT_COLOR; }); this.hasPastedContent = true; diff --git a/src/js/selection/SelectionManager.js b/src/js/selection/SelectionManager.js index c9ade102..ea64fc58 100644 --- a/src/js/selection/SelectionManager.js +++ b/src/js/selection/SelectionManager.js @@ -88,20 +88,18 @@ }; ns.SelectionManager.prototype.paste = function() { - if (this.currentSelection && this.currentSelection.hasPastedContent) { - var pixels = this.currentSelection.pixels; - var opaquePixels = pixels.filter(function (p) { - return p.color !== Constants.TRANSPARENT_COLOR; - }); - this.pastePixels(opaquePixels); + if (!this.currentSelection || !this.currentSelection.hasPastedContent) { + return; } - }; - ns.SelectionManager.prototype.pastePixels = function(pixels) { - var currentFrame = this.piskelController.getCurrentFrame(); + var pixels = this.currentSelection.pixels; + var frame = this.piskelController.getCurrentFrame(); pixels.forEach(function (pixel) { - currentFrame.setPixel(pixel.col, pixel.row, pixel.color); + if (pixel.color === Constants.TRANSPARENT_COLOR || pixel.color === null) { + return; + } + frame.setPixel(pixel.col, pixel.row, pixel.color); }); $.publish(Events.PISKEL_SAVE_STATE, { @@ -115,8 +113,7 @@ }; ns.SelectionManager.prototype.replay = function (frame, replayData) { - var pixels = replayData.pixels; - pixels.forEach(function (pixel) { + replayData.pixels.forEach(function (pixel) { var color = replayData.type === SELECTION_REPLAY.PASTE ? pixel.color : Constants.TRANSPARENT_COLOR; frame.setPixel(pixel.col, pixel.row, color); }); diff --git a/src/js/utils/FrameUtils.js b/src/js/utils/FrameUtils.js index 54ebb096..01393d23 100644 --- a/src/js/utils/FrameUtils.js +++ b/src/js/utils/FrameUtils.js @@ -5,8 +5,8 @@ /** * Render a Frame object as an image. * Can optionally scale it (zoom) - * @param {Frame} frame - * @param {Number} zoom + * @param frame {Frame} frame + * @param zoom {Number} zoom * @return {Image} */ toImage : function (frame, zoom) { @@ -19,9 +19,9 @@ /** * Draw the provided frame in a 2d canvas * - * @param {pskl.model.Frame} frame the frame to draw - * @param {Canvas} canvas the canvas target - * @param {String} transparentColor (optional) color to use to represent transparent pixels. + * @param frame {pskl.model.Frame} frame the frame to draw + * @param canvas {Canvas} canvas the canvas target + * @param transparentColor {String} transparentColor (optional) color to use to represent transparent pixels. */ drawToCanvas : function (frame, canvas, transparentColor) { var context = canvas.getContext('2d'); @@ -30,6 +30,9 @@ for (var x = 0, width = frame.getWidth() ; x < width ; x++) { for (var y = 0, height = frame.getHeight() ; y < height ; y++) { var color = frame.getPixel(x, y); + + // accumulate all the pixels of the same color to speed up rendering + // by reducting fillRect calls var w = 1; while (color === frame.getPixel(x, y + w) && (y + w) < height) { w++; @@ -43,14 +46,23 @@ y = y + w - 1; } } - }, + /** + * Render a line of a single color in a given canvas 2D context. + * + * @param color {String} color to draw + * @param x {Number} x coordinate + * @param y {Number} y coordinate + * @param width {Number} width of the line to draw, in pixels + * @param context {CanvasRenderingContext2D} context of the canvas target + */ renderLine_ : function (color, x, y, width, context) { - if (color != Constants.TRANSPARENT_COLOR) { - context.fillStyle = color; - context.fillRect(x, y, 1, width); + if (color === Constants.TRANSPARENT_COLOR || color === null) { + return; } + context.fillStyle = color; + context.fillRect(x, y, 1, width); }, merge : function (frames) { diff --git a/src/js/utils/PixelUtils.js b/src/js/utils/PixelUtils.js index 81b47e71..9400e709 100644 --- a/src/js/utils/PixelUtils.js +++ b/src/js/utils/PixelUtils.js @@ -128,7 +128,7 @@ var nextCol = currentItem.col + dx[i]; var nextRow = currentItem.row + dy[i]; try { - if (frame.containsPixel(nextCol, nextRow) && frame.getPixel(nextCol, nextRow) == targetColor) { + if (frame.containsPixel(nextCol, nextRow) && frame.getPixel(nextCol, nextRow) == targetColor) { queue.push({'col': nextCol, 'row': nextRow}); } } catch (e) { diff --git a/test/js/testutils/TestUtils.js b/test/js/testutils/TestUtils.js index bf5de19f..5153bec4 100644 --- a/test/js/testutils/TestUtils.js +++ b/test/js/testutils/TestUtils.js @@ -1,6 +1,28 @@ (function () { var ns = $.namespace('test.testutils'); + /** + * Frame.createFromGrid accepts grids that are rotated by 90deg from + * the visual/usual way. (column-based grid) + * + * For testing, it's easier for be able to specify a row-based grid, because + * it visually matches what the image will look like. + * + * For instance : + * + * [[black, black, black], + * [white, white, white]] + * + * we expect this to be a 3x2 image, one black line above a white line. + * + * However Frame.createFromGrid needs the following input to create such an image : + * + * [[black, white], + * [black, white], + * [black, white]] + * + * This helper will build the second array from the first array. + */ ns.toFrameGrid = function (normalGrid) { var frameGrid = []; var w = normalGrid[0].length; diff --git a/test/js/utils/FrameUtilsTest.js b/test/js/utils/FrameUtilsTest.js index ea1b2d41..40313012 100644 --- a/test/js/utils/FrameUtilsTest.js +++ b/test/js/utils/FrameUtilsTest.js @@ -3,35 +3,41 @@ describe("FrameUtils suite", function() { var red = '#ff0000'; var transparent = Constants.TRANSPARENT_COLOR; + // shortcuts + var toFrameGrid = test.testutils.toFrameGrid; + var frameEqualsGrid = test.testutils.frameEqualsGrid; + it("merges 2 frames", function () { + var B = black, R = red, T = transparent; var frame1 = pskl.model.Frame.fromPixelGrid([ - [black, transparent], - [transparent, black] + [B, T], + [T, B] ]); var frame2 = pskl.model.Frame.fromPixelGrid([ - [transparent, red], - [red, transparent] + [T, R], + [R, T] ]); var mergedFrame = pskl.utils.FrameUtils.merge([frame1, frame2]); - expect(mergedFrame.getPixel(0,0)).toBe(black); - expect(mergedFrame.getPixel(0,1)).toBe(red); - expect(mergedFrame.getPixel(1,0)).toBe(red); - expect(mergedFrame.getPixel(1,1)).toBe(black); + frameEqualsGrid(mergedFrame, [ + [B, R], + [R, B] + ]); }); it("returns same frame when merging single frame", function () { - var frame1 = pskl.model.Frame.fromPixelGrid([ - [black, transparent], - [transparent, black] - ]); + var B = black, T = transparent; + var frame1 = pskl.model.Frame.fromPixelGrid(toFrameGrid([ + [B, T], + [B, T] + ])); var mergedFrame = pskl.utils.FrameUtils.merge([frame1]); - expect(mergedFrame.getPixel(0,0)).toBe(black); - expect(mergedFrame.getPixel(0,1)).toBe(transparent); - expect(mergedFrame.getPixel(1,0)).toBe(transparent); - expect(mergedFrame.getPixel(1,1)).toBe(black); + frameEqualsGrid(mergedFrame, [ + [B, T], + [B, T] + ]); }); var checkPixelsColor = function (frame, pixels, color) { @@ -42,9 +48,10 @@ describe("FrameUtils suite", function() { }; it ("converts an image to a frame", function () { + var B = black, T = transparent; var frame1 = pskl.model.Frame.fromPixelGrid([ - [black, transparent], - [transparent, black] + [B, T], + [T, B] ]); var image = pskl.utils.FrameUtils.toImage(frame1); @@ -57,48 +64,66 @@ describe("FrameUtils suite", function() { var biggerFrame = pskl.utils.FrameUtils.createFromImage(biggerImage); - checkPixelsColor(biggerFrame, [ - [0,0],[0,1],[0,2], - [1,0],[1,1],[1,2], - [2,0],[2,1],[2,2], - [3,3],[3,4],[3,5], - [4,3],[4,4],[4,5], - [5,3],[5,4],[5,5] - ], black); - - checkPixelsColor(biggerFrame, [ - [0,3],[0,4],[0,5], - [1,3],[1,4],[1,5], - [2,3],[2,4],[2,5], - [3,0],[3,1],[3,2], - [4,0],[4,1],[4,2], - [5,0],[5,1],[5,2] - ], transparent); + frameEqualsGrid(biggerFrame, [ + [B, B, B, T, T, T], + [B, B, B, T, T, T], + [B, B, B, T, T, T], + [T, T, T, B, B, B], + [T, T, T, B, B, B], + [T, T, T, B, B, B] + ]); }); it ("[LayerUtils] creates a layer from a simple spritesheet", function () { - var frame = pskl.model.Frame.fromPixelGrid([ - [black, red], - [red, black], - [black, black], - [red, red] - ]); + var B = black, R = red; + + // original image in 4x2 + var frame = pskl.model.Frame.fromPixelGrid(toFrameGrid([ + [B, R, B, R], + [R, B, B, R] + ])); + var spritesheet = pskl.utils.FrameUtils.toImage(frame); + // split the spritesheet by 4 var frames = pskl.utils.LayerUtils.createLayerFromSpritesheet(spritesheet, 4); + + // expect 4 frames of 1x2 expect(frames.length).toBe(4); - expect(frames[0].getPixel(0,0)).toBe(black); - expect(frames[0].getPixel(0,1)).toBe(red); + // verify frame content + frameEqualsGrid(frames[0], [ + [B], + [R] + ]); + frameEqualsGrid(frames[1], [ + [R], + [B] + ]); + frameEqualsGrid(frames[2], [ + [B], + [B] + ]); + frameEqualsGrid(frames[3], [ + [R], + [R] + ]); + }); - expect(frames[1].getPixel(0,0)).toBe(red); - expect(frames[1].getPixel(0,1)).toBe(black); + it ("supports null values in frame array", function () { + var B = black, T = transparent; + var frame = pskl.model.Frame.fromPixelGrid([ + [B, null], + [null, B] + ]); - expect(frames[2].getPixel(0,0)).toBe(black); - expect(frames[2].getPixel(0,1)).toBe(black); - - expect(frames[3].getPixel(0,0)).toBe(red); - expect(frames[3].getPixel(0,1)).toBe(red); + var image = pskl.utils.FrameUtils.toImage(frame); + // transform back to frame for ease of testing + var testFrame = pskl.utils.FrameUtils.createFromImage(image); + frameEqualsGrid(testFrame, [ + [B, T], + [T, B] + ]); }); }); \ No newline at end of file