Image and shape clip paths

src/components/images/draw.js

@@ -143,9 +143,10 @@ module.exports = function draw(gd) {
             yId = ya ? ya._id : '',
             clipAxes = xId + yId;
 
-        if(clipAxes) {
-            thisImage.call(Drawing.setClipUrl, 'clip' + fullLayout._uid + clipAxes);
-        }
+        thisImage.call(Drawing.setClipUrl, clipAxes ?
+            ('clip' + fullLayout._uid + clipAxes) :
+            null
+        );
     }
 
     var imagesBelow = fullLayout._imageLowerLayer.selectAll('image')

src/components/shapes/draw.js

@@ -70,13 +70,11 @@ function drawOne(gd, index) {
     // TODO: use d3 idioms instead of deleting and redrawing every time
     if(!optionsIn || options.visible === false) return;
 
-    var clipAxes;
+    // var clipAxes;
     if(options.layer !== 'below') {
-        clipAxes = (options.xref + options.yref).replace(/paper/g, '');
         drawShape(gd._fullLayout._shapeUpperLayer);
     }
     else if(options.xref === 'paper' && options.yref === 'paper') {
-        clipAxes = '';
         drawShape(gd._fullLayout._shapeLowerLayer);
     }
     else {
@@ -86,10 +84,13 @@ function drawOne(gd, index) {
 
         for(i = 0, n = subplots.length; i < n; i++) {
             plotinfo = plots[subplots[i]];
-            clipAxes = subplots[i];
 
             if(isShapeInSubplot(gd, options, plotinfo)) {
                 drawShape(plotinfo.shapelayer);
+
+                // make sure we only draw the shape once.
+                // See https://github.com/plotly/plotly.js/issues/1452
+                break;
             }
         }
     }
@@ -110,10 +111,15 @@ function drawOne(gd, index) {
             .call(Color.fill, options.fillcolor)
             .call(Drawing.dashLine, options.line.dash, options.line.width);
 
-        if(clipAxes) {
-            path.call(Drawing.setClipUrl,
-                'clip' + gd._fullLayout._uid + clipAxes);
-        }
+        // note that for layer="below" the clipAxes can be different from the
+        // subplot we're drawing this in. This could cause problems if the shape
+        // spans two subplots. See https://github.com/plotly/plotly.js/issues/1452
+        var clipAxes = (options.xref + options.yref).replace(/paper/g, '');
+
+        path.call(Drawing.setClipUrl, clipAxes ?
+            ('clip' + gd._fullLayout._uid + clipAxes) :
+            null
+        );
 
         if(gd._context.editable) setupDragElement(gd, path, options, index);
     }

test/jasmine/tests/layout_images_test.js

@@ -307,7 +307,7 @@ describe('Layout images', function() {
 
         afterEach(destroyGraphDiv);
 
-        it('should properly add and removing image', function(done) {
+        it('should properly add and remove image', function(done) {
             var gd = createGraphDiv(),
                 data = [{ x: [1, 2, 3], y: [1, 2, 3] }],
                 layout = { width: 500, height: 400 };
@@ -425,23 +425,38 @@ describe('images log/linear axis changes', function() {
         // we don't try to figure out the position on a new axis / canvas
         // automatically when you change xref / yref, we leave it to the caller.
 
+        // initial clip path should end in 'xy' to match xref/yref
+        expect(d3.select('image').attr('clip-path') || '').toMatch(/xy\)$/);
+
         // linear to log
         Plotly.relayout(gd, {'images[0].yref': 'y2'})
         .then(function() {
             expect(gd.layout.images[0].y).toBe(1);
 
+            expect(d3.select('image').attr('clip-path') || '').toMatch(/xy2\)$/);
+
             // log to paper
             return Plotly.relayout(gd, {'images[0].yref': 'paper'});
         })
         .then(function() {
             expect(gd.layout.images[0].y).toBe(1);
 
+            expect(d3.select('image').attr('clip-path') || '').toMatch(/x\)$/);
+
+            // change to full paper-referenced, to make sure the clip path disappears
+            return Plotly.relayout(gd, {'images[0].xref': 'paper'});
+        })
+        .then(function() {
+            expect(d3.select('image').attr('clip-path')).toBe(null);
+
             // paper to log
             return Plotly.relayout(gd, {'images[0].yref': 'y2'});
         })
         .then(function() {
             expect(gd.layout.images[0].y).toBe(1);
 
+            expect(d3.select('image').attr('clip-path') || '').toMatch(/^[^x]+y2\)$/);
+
             // log to linear
             return Plotly.relayout(gd, {'images[0].yref': 'y'});
         })

test/jasmine/tests/shapes_test.js

@@ -382,6 +382,71 @@ describe('Test shapes:', function() {
     });
 });
 
+describe('shapes axis reference changes', function() {
+    'use strict';
+
+    var gd;
+
+    beforeEach(function(done) {
+        gd = createGraphDiv();
+
+        Plotly.plot(gd, [
+            {y: [1, 2, 3]},
+            {y: [1, 2, 3], yaxis: 'y2'}
+        ], {
+            yaxis: {domain: [0, 0.4]},
+            yaxis2: {domain: [0.6, 1]},
+            shapes: [{
+                xref: 'x', yref: 'paper', type: 'rect',
+                x0: 0.8, x1: 1.2, y0: 0, y1: 1,
+                fillcolor: '#eee', layer: 'below'
+            }]
+        }).then(done);
+    });
+
+    afterEach(destroyGraphDiv);
+
+    function getShape(index) {
+        var s = d3.selectAll('path[data-index="' + index + '"]');
+        expect(s.size()).toBe(1);
+        return s;
+    }
+
+    it('draws the right number of objects and updates clip-path correctly', function(done) {
+
+        expect(getShape(0).attr('clip-path') || '').toMatch(/x\)$/);
+
+        Plotly.relayout(gd, {
+            'shapes[0].xref': 'paper',
+            'shapes[0].x0': 0.2,
+            'shapes[0].x1': 0.6
+        })
+        .then(function() {
+            expect(getShape(0).attr('clip-path')).toBe(null);
+
+            return Plotly.relayout(gd, {
+                'shapes[0].yref': 'y2',
+                'shapes[0].y0': 1.8,
+                'shapes[0].y1': 2.2,
+            });
+        })
+        .then(function() {
+            expect(getShape(0).attr('clip-path') || '').toMatch(/^[^x]+y2\)$/);
+
+            return Plotly.relayout(gd, {
+                'shapes[0].xref': 'x',
+                'shapes[0].x0': 1.5,
+                'shapes[0].x1': 20
+            });
+        })
+        .then(function() {
+            expect(getShape(0).attr('clip-path') || '').toMatch(/xy2\)$/);
+        })
+        .catch(failTest)
+        .then(done);
+    });
+});
+
 describe('shapes autosize', function() {
     'use strict';