From 5831447f75502c5b9c75e47429b91b81548b63c5 Mon Sep 17 00:00:00 2001 From: juliandescottes Date: Sat, 27 Dec 2014 15:02:41 +0100 Subject: [PATCH] Fix #242, onion skin rendered not cleared if 0 frames --- src/js/controller/DrawingController.js | 2 +- src/js/rendering/OnionSkinRenderer.js | 79 +++++++----- src/js/rendering/frame/CachedFrameRenderer.js | 6 +- src/js/rendering/frame/FrameRenderer.js | 10 +- test/js/rendering/CanvasRendererTest.js | 15 +-- test/js/rendering/OnionSkinRendererTest.js | 120 ++++++++++++++++++ test/js/testutils/TestUtils.js | 22 ++++ test/js/utils/FrameTransformTest.js | 26 +--- 8 files changed, 201 insertions(+), 79 deletions(-) create mode 100644 test/js/rendering/OnionSkinRendererTest.js create mode 100644 test/js/testutils/TestUtils.js diff --git a/src/js/controller/DrawingController.js b/src/js/controller/DrawingController.js index f6ed0653..cf185110 100644 --- a/src/js/controller/DrawingController.js +++ b/src/js/controller/DrawingController.js @@ -33,7 +33,7 @@ this.overlayRenderer = new pskl.rendering.frame.CachedFrameRenderer(this.container, renderingOptions, ["canvas-overlay"]); this.renderer = new pskl.rendering.frame.CachedFrameRenderer(this.container, renderingOptions, ["drawing-canvas"]); - this.onionSkinRenderer = new pskl.rendering.OnionSkinRenderer(this.container, renderingOptions, piskelController); + this.onionSkinRenderer = pskl.rendering.OnionSkinRenderer.createInContainer(this.container, renderingOptions, piskelController); this.layersRenderer = new pskl.rendering.layer.LayersRenderer(this.container, renderingOptions, piskelController); this.compositeRenderer = new pskl.rendering.CompositeRenderer(); diff --git a/src/js/rendering/OnionSkinRenderer.js b/src/js/rendering/OnionSkinRenderer.js index a4a27b93..970f83a8 100644 --- a/src/js/rendering/OnionSkinRenderer.js +++ b/src/js/rendering/OnionSkinRenderer.js @@ -1,32 +1,63 @@ (function () { var ns = $.namespace('pskl.rendering'); - ns.OnionSkinRenderer = function (container, renderingOptions, piskelController) { + ns.OnionSkinRenderer = function (renderer, piskelController) { pskl.rendering.CompositeRenderer.call(this); this.piskelController = piskelController; - - // Do not use CachedFrameRenderers here, since the caching will be performed in the render method of LayersRenderer - this.renderer = new pskl.rendering.frame.FrameRenderer(container, renderingOptions, ["onion-skin-canvas"]); - + this.renderer = renderer; this.add(this.renderer); - this.serializedRendering = ''; + this.hash = ''; + }; + + ns.OnionSkinRenderer.createInContainer = function (container, renderingOptions, piskelController) { + // Do not use CachedFrameRenderers here, caching is performed in the render method + var renderer = new pskl.rendering.frame.FrameRenderer(container, renderingOptions, ['onion-skin-canvas']); + return new ns.OnionSkinRenderer(renderer, piskelController); }; pskl.utils.inherit(pskl.rendering.OnionSkinRenderer, pskl.rendering.CompositeRenderer); ns.OnionSkinRenderer.prototype.render = function () { + var frames = this.getOnionFrames_(); + var hash = this.computeHash_(frames); + if (this.hash != hash) { + this.hash = hash; + this.clear(); + if (frames.length > 0) { + var mergedFrame = pskl.utils.FrameUtils.merge(frames); + this.renderer.render(mergedFrame); + } + } + }; + + ns.OnionSkinRenderer.prototype.getOnionFrames_ = function () { + var frames = []; + + var currentFrameIndex = this.piskelController.getCurrentFrameIndex(); + var layer = this.piskelController.getCurrentLayer(); + + var previousIndex = currentFrameIndex - 1; + var previousFrame = layer.getFrameAt(previousIndex); + if (previousFrame) { + frames.push(previousFrame); + } + + var nextIndex = currentFrameIndex + 1; + var nextFrame = layer.getFrameAt(nextIndex); + if (nextFrame) { + frames.push(nextFrame); + } + + return frames; + }; + + ns.OnionSkinRenderer.prototype.computeHash_ = function (frames) { var offset = this.getOffset(); var size = this.getDisplaySize(); var layers = this.piskelController.getLayers(); - var currentFrameIndex = this.piskelController.getCurrentFrameIndex(); - - var frames = []; - this.addFrameAtIndexToArray_(currentFrameIndex - 1, frames); - this.addFrameAtIndexToArray_(currentFrameIndex + 1, frames); - - var serializedRendering = [ + return [ this.getZoom(), this.getGridWidth(), offset.x, @@ -37,25 +68,7 @@ return f.getHash(); }).join('-'), layers.length - ].join("-"); - - - if (this.serializedRendering != serializedRendering) { - this.serializedRendering = serializedRendering; - - if (frames.length > 0) { - this.clear(); - var mergedFrame = pskl.utils.FrameUtils.merge(frames); - this.renderer.render(mergedFrame); - } - } - }; - - ns.OnionSkinRenderer.prototype.addFrameAtIndexToArray_ = function (frameIndex, frames) { - var layer = this.piskelController.getCurrentLayer(); - if (this.piskelController.hasFrameAt(frameIndex)) { - frames.push(layer.getFrameAt(frameIndex)); - } + ].join('-'); }; /** @@ -72,6 +85,6 @@ }; ns.OnionSkinRenderer.prototype.flush = function () { - this.serializedRendering = ''; + this.hash = ''; }; })(); \ No newline at end of file diff --git a/src/js/rendering/frame/CachedFrameRenderer.js b/src/js/rendering/frame/CachedFrameRenderer.js index 0c77ac07..ebd1fa07 100644 --- a/src/js/rendering/frame/CachedFrameRenderer.js +++ b/src/js/rendering/frame/CachedFrameRenderer.js @@ -5,10 +5,10 @@ * FrameRenderer implementation that prevents unnecessary redraws. * @param {HtmlElement} container HtmlElement to use as parentNode of the Frame * @param {Object} renderingOptions - * @param {Array} classes array of strings to use for css classes + * @param {Array} classList array of strings to use for css classes */ - ns.CachedFrameRenderer = function (container, renderingOptions, classes) { - pskl.rendering.frame.FrameRenderer.call(this, container, renderingOptions, classes); + ns.CachedFrameRenderer = function (container, renderingOptions, classList) { + pskl.rendering.frame.FrameRenderer.call(this, container, renderingOptions, classList); this.serializedFrame = ''; }; diff --git a/src/js/rendering/frame/FrameRenderer.js b/src/js/rendering/frame/FrameRenderer.js index d8291a5f..7738d3cf 100644 --- a/src/js/rendering/frame/FrameRenderer.js +++ b/src/js/rendering/frame/FrameRenderer.js @@ -5,9 +5,9 @@ * FrameRenderer will display a given frame inside a canvas element. * @param {HtmlElement} container HtmlElement to use as parentNode of the Frame * @param {Object} renderingOptions - * @param {Array} classes array of strings to use for css classes + * @param {Array} classList array of strings to use for css classList */ - ns.FrameRenderer = function (container, renderingOptions, classes) { + ns.FrameRenderer = function (container, renderingOptions, classList) { this.defaultRenderingOptions = { 'supportGridRendering' : false, 'zoom' : 1 @@ -39,8 +39,8 @@ this.supportGridRendering = renderingOptions.supportGridRendering; - this.classes = classes || []; - this.classes.push('canvas'); + this.classList = classList || []; + this.classList.push('canvas'); /** * Off dom canvas, will be used to draw the frame at 1:1 ratio @@ -153,7 +153,7 @@ var height = this.displayHeight; var width = this.displayWidth; - this.displayCanvas = pskl.utils.CanvasUtils.createCanvas(width, height, this.classes); + this.displayCanvas = pskl.utils.CanvasUtils.createCanvas(width, height, this.classList); pskl.utils.CanvasUtils.disableImageSmoothing(this.displayCanvas); this.container.append(this.displayCanvas); }; diff --git a/test/js/rendering/CanvasRendererTest.js b/test/js/rendering/CanvasRendererTest.js index 043be14b..a4a04e62 100644 --- a/test/js/rendering/CanvasRendererTest.js +++ b/test/js/rendering/CanvasRendererTest.js @@ -3,25 +3,12 @@ describe("Canvas Renderer test", function() { var WHITE = '#ffffff'; var TRANS = Constants.TRANSPARENT_COLOR; - var toFrameGrid = function (normalGrid) { - var frameGrid = []; - var w = normalGrid[0].length; - var h = normalGrid.length; - for (var x = 0 ; x < w ; x++) { - frameGrid[x] = []; - for (var y = 0 ; y < h ; y++) { - frameGrid[x][y] = normalGrid[y][x]; - } - } - return frameGrid; - }; - beforeEach(function() {}); afterEach(function() {}); it("draws transparent as white by default", function() { // create frame - var frame = pskl.model.Frame.fromPixelGrid(toFrameGrid([ + var frame = pskl.model.Frame.fromPixelGrid(test.testutils.toFrameGrid([ [BLACK, TRANS], [TRANS, BLACK] ])); diff --git a/test/js/rendering/OnionSkinRendererTest.js b/test/js/rendering/OnionSkinRendererTest.js new file mode 100644 index 00000000..9342fbb1 --- /dev/null +++ b/test/js/rendering/OnionSkinRendererTest.js @@ -0,0 +1,120 @@ +describe("Onion Skin Renderer test", function() { + var BLACK = '#000000'; + var WHITE = '#ffffff'; + var TRANS = Constants.TRANSPARENT_COLOR; + + beforeEach(function() {}); + afterEach(function() {}); + + it("renders correctly :)", function() { + var fakeRenderer = createMockRenderer(); + var layer = createMockLayer(); + var piskelController = createMockPiskelController(); + piskelController.currentLayer_ = layer; + + var onionSkinRenderer = new pskl.rendering.OnionSkinRenderer(fakeRenderer, piskelController); + // create frame + var previousFrame = pskl.model.Frame.fromPixelGrid(test.testutils.toFrameGrid([ + [BLACK, TRANS], + [TRANS, TRANS] + ])); + + var nextFrame = pskl.model.Frame.fromPixelGrid(test.testutils.toFrameGrid([ + [TRANS, TRANS], + [TRANS, BLACK] + ])); + + piskelController.currentFrameIndex_ = 1; + layer.frames = [previousFrame, {}, nextFrame]; + + // initial state, all counters at 0 + expect(fakeRenderer.clearCounter_).toBe(0); + expect(fakeRenderer.renderCounter_).toBe(0); + + // First rendering, should call clear + render + onionSkinRenderer.render(); + expect(fakeRenderer.clearCounter_).toBe(1); + expect(fakeRenderer.renderCounter_).toBe(1); + test.testutils.frameEqualsGrid(fakeRenderer.renderedFrame_, [ + [BLACK, TRANS], + [TRANS, BLACK] + ]); + + // Second rendering, nothing changed, should not clear or render + onionSkinRenderer.render(); + expect(fakeRenderer.clearCounter_).toBe(1); + expect(fakeRenderer.renderCounter_).toBe(1); + + // remove one frame + layer.frames = [previousFrame, {}]; + onionSkinRenderer.render(); + expect(fakeRenderer.clearCounter_).toBe(2); + expect(fakeRenderer.renderCounter_).toBe(2); + test.testutils.frameEqualsGrid(fakeRenderer.renderedFrame_, [ + [BLACK, TRANS], + [TRANS, TRANS] + ]); + + // remove the other frame + layer.frames = [{}]; + piskelController.currentFrameIndex_ = 0; + onionSkinRenderer.render(); + // nothing to render, but the underlying renderer should still be cleared + expect(fakeRenderer.clearCounter_).toBe(3); + expect(fakeRenderer.renderCounter_).toBe(2); + }); + + var createMockLayer = function () { + return { + frames : [], + getFrameAt : function (index) { + return this.frames[index]; + } + }; + }; + + var createMockPiskelController = function () { + return { + currentFrameIndex_ : 1, + currentLayer_ : null, + getLayers : function () { + return [this.currentLayer_]; + }, + + getCurrentFrameIndex : function () { + return this.currentFrameIndex_; + }, + + getCurrentLayer : function () { + return this.currentLayer_; + } + }; + }; + + var createMockRenderer = function () { + return { + clearCounter_ : 0, + clear : function () { + this.clearCounter_++; + }, + renderedFrame_ : null, + renderCounter_ : 0, + render : function (frame) { + this.renderCounter_++; + this.renderedFrame_ = frame; + }, + getZoom : function () { + return 1; + }, + getGridWidth : function () { + return 0; + }, + getOffset : function () { + return {x:0,y:0}; + }, + getDisplaySize : function () { + return {width:10,height:10}; + } + }; + }; +}); \ No newline at end of file diff --git a/test/js/testutils/TestUtils.js b/test/js/testutils/TestUtils.js new file mode 100644 index 00000000..bf5de19f --- /dev/null +++ b/test/js/testutils/TestUtils.js @@ -0,0 +1,22 @@ +(function () { + var ns = $.namespace('test.testutils'); + + ns.toFrameGrid = function (normalGrid) { + var frameGrid = []; + var w = normalGrid[0].length; + var h = normalGrid.length; + for (var x = 0 ; x < w ; x++) { + frameGrid[x] = []; + for (var y = 0 ; y < h ; y++) { + frameGrid[x][y] = normalGrid[y][x]; + } + } + return frameGrid; + }; + + ns.frameEqualsGrid = function (frame, grid) { + frame.forEachPixel(function (color, col, row) { + expect(color).toBe(grid[row][col]); + }); + }; +})(); \ No newline at end of file diff --git a/test/js/utils/FrameTransformTest.js b/test/js/utils/FrameTransformTest.js index 80528b88..0a8455a6 100644 --- a/test/js/utils/FrameTransformTest.js +++ b/test/js/utils/FrameTransformTest.js @@ -9,29 +9,9 @@ describe("FrameTransform suite", function() { var CLOCKWISE = pskl.utils.FrameTransform.CLOCKWISE; var COUNTERCLOCKWISE = pskl.utils.FrameTransform.COUNTERCLOCKWISE; - /** - * Check a frame has the pixels as a given grid - * @param {Frame} frame [description] - * @param {Array>} grid - */ - var frameEqualsGrid = function (frame, grid) { - frame.forEachPixel(function (color, col, row) { - expect(color).toBe(grid[row][col]); - }); - }; - - var toFrameGrid = function (normalGrid) { - var frameGrid = []; - var w = normalGrid[0].length; - var h = normalGrid.length; - for (var x = 0 ; x < w ; x++) { - frameGrid[x] = []; - for (var y = 0 ; y < h ; y++) { - frameGrid[x][y] = normalGrid[y][x]; - } - } - return frameGrid; - }; + // shortcuts + var frameEqualsGrid = test.testutils.frameEqualsGrid; + var toFrameGrid = test.testutils.toFrameGrid; /*******************************/ /************ FLIP *************/