Input brush isn't properly cleared when new brush is drawn

[cpsievert]

Observed in RStudio viewer 1.1.456 with v1.2-rc in https://github.com/rstudio/shiny-examples/tree/master/108-module-output

Interestingly, the issue doesn't occur in Chrome (with v1.2-rc), nor does it appear in the viewer with shiny v1.1

[jcheng5]

@schloerke, @cpsievert and I did some investigation and learned some things. There are three separate issues here.

Some code that imports and removes the previous brush is not being called

This is an issue that was introduced in #2198 (merged this morning). This block of code waits for the image to load, then imports the old brush (and removes the old brush's <div> when complete):

shiny/srcjs/output_binding_image.js

Lines 1032 to 1035 in 1d0edd2

	$el.find("img").one("load.shiny-image-interaction", function() {
	brush.importOldBrush();
	brushInfoSender.immediateCall();
	});

#2198 moved the caller of this code into $img.on("load"), so the brush.importOldBrush() etc. will never be called. The fix is simple, just run these lines immediately instead of using the load event.

Each time a brush occurs, the plot is redrawn twice.

With options(shiny.trace=TRUE), we can see some differences in the JSON. The first plot's input/output looks like this (this is from the server's perspective, so RECV is stuff the client sends to the server, and SEND is the opposite):

RECV {"method":"update","data":{"scatters-brush":{"xmin":13.055710626153,"xmax":20.25859409253,"ymin":18.528245419468,"ymax":31.066840387247,"coords_css":{"xmin":130,"xmax":260,"ymin":155,"ymax":284},"coords_img":{"xmin":260,"xmax":520,"ymin":310,"ymax":568},"img_css_ratio":{"x":2,"y":2},"mapping":{"colour":"selected_","x":"cty","y":"hwy"},"domain":{"left":7.7,"right":36.3,"bottom":10.4,"top":45.6},"range":{"left":66.6767608694834,"right":1099.04109589041,"bottom":735.250582988923,"top":10.958904109589},"log":{"x":null,"y":null},"direction":"xy","brushId":"scatters-brush","outputId":"scatters-plot1"}}}
SEND {"errors":[],"values":{"summary":"120 observation(s) selected","scatters-plot1":{"src":"data:image/png;[base64 data]","width":555,"height":400,"coordmap":{"panels":[{"panel":1,"row":1,"col":1,"panel_vars":{},"log":{"x":null,"y":null},"domain":{"left":7.7,"right":36.3,"bottom":10.4,"top":45.6},"mapping":{"colour":"selected_","x":"cty","y":"hwy"},"range":{"left":66.6767608694834,"right":1099.04109589041,"bottom":735.250582988923,"top":10.958904109589}}],"dims":{"width":1110,"height":800}}},"scatters-plot2":{"src":"data:image/png;[base64 data]","width":555,"height":400,"coordmap":{"panels":[{"panel":1,"row":1,"col":1,"panel_vars":{},"log":{"x":null,"y":null},"domain":{"left":0.4,"right":3.6,"bottom":10.4,"top":45.6},"mapping":{"colour":"selected_","x":"drv","y":"hwy"},"range":{"left":66.6767608694834,"right":1099.04109589041,"bottom":735.250582988923,"top":10.958904109589}}],"dims":{"width":1110,"height":800}}}},"inputMessages":[]}

The second looks like this:

RECV {"method":"update","data":{"scatters-brush":{"xmin":13.055710626153,"xmax":20.25859409253,"ymin":18.528245419468,"ymax":31.066840387247,"coords_css":{"xmin":130.00000000000307,"xmax":259.99999999999466,"ymin":154.99999999999622,"ymax":284.00000000000506},"coords_img":{"xmin":260.00000000000614,"xmax":519.9999999999893,"ymin":309.99999999999244,"ymax":568.0000000000101},"img_css_ratio":{"x":2,"y":2},"mapping":{"colour":"selected_","x":"cty","y":"hwy"},"domain":{"left":7.7,"right":36.3,"bottom":10.4,"top":45.6},"range":{"left":66.6767608694834,"right":1099.04109589041,"bottom":735.250582988923,"top":10.958904109589},"log":{"x":null,"y":null},"direction":"xy","brushId":"scatters-brush","outputId":"scatters-plot1"}}}
SEND {"errors":[],"values":{"summary":"120 observation(s) selected","scatters-plot1":{"src":"data:image/png;[base64 data]","width":555,"height":400,"coordmap":{"panels":[{"panel":1,"row":1,"col":1,"panel_vars":{},"log":{"x":null,"y":null},"domain":{"left":7.7,"right":36.3,"bottom":10.4,"top":45.6},"mapping":{"colour":"selected_","x":"cty","y":"hwy"},"range":{"left":66.6767608694834,"right":1099.04109589041,"bottom":735.250582988923,"top":10.958904109589}}],"dims":{"width":1110,"height":800}}},"scatters-plot2":{"src":"data:image/png;[base64 data]","width":555,"height":400,"coordmap":{"panels":[{"panel":1,"row":1,"col":1,"panel_vars":{},"log":{"x":null,"y":null},"domain":{"left":0.4,"right":3.6,"bottom":10.4,"top":45.6},"mapping":{"colour":"selected_","x":"drv","y":"hwy"},"range":{"left":66.6767608694834,"right":1099.04109589041,"bottom":735.250582988923,"top":10.958904109589}}],"dims":{"width":1110,"height":800}}}},"inputMessages":[]}

The diff:

So, that's one thing. This causes the server to generate the same exact plot twice.

On WebKit (only), an image's load event doesn't fire when you try to load the same image that's already displaying

https://stackoverflow.com/questions/5024111/javascript-image-onload-doesnt-fire-in-webkit-if-loading-same-image

And "try to load the same image that's already displaying" is exactly what the previous heading causes to happen.

This is causing all of the logic that's directly in renderValue to be executed, but the rest of the initialization code that waits for $img.on("load") never gets called.

[jcheng5]

@schloerke Temporarily setting .src to a dummy value seems to work around the load event not firing:

[jcheng5]

Actually, better than img.src = "about:blank";, is img.removeAttribute("src").

[jcheng5]

Relevant for the second issue, of the plot being drawn twice because coords_css and coords_img are slightly off:
#1634

[schloerke]

I never applied the rounding to 14 digits code to coords_imag and coords_css. So that makes sense.

shiny/srcjs/output_binding_image.js

Lines 1259 to 1261 in 3cea5fb

	// Round to 14 significant digits to avoid spurious changes in FP values
	// (#1634).
	state.boundsData = mapValues(state.boundsData, val => roundSignif(val, 14));