responding to review

This commit is contained in:
Tom 2023-12-12 21:45:24 -05:00
parent e50d3639ce
commit a364c3f168
5 changed files with 24 additions and 40 deletions

View File

@ -199,7 +199,7 @@ module.exports = function(grunt) {
} }
}, },
watch: { watch: {
files: [ "Gruntfile.js", "src/*.js", "images/*", "src/webgl/*.js" ], files: [ "Gruntfile.js", "src/*.js", "images/*" ],
tasks: "watchTask" tasks: "watchTask"
}, },
eslint: { eslint: {

View File

@ -1,5 +1,5 @@
/* /*
* OpenSeadragon - Drawer * OpenSeadragon - CanvasDrawer
* *
* Copyright (C) 2009 CodePlex Foundation * Copyright (C) 2009 CodePlex Foundation
* Copyright (C) 2010-2022 OpenSeadragon contributors * Copyright (C) 2010-2022 OpenSeadragon contributors
@ -49,31 +49,20 @@ class CanvasDrawer extends $.DrawerBase{
constructor(){ constructor(){
super(...arguments); 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 * @member {Object} context
* @memberof OpenSeadragon.Drawer# * @memberof OpenSeadragon.CanvasDrawer#
*/ */
this.context = this.canvas.getContext( '2d' ); this.context = this.canvas.getContext( '2d' );
/** // Sketch canvas used to temporarily draw tiles which cannot be drawn directly
* Sketch canvas used to temporarily draw tiles which cannot be drawn directly // to the main canvas due to opacity. Lazily initialized.
* to the main canvas due to opacity. Lazily initialized.
*/
this.sketchCanvas = null; this.sketchCanvas = null;
this.sketchContext = 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). // 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; 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 * create the HTML element (e.g. canvas, div) that the image will be drawn into
* @returns {Element} the canvas to draw into * @returns {Element} the canvas to draw into
*/ */
createDrawingElement(){ _createDrawingElement(){
let canvas = $.makeNeutralElement("canvas"); let canvas = $.makeNeutralElement("canvas");
let viewportSize = this._calculateCanvasSize(); let viewportSize = this._calculateCanvasSize();
canvas.width = viewportSize.x; canvas.width = viewportSize.x;
@ -105,14 +94,13 @@ class CanvasDrawer extends $.DrawerBase{
* Draws the TiledImages * Draws the TiledImages
*/ */
draw(tiledImages) { draw(tiledImages) {
var _this = this;
this._prepareNewFrame(); // prepare to draw a new frame this._prepareNewFrame(); // prepare to draw a new frame
tiledImages.forEach(function(tiledImage){
for(const tiledImage of tiledImages){
if (tiledImage.opacity !== 0 || tiledImage._preload) { if (tiledImage.opacity !== 0 || tiledImage._preload) {
// _this._updateViewportWithTiledImage(tiledImage); this._drawTiles(tiledImage);
_this._drawTiles(tiledImage);
} }
}); }
} }
/** /**
@ -178,7 +166,6 @@ class CanvasDrawer extends $.DrawerBase{
/** /**
* @private * @private
* @inner
* Fires the tile-drawing event. * Fires the tile-drawing event.
* *
*/ */
@ -553,8 +540,6 @@ class CanvasDrawer extends $.DrawerBase{
} }
} }
/* Methods from Tile */
/** /**
* @private * @private
* @inner * @inner
@ -566,11 +551,12 @@ class CanvasDrawer extends $.DrawerBase{
_clipWithPolygons (polygons, useSketch) { _clipWithPolygons (polygons, useSketch) {
var context = this._getContext(useSketch); var context = this._getContext(useSketch);
context.beginPath(); context.beginPath();
polygons.forEach(function (polygon) { for(const polygon of polygons){
polygon.forEach(function (coord, i) { for(const [i, coord] of polygon.entries() ){
context[i === 0 ? 'moveTo' : 'lineTo'](coord.x, coord.y); context[i === 0 ? 'moveTo' : 'lineTo'](coord.x, coord.y);
}); }
}); }
context.clip(); context.clip();
} }

View File

@ -90,8 +90,6 @@ $.DrawerBase = class DrawerBase{
*/ */
this.container = $.getElement( options.element ); 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. // 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. // 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. // 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(){ get canvas(){
if(!this._renderingTarget){ if(!this._renderingTarget){
this._renderingTarget = this.createDrawingElement(); this._renderingTarget = this._createDrawingElement();
} }
return this._renderingTarget; 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 * create the HTML element (e.g. canvas, div) that the image will be drawn into
* @returns {Element} the element to draw into * @returns {Element} the element to draw into
*/ */
createDrawingElement() { _createDrawingElement() {
$.console.error('Drawer.createDrawingElement must be implemented by child class'); $.console.error('Drawer._createDrawingElement must be implemented by child class');
return null; return null;
} }
@ -214,8 +212,8 @@ $.DrawerBase = class DrawerBase{
* placeholder methods are still in place. * placeholder methods are still in place.
*/ */
_checkForAPIOverrides(){ _checkForAPIOverrides(){
if(this.createDrawingElement === $.DrawerBase.prototype.createDrawingElement){ if(this._createDrawingElement === $.DrawerBase.prototype._createDrawingElement){
throw("[drawer].createDrawingElement must be implemented by child class"); throw("[drawer]._createDrawingElement must be implemented by child class");
} }
if(this.draw === $.DrawerBase.prototype.draw){ if(this.draw === $.DrawerBase.prototype.draw){
throw("[drawer].draw must be implemented by child class"); throw("[drawer].draw must be implemented by child class");

View File

@ -80,7 +80,7 @@ class HTMLDrawer extends $.DrawerBase{
* create the HTML element (e.g. canvas, div) that the image will be drawn into * create the HTML element (e.g. canvas, div) that the image will be drawn into
* @returns {Element} the div to draw into * @returns {Element} the div to draw into
*/ */
createDrawingElement(){ _createDrawingElement(){
let canvas = $.makeNeutralElement("div"); let canvas = $.makeNeutralElement("div");
return canvas; return canvas;
} }

View File

@ -166,7 +166,7 @@
* create the HTML element (canvas in this case) that the image will be drawn into * create the HTML element (canvas in this case) that the image will be drawn into
* @returns {Element} the canvas to draw into * @returns {Element} the canvas to draw into
*/ */
createDrawingElement(){ _createDrawingElement(){
let canvas = $.makeNeutralElement("canvas"); let canvas = $.makeNeutralElement("canvas");
let viewportSize = this._calculateCanvasSize(); let viewportSize = this._calculateCanvasSize();
canvas.width = viewportSize.x; canvas.width = viewportSize.x;