the canvas renderer may mess up when rendering external graphics, r=erilem (closes #3264)

git-svn-id: http://svn.openlayers.org/trunk/openlayers@11899 dc9f47b5-9b13-0410-9fdd-eb0c1a62fdaf

lib/OpenLayers/Renderer/Canvas.js

@@ -202,7 +202,7 @@ OpenLayers.Renderer.Canvas = OpenLayers.Class(OpenLayers.Renderer, {
      * style    - {Object}
      * featureId - {String}
      */ 
-    drawExternalGraphic: function(pt, style, featureId) {
+    drawExternalGraphic: function(geometry, style, featureId) {
         var img = new Image();
 
         if (style.graphicTitle) {
@@ -218,20 +218,25 @@ OpenLayers.Renderer.Canvas = OpenLayers.Class(OpenLayers.Renderer, {
         var yOffset = (style.graphicYOffset != undefined) ?
            style.graphicYOffset : -(0.5 * height);
 
-        var x = pt[0] + xOffset;
-        var y = pt[1] + yOffset;
-
         var opacity = style.graphicOpacity || style.fillOpacity;
         
         var onLoad = function() {
-            // TODO: check that we haven't moved
+            if(!this.features[featureId]) {
+                return;
+            }
+            var pt = this.getLocalXY(geometry);
+            var p0 = pt[0];
+            var p1 = pt[1];
+            if(!isNaN(p0) && !isNaN(p1)) {
+                var x = p0 + xOffset;
+                var y = p1 + yOffset;
                 var canvas = this.canvas;
                 canvas.globalAlpha = opacity;
                 var factor = OpenLayers.Renderer.Canvas.drawImageScaleFactor ||
                     (OpenLayers.Renderer.Canvas.drawImageScaleFactor =
                         /android 2.1/.test(navigator.userAgent.toLowerCase()) ?
-                        // 320 is the screen width of the G1 phone, for which
+                            // 320 is the screen width of the G1 phone, for
-                        // drawImage works out of the box.
+                            // which drawImage works out of the box.
                             320 / window.screen.width : 1
                     );
                 canvas.drawImage(
@@ -241,6 +246,7 @@ OpenLayers.Renderer.Canvas = OpenLayers.Class(OpenLayers.Renderer, {
                     this.setHitContextStyle("fill", featureId);
                     this.hitContext.fillRect(x, y, width, height);
                 }
+            }
         };
 
         img.onload = OpenLayers.Function.bind(onLoad, this);
@@ -327,13 +333,13 @@ OpenLayers.Renderer.Canvas = OpenLayers.Class(OpenLayers.Renderer, {
      */ 
     drawPoint: function(geometry, style, featureId) {
         if(style.graphic !== false) {
+            if(style.externalGraphic) {
+                this.drawExternalGraphic(geometry, style, featureId);
+            } else {
                 var pt = this.getLocalXY(geometry);
                 var p0 = pt[0];
                 var p1 = pt[1];
-            if (!isNaN(p0) && !isNaN(p1)) {
+                if(!isNaN(p0) && !isNaN(p1)) {
-                if (style.externalGraphic) {
-                    this.drawExternalGraphic(pt, style, featureId);
-                } else {
                     var twoPi = Math.PI*2;
                     var radius = style.pointRadius;
                     if(style.fill !== false) {

tests/Renderer/Canvas.html

@@ -330,6 +330,128 @@
         
     }
 
+    // see http://trac.osgeo.org/openlayers/ticket/3264
+    function test_externalGraphic_destroyFeatures(t) {
+        if (!supported) {
+            t.plan(0);
+            return;
+        }
+
+        t.plan(1);
+
+        // set up
+
+        var layer = new OpenLayers.Layer.Vector(null, {
+            isBaseLayer: true,
+            renderers: ["Canvas"]
+        });
+
+        var map = new OpenLayers.Map({
+            div: "map",
+            controls: [],
+            layers: [layer],
+            center: new OpenLayers.LonLat(0, 0),
+            zoom: 0
+        });
+
+        layer.addFeatures([
+            new OpenLayers.Feature.Vector(
+                new OpenLayers.Geometry.Point(0, 0),
+                null,
+                {
+                    externalGraphic: '../../img/marker.png',
+                    graphicHeight: 20,
+                    graphicWidth: 20
+                }
+            )
+        ]);
+
+        var called = false;
+        layer.renderer.canvas.drawImage = function(img, x, y, w, h) {
+            called = true;
+        };
+
+        // test
+
+        // schedule a canvas.drawImage
+        layer.renderer.redraw();
+
+        // destroy the feature before drawImage gets called
+        layer.destroyFeatures();
+
+        t.delay_call(0.1, function() {
+            t.ok(!called,
+                 'canvas.drawImage not called if feature is destroyed');
+
+            // tear down
+            map.destroy();
+        });
+    }
+
+    // see http://trac.osgeo.org/openlayers/ticket/3264
+    function test_externalGraphic_moveTo(t) {
+        if (!supported) {
+            t.plan(0);
+            return;
+        }
+
+        t.plan(2);
+
+        // set up
+
+        var layer = new OpenLayers.Layer.Vector(null, {
+            isBaseLayer: true,
+            renderers: ["Canvas"]
+        });
+
+        var map = new OpenLayers.Map({
+            div: "map",
+            controls: [],
+            layers: [layer],
+            center: new OpenLayers.LonLat(0, 0),
+            zoom: 0
+        });
+
+        var feature = new OpenLayers.Feature.Vector(
+            new OpenLayers.Geometry.Point(0, 0),
+            null,
+            {
+                externalGraphic: '../../img/marker.png',
+                graphicHeight: 20,
+                graphicWidth: 20,
+                graphicXOffset: 0,
+                graphicYOffset: 0
+            }
+        );
+
+        layer.addFeatures([feature]);
+
+        // test
+
+        // delay_call to let the first drawImage (the one
+        // resulting from addFeatures) run
+        t.delay_call(0.1, function() {
+
+            var log = [];
+            layer.renderer.canvas.drawImage = function(img, x, y, w, h) {
+                log.push({x: x, y: y});
+            };
+
+            layer.renderer.redraw();
+            map.setCenter(new OpenLayers.LonLat(45, 0), 0);
+
+            t.delay_call(0.1, function() {
+                t.eq(log.length, 2,
+                     "canvas.drawImage called twice");
+                t.ok(log[0].x == log[1].x && log[0].y == log[1].y,
+                     "image drawn at the same location");
+
+                // tear down
+                map.destroy();
+            });
+        });
+    }
+
   </script>
 </head>
 <body>