From 809fc8b688e5b34087ba2a84c78fed9c385eb1a7 Mon Sep 17 00:00:00 2001 From: Patrick Brosset Date: Sun, 9 Sep 2012 01:35:29 +0200 Subject: [PATCH 1/4] Fix the duplication in the overlay when grid toggl When the grid was toggled, the overlay would actually draw the main drawing frame instead of just redrawing itself with transparent pixels. This was due to the drawingcontroller passing the ref to the same frame object when initialiwing both renders. By the way, the overlay should probably be treated a bit differently as redrawing all its transprent pixels in that case is useless. --- js/controller/DrawingController.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/js/controller/DrawingController.js b/js/controller/DrawingController.js index 97997a9c..8fa99c2c 100644 --- a/js/controller/DrawingController.js +++ b/js/controller/DrawingController.js @@ -35,7 +35,7 @@ "canvas-overlay"); this.renderer.init(this.frame); - this.overlayRenderer.init(this.frame); + this.overlayRenderer.init(this.overlayFrame); // State of drawing controller: this.isClicked = false; From d18c3cd5f778e6ad6240da705eb5a0b30c8c0b2f Mon Sep 17 00:00:00 2001 From: juliandescottes Date: Sun, 9 Sep 2012 02:34:54 +0200 Subject: [PATCH 2/4] Cleaned FrameRenderer.js FrameRenderer should also keep a reference on the frame it is updating - initially I wanted the renderer to be frame independant, but it doesnt bring much --- js/controller/DrawingController.js | 7 +-- js/drawingtools/Stroke.js | 3 -- js/rendering/FrameRenderer.js | 68 ++++++++++++------------------ 3 files changed, 30 insertions(+), 48 deletions(-) diff --git a/js/controller/DrawingController.js b/js/controller/DrawingController.js index 63dccdc6..145d83d8 100644 --- a/js/controller/DrawingController.js +++ b/js/controller/DrawingController.js @@ -206,13 +206,11 @@ this.renderer.updateDPI(newDPI); this.overlayRenderer.updateDPI(newDPI); - this.renderer.render(this.frame); - this.overlayRenderer.render(this.overlayFrame); + this.render(); }; ns.DrawingController.prototype.render = function () { try { - this.renderFrame(); this.renderOverlay(); } catch (e) { @@ -236,12 +234,11 @@ var serializedOverlay = this.overlayFrame.serialize(); if (this.serializedOverlay != serializedOverlay) { this.serializedOverlay = serializedOverlay - this.renderer.render(this.overlayFrame); + this.overlayRenderer.render(this.overlayFrame); } }; ns.DrawingController.prototype.clearOverlay = function () { this.overlayFrame = pskl.model.Frame.createEmptyFromFrame(this.frame); - this.overlayRenderer.clear(); }; })(); \ No newline at end of file diff --git a/js/drawingtools/Stroke.js b/js/drawingtools/Stroke.js index 13dabac3..7a3137c7 100644 --- a/js/drawingtools/Stroke.js +++ b/js/drawingtools/Stroke.js @@ -35,7 +35,6 @@ // The fake canvas where we will draw the preview of the stroke: // Drawing the first point of the stroke in the fake overlay canvas: drawer.overlayFrame.setPixel(col, row, color); - drawer.renderOverlay(); }; ns.Stroke.prototype.moveToolAt = function(col, row, color, drawer) { @@ -59,7 +58,6 @@ } drawer.overlayFrame.setPixel(strokePoints[i].col, strokePoints[i].row, color); } - drawer.renderOverlay(); }; /** @@ -78,7 +76,6 @@ } // Draw in canvas: // TODO: Remove that when we have the centralized redraw loop - drawer.renderFrame(); } // For now, we are done with the stroke tool and don't need an overlay anymore: drawer.clearOverlay(); diff --git a/js/rendering/FrameRenderer.js b/js/rendering/FrameRenderer.js index a2f97e44..5761f85f 100644 --- a/js/rendering/FrameRenderer.js +++ b/js/rendering/FrameRenderer.js @@ -58,33 +58,42 @@ }; ns.FrameRenderer.prototype.render = function (frame) { + this.clear(frame); + var context = this.getCanvas_(frame).getContext('2d'); for(var col = 0, width = frame.getWidth(); col < width; col++) { for(var row = 0, height = frame.getHeight(); row < height; row++) { - this.drawPixel(col, row, frame, this.getCanvas_(frame, col, row), this.dpi); + var color = frame.getPixel(col, row); + this.renderPixel_(color, col, row, context); } } this.lastRenderedFrame = frame; }; ns.FrameRenderer.prototype.drawPixel = function (col, row, frame) { - var context = this.getCanvas_(frame, col, row).getContext('2d'); + var context = this.getCanvas_(frame).getContext('2d'); var color = frame.getPixel(col, row); if(color == Constants.TRANSPARENT_COLOR) { - context.clearRect(this.getFrameY_(col), this.getFrameY_(row), this.dpi, this.dpi); - } - else { - if(color != Constants.SELECTION_TRANSPARENT_COLOR) { - // TODO(vincz): Found a better design to update the palette, it's called too frequently. - $.publish(Events.COLOR_USED, [color]); - } - context.fillStyle = color; - context.fillRect(this.getFrameY_(col), this.getFrameY_(row), this.dpi, this.dpi); + context.clearRect(this.getFramePos_(col), this.getFramePos_(row), this.dpi, this.dpi); + } else { + this.renderPixel_(color, col, row, context); } this.lastRenderedFrame = frame; }; - ns.FrameRenderer.prototype.clear = function (col, row, frame) { - var canvas = this.getCanvas_(frame, col, row) + ns.FrameRenderer.prototype.renderPixel_ = function (color, col, row, context) { + if(color != Constants.TRANSPARENT_COLOR) { + context.fillStyle = color; + context.fillRect(this.getFramePos_(col), this.getFramePos_(row), this.dpi, this.dpi); + } + + if(color != Constants.SELECTION_TRANSPARENT_COLOR) { + // TODO(vincz): Found a better design to update the palette, it's called too frequently. + $.publish(Events.COLOR_USED, [color]); + } + }; + + ns.FrameRenderer.prototype.clear = function (frame) { + var canvas = this.getCanvas_(frame); canvas.getContext("2d").clearRect(0, 0, canvas.width, canvas.height); }; @@ -104,15 +113,8 @@ /** * @private */ - ns.FrameRenderer.prototype.getFrameX_ = function(col) { - return col * this.dpi + ((col - 1) * this.gridStrokeWidth); - }; - - /** - * @private - */ - ns.FrameRenderer.prototype.getFrameY_ = function(row) { - return row * this.dpi + ((row - 1) * this.gridStrokeWidth); + ns.FrameRenderer.prototype.getFramePos_ = function(index) { + return index * this.dpi + ((index - 1) * this.gridStrokeWidth); }; /** @@ -123,14 +125,14 @@ ctx.lineWidth = Constants.GRID_STROKE_WIDTH; ctx.strokeStyle = Constants.GRID_STROKE_COLOR; for(var c=1; c < col; c++) { - ctx.moveTo(this.getFrameX_(c), 0); - ctx.lineTo(this.getFrameX_(c), height); + ctx.moveTo(this.getFramePos_(c), 0); + ctx.lineTo(this.getFramePos_(c), height); ctx.stroke(); } for(var r=1; r < row; r++) { - ctx.moveTo(0, this.getFrameY_(r)); - ctx.lineTo(width, this.getFrameY_(r)); + ctx.moveTo(0, this.getFramePos_(r)); + ctx.lineTo(width, this.getFramePos_(r)); ctx.stroke(); } }; @@ -168,18 +170,4 @@ } return this.canvas; }; - - /** - * @private - */ - ns.FrameRenderer.prototype.createCanvasForFrame_ = function (frame) { - var canvas = document.createElement("canvas"); - canvas.setAttribute("width", frame.getWidth() * this.dpi); - canvas.setAttribute("height", frame.getHeight() * this.dpi); - - canvas.classList.add("canvas"); - if(this.className) canvas.classList.add(this.className); - - return canvas; - }; })(); \ No newline at end of file From 8982a5b4790bf9e349700b5b447d45d01161f64f Mon Sep 17 00:00:00 2001 From: juliandescottes Date: Sun, 9 Sep 2012 11:37:52 +0200 Subject: [PATCH 3/4] Removed 2 useless instance variables in AnimatedPreviewController --- js/controller/AnimatedPreviewController.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/js/controller/AnimatedPreviewController.js b/js/controller/AnimatedPreviewController.js index ee1e7f1c..34b84eef 100644 --- a/js/controller/AnimatedPreviewController.js +++ b/js/controller/AnimatedPreviewController.js @@ -8,8 +8,7 @@ this.currentIndex = 0; this.fps = parseInt($("#preview-fps")[0].value, 10); - this.deltaTime = 0; - this.previousTime = 0; + var renderingOptions = { "dpi": dpi }; From 7b90873324f953ef866baf2402ad9215336d099d Mon Sep 17 00:00:00 2001 From: juliandescottes Date: Mon, 10 Sep 2012 19:53:34 +0200 Subject: [PATCH 4/4] Fixed initialization bug + performance issue with jquery pub sub --- js/piskel.js | 2 +- js/rendering/FrameRenderer.js | 5 ----- 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/js/piskel.js b/js/piskel.js index 2aee3ebb..01316a6a 100644 --- a/js/piskel.js +++ b/js/piskel.js @@ -83,7 +83,7 @@ $.namespace("pskl"); }); $.subscribe('FRAMESHEET_RESET', function(evt, frameId) { - piskel.redraw(); + piskel.render(); }); var drawingLoop = new pskl.rendering.DrawingLoop(); drawingLoop.addCallback(this.render, this); diff --git a/js/rendering/FrameRenderer.js b/js/rendering/FrameRenderer.js index 5761f85f..20d42ad4 100644 --- a/js/rendering/FrameRenderer.js +++ b/js/rendering/FrameRenderer.js @@ -84,11 +84,6 @@ if(color != Constants.TRANSPARENT_COLOR) { context.fillStyle = color; context.fillRect(this.getFramePos_(col), this.getFramePos_(row), this.dpi, this.dpi); - } - - if(color != Constants.SELECTION_TRANSPARENT_COLOR) { - // TODO(vincz): Found a better design to update the palette, it's called too frequently. - $.publish(Events.COLOR_USED, [color]); } };