Apply viewport transform when calculating isTargetTransparent()

[kherock]

This fixes a bug with how an interactive canvas with perPixelTargetFind behaves after being panned/zoomed.

Version

1.6.2

Test Case

http://jsfiddle.net/d10y74q7/5/

Steps to reproduce

1. Turn perPixelTargetFind on for a Canvas instance
2. Modify the viewport of the canvas (pan or zoom)
3. Attempt to select objects of the interactive canvas

Expected Behavior

Cursor should change to the move cursor when over opaque pixels, and clicking & dragging should move items.

Actual Behavior

Objects can only be interacted with when the XY coordinates of the pointer are coincidentally over opaque pixels in the canvas with an unmodified viewport.

[kherock]

Ok, I just discovered that the fix is still incomplete with my commit as the resize handles are still not selectable after panning/zooming. I'll see if I can figure out what's wrong.

EDIT: I adjusted the commit, it turned out that _renderControls(ctx) already handles applying the viewport transform, so the context just needed to be restored before that line. This issue appears to be completely fixed now.

[asturur]

i'm guessing if is better to transform the coordinate of mouse or the context from a performance point of view.
I added that shouldTransform because i did not want to save restore and transform on every mouse move when isTargetTransparent is used.

[kherock]

I suppose the only optimization that could be made there is to check that viewportTransform hasn't deviated from its default setting before we start saving or restoring the context. However it should be expected that there be some performance hit when we have perPixelTargetFind turned on.

What do you suggest should be done to get this resolved?

[kherock]

Keep in mind, this transform is already being done when _renderControls(ctx) is called. If you wanted, we could just have _renderControls() ignore the viewport and have the caller take care of that, like it currently is in renderAll() calling render():

/* static_canvas.class.js */

    renderAll: function () {
      ...
      canvasToDrawOn.save();
      objsToRender = this._chooseObjectsToRender();
      //apply viewport transform once for all rendering process
      canvasToDrawOn.transform.apply(canvasToDrawOn, this.viewportTransform);
      this._renderObjects(canvasToDrawOn, objsToRender);
      ...
    }

[asturur]

no renderControls can't be touched easily. Jsut guessing if is lighter to tranform a context or a mouse coordinates.

[kherock]

Ok. I'm not sure transforming mouse coordinates would be a proper fix though. I updated the jsfiddle to better illustrate the issue. The problem with transforming mouse coordinates is that when your transformed pointer leaves the original viewport, it is seen as transparent since the cache canvas has not rendered that area.

[asturur]

I'm getting a mac next week. waiting for it so i test this + retina screen and i will merge or ask you for tweak.

[kherock]

Any updates?

By the way, you can set the device pixel ratio with Chrome's dev tools or just by setting the browser's zoom level before the page loads. I've tested that to all work fine.

[asturur]

got mac yesterday. I know i can test with device pixel ration ( also with browser zoom sadly, that is the problem ). I need some days to get back on the issues and prs.

[kherock]

Bumping. Have you had time to test this recently?