From a364c3f1680aacb40d72ffc04b0b561d28005c20 Mon Sep 17 00:00:00 2001 From: Tom Date: Tue, 12 Dec 2023 21:45:24 -0500 Subject: [PATCH] responding to review --- Gruntfile.js | 2 +- src/canvasdrawer.js | 46 ++++++++++++++++----------------------------- src/drawerbase.js | 12 +++++------- src/htmldrawer.js | 2 +- src/webgldrawer.js | 2 +- 5 files changed, 24 insertions(+), 40 deletions(-) diff --git a/Gruntfile.js b/Gruntfile.js index ea94b86c..2aa4b423 100644 --- a/Gruntfile.js +++ b/Gruntfile.js @@ -199,7 +199,7 @@ module.exports = function(grunt) { } }, watch: { - files: [ "Gruntfile.js", "src/*.js", "images/*", "src/webgl/*.js" ], + files: [ "Gruntfile.js", "src/*.js", "images/*" ], tasks: "watchTask" }, eslint: { diff --git a/src/canvasdrawer.js b/src/canvasdrawer.js index 9a4ac2dc..645a5d8b 100644 --- a/src/canvasdrawer.js +++ b/src/canvasdrawer.js @@ -1,5 +1,5 @@ /* - * OpenSeadragon - Drawer + * OpenSeadragon - CanvasDrawer * * Copyright (C) 2009 CodePlex Foundation * Copyright (C) 2010-2022 OpenSeadragon contributors @@ -49,31 +49,20 @@ class CanvasDrawer extends $.DrawerBase{ constructor(){ super(...arguments); - // Options - - - /** - * 2d drawing context for {@link OpenSeadragon.Drawer#canvas} if it's a <canvas> element, otherwise null. + * 2d drawing context for {@link OpenSeadragon.CanvasDrawer#canvas}. * @member {Object} context - * @memberof OpenSeadragon.Drawer# + * @memberof OpenSeadragon.CanvasDrawer# */ this.context = this.canvas.getContext( '2d' ); - /** - * Sketch canvas used to temporarily draw tiles which cannot be drawn directly - * to the main canvas due to opacity. Lazily initialized. - */ + // Sketch canvas used to temporarily draw tiles which cannot be drawn directly + // to the main canvas due to opacity. Lazily initialized. this.sketchCanvas = null; this.sketchContext = null; - // We force our container to ltr because our drawing math doesn't work in rtl. - // This issue only affects our canvas renderer, but we do it always for consistency. - // Note that this means overlays you want to be rtl need to be explicitly set to rtl. - this.container.dir = 'ltr'; - // Image smoothing for canvas rendering (only if canvas is used). - // Canvas default is "true", so this will only be changed if user specified "false". + // Canvas default is "true", so this will only be changed if user specifies "false" in setImageSmoothinEnabled. this._imageSmoothingEnabled = true; } @@ -93,7 +82,7 @@ class CanvasDrawer extends $.DrawerBase{ * create the HTML element (e.g. canvas, div) that the image will be drawn into * @returns {Element} the canvas to draw into */ - createDrawingElement(){ + _createDrawingElement(){ let canvas = $.makeNeutralElement("canvas"); let viewportSize = this._calculateCanvasSize(); canvas.width = viewportSize.x; @@ -105,14 +94,13 @@ class CanvasDrawer extends $.DrawerBase{ * Draws the TiledImages */ draw(tiledImages) { - var _this = this; this._prepareNewFrame(); // prepare to draw a new frame - tiledImages.forEach(function(tiledImage){ + + for(const tiledImage of tiledImages){ if (tiledImage.opacity !== 0 || tiledImage._preload) { - // _this._updateViewportWithTiledImage(tiledImage); - _this._drawTiles(tiledImage); + this._drawTiles(tiledImage); } - }); + } } /** @@ -178,7 +166,6 @@ class CanvasDrawer extends $.DrawerBase{ /** * @private - * @inner * Fires the tile-drawing event. * */ @@ -553,8 +540,6 @@ class CanvasDrawer extends $.DrawerBase{ } } - /* Methods from Tile */ - /** * @private * @inner @@ -566,11 +551,12 @@ class CanvasDrawer extends $.DrawerBase{ _clipWithPolygons (polygons, useSketch) { var context = this._getContext(useSketch); context.beginPath(); - polygons.forEach(function (polygon) { - polygon.forEach(function (coord, i) { + for(const polygon of polygons){ + for(const [i, coord] of polygon.entries() ){ context[i === 0 ? 'moveTo' : 'lineTo'](coord.x, coord.y); - }); - }); + } + } + context.clip(); } diff --git a/src/drawerbase.js b/src/drawerbase.js index a55cdcab..0b7c3b9f 100644 --- a/src/drawerbase.js +++ b/src/drawerbase.js @@ -90,8 +90,6 @@ $.DrawerBase = class DrawerBase{ */ this.container = $.getElement( options.element ); - // TODO: Does this need to be in DrawerBase, or only in Drawer implementations? - // Original commment: // We force our container to ltr because our drawing math doesn't work in rtl. // This issue only affects our canvas renderer, but we do it always for consistency. // Note that this means overlays you want to be rtl need to be explicitly set to rtl. @@ -117,7 +115,7 @@ $.DrawerBase = class DrawerBase{ get canvas(){ if(!this._renderingTarget){ - this._renderingTarget = this.createDrawingElement(); + this._renderingTarget = this._createDrawingElement(); } return this._renderingTarget; } @@ -144,8 +142,8 @@ $.DrawerBase = class DrawerBase{ * create the HTML element (e.g. canvas, div) that the image will be drawn into * @returns {Element} the element to draw into */ - createDrawingElement() { - $.console.error('Drawer.createDrawingElement must be implemented by child class'); + _createDrawingElement() { + $.console.error('Drawer._createDrawingElement must be implemented by child class'); return null; } @@ -214,8 +212,8 @@ $.DrawerBase = class DrawerBase{ * placeholder methods are still in place. */ _checkForAPIOverrides(){ - if(this.createDrawingElement === $.DrawerBase.prototype.createDrawingElement){ - throw("[drawer].createDrawingElement must be implemented by child class"); + if(this._createDrawingElement === $.DrawerBase.prototype._createDrawingElement){ + throw("[drawer]._createDrawingElement must be implemented by child class"); } if(this.draw === $.DrawerBase.prototype.draw){ throw("[drawer].draw must be implemented by child class"); diff --git a/src/htmldrawer.js b/src/htmldrawer.js index cf57c695..5529a849 100644 --- a/src/htmldrawer.js +++ b/src/htmldrawer.js @@ -80,7 +80,7 @@ class HTMLDrawer extends $.DrawerBase{ * create the HTML element (e.g. canvas, div) that the image will be drawn into * @returns {Element} the div to draw into */ - createDrawingElement(){ + _createDrawingElement(){ let canvas = $.makeNeutralElement("div"); return canvas; } diff --git a/src/webgldrawer.js b/src/webgldrawer.js index 0bb2f3a0..344011cf 100644 --- a/src/webgldrawer.js +++ b/src/webgldrawer.js @@ -166,7 +166,7 @@ * create the HTML element (canvas in this case) that the image will be drawn into * @returns {Element} the canvas to draw into */ - createDrawingElement(){ + _createDrawingElement(){ let canvas = $.makeNeutralElement("canvas"); let viewportSize = this._calculateCanvasSize(); canvas.width = viewportSize.x;