KineticJS icon indicating copy to clipboard operation
KineticJS copied to clipboard

Incorrect shape hit area (in pixelRatio+clip scenario)

Open zachkinstner opened this issue 10 years ago • 11 comments

A shape's hit area does not align with its visual position in the following scenario:

  • Use a pixelRatio not equal to one
  • Contain the shape within a group that has clipping bounds

The code below demonstrates the issue using version 5.1.0. The gray actualHitArea shape predicts where the green box shape's hit area will be. The box shape's hit area (both size and position) is being incorrectly scaled by the pixelRatio.

<html>

    <head>
        <title>PixelRatio Hit Area Issue</title>
        <script src="kinetic-v5.1.0.js"></script>
    </head>

    <body>
        <div id="container">
        </div>
    </body>

    <script type="text/javascript">
        Kinetic.pixelRatio = 2;

        var stage = new Kinetic.Stage({
            container: 'container',
            width: 400,
            height: 400
        });

        var layer = new Kinetic.Layer();
        stage.add(layer);

        var group = new Kinetic.Group({
            clip: {
                x: 0,
                y: 0,
                width: 400,
                height: 400
            }
        });
        layer.add(group);

        var box = new Kinetic.Rect({
            x: 60,
            y: 30,
            width: 40,
            height: 40,
            fill: '#0f0'
        });
        box.on('touchstart', function() { console.log('touchstart'); });
        box.on('mousedown', function() { console.log('mousedown'); });
        group.add(box);

        var actualHitArea = new Kinetic.Rect({
            x: box.x()*Kinetic.pixelRatio,
            y: box.y()*Kinetic.pixelRatio,
            width: box.width()*Kinetic.pixelRatio,
            height: box.height()*Kinetic.pixelRatio,
            fill: '#eee'
        });
        group.add(actualHitArea);

        stage.draw();
    </script>

</html>

zachkinstner avatar Apr 08 '14 19:04 zachkinstner

It is not reproducing on a real device. I tested on ipad 3. So I am not sure it is a bug.

lavrton avatar Apr 09 '14 13:04 lavrton

I am able to reproduce this on my iPhone and iPad devices (both have Retina screens).

I wrote a new demo to ensure we're looking at the same thing -- available here. The mouse/touch event changes the box color rather than using console.log(). Also, this demo does not set Kinetic.pixelRatio directly. Instead, the actualHitArea shape scales by the value of window.devicePixelRatio to predict where the hit area will be.

Note: the original HTML code demonstrates the issue regardless of the device's native pixelRatio (because it forces the value to be 2). This new HTML code only demonstrates the issue when viewing on a device that has a native pixelRatio that is not equal to 1.

The new demo code:

<html>

    <head>
        <title>PixelRatio Hit Area Issue</title>
        <script src="kinetic-v5.1.0.js"></script>
    </head>

    <body>
        <div id="container">
        </div>
    </body>

    <script type="text/javascript">
        var stage = new Kinetic.Stage({
            container: 'container',
            width: 400,
            height: 400
        });

        var layer = new Kinetic.Layer();
        stage.add(layer);

        var group = new Kinetic.Group({
            clip: {
                x: 0,
                y: 0,
                width: 400,
                height: 400
            }
        });
        layer.add(group);

        var box = new Kinetic.Rect({
            x: 60,
            y: 30,
            width: 40,
            height: 40,
            fill: '#0f0'
        });

        var hitFunc = function() {
            box.fill(box.fill() == '#00f' ? '#0f0' : '#00f');
            stage.draw();
        };

        box.on('touchstart', hitFunc);
        box.on('mousedown', hitFunc);
        group.add(box);

        var pr = window.devicePixelRatio;

        var actualHitArea = new Kinetic.Rect({
            x: box.x()*pr,
            y: box.y()*pr,
            width: box.width()*pr,
            height: box.height()*pr,
            fill: '#eee'
        });
        group.add(actualHitArea);
        actualHitArea.moveToBottom();

        stage.draw();
    </script>

</html>

zachkinstner avatar Apr 09 '14 17:04 zachkinstner

I've tried a few different ideas for a fix, but they always create side-effect issues. Here is something that I noticed -- maybe it will be helpful.

First, I added this.isHit = true to Kinetic.HitCanvas for use with the debug output. When I add the following outputs to the Container.js code...

        _drawChildren: function(canvas, drawMethod, top) {
            [...]

            if (hasClip && layer) {
                console.log('hit='+(canvas.isHit==true)+' / pr='+canvas.getPixelRatio());
                console.log(context._context.mozCurrentTransform || context._context);

                clipX = this.getClipX();
                clipY = this.getClipY();

                context.save();
                layer._applyTransform(this, context);
                context.beginPath();
                [...]

...it produces this output (in Firefox):

hit=false / pr=2
[2, 0, 0, 2, 0, 0]
hit=true / pr=2
[1, 0, 0, 1, 0, 0]

Perhaps the scene/hit contexts are supposed to have the same transformation state here? I'm not sure if mozCurrentTransform is reliable, but I couldn't find another way to access this information.

zachkinstner avatar Apr 10 '14 19:04 zachkinstner

Have you found any issues adding only this line? it seems to fix the issue for me

Kinetic.HitCanvas = function(config) {
    config = config || {};
    config.pixelRatio = 1; //force a pixelRatio of 1 for the hit canvas
    [...]

kaplag avatar Jul 18 '14 14:07 kaplag

Hi @kaplag, that fix also seemed to work for me. It was the end result of my earlier pull request: https://github.com/ericdrowell/KineticJS/pull/885/files

However, that pull request caused some failed test cases, so I closed it.

zachkinstner avatar Jul 18 '14 15:07 zachkinstner

@zachkinstner, ah ok. I thought the other change in that pull was what caused issues. So this doesn't fully solve the problem but works for some cases? Thank you for posting it. It made a big difference to the quality of our project.

kaplag avatar Jul 18 '14 18:07 kaplag

@kaplag, this was my first look into the internals of the KineticJS project, so I can't say for sure what side-effects that fix might have.

My guess is that this change causes the "hit canvas" to render at a pixelRatio of 1, and (somehow) the "hit canvas" scales up to match the size of the "draw canvas" (which has a pixelRatio of 2). The result would be that, while the two canvases have the same dimensions, the drawing resolution of the "hit canvas" is not as good. (Again, my guess is...) This may cause issues for the hit-states of thin lines or shapes with lots of fine-grained detail.

zachkinstner avatar Jul 18 '14 20:07 zachkinstner

@zachkinstner, If that is the only down side it's not bad. The only work around we had before was setting pixelRatio to 1 anyway so both the "draw canvas" and "hit canvas" would have equally bad resolution. I don't think this would make to much of a difference on a touch device but I"ll post findings to here as we test more. My project is dependent on moving small shapes sometimes.

I'm surprised this didn't make it into the next milestone. Hdpi is now the norm and using clipping masks is common enough.

kaplag avatar Jul 18 '14 20:07 kaplag

I'm having this issue as well. Working on a Retina MBPr and trying to clip a Group containing an Image. I'm unable to detect events over the Image unless I set Kinetic.pixelRatio = 1.

frnsys avatar Jul 24 '14 23:07 frnsys

@ftzeng, have you tried the solution @zachkinstner came up with? it works for me on retina iPad. Haven't tried it on laptop. The problem seems to be the hitCanvas is being drawn at twice the resolution but is not being scaled down properly like the drawCanvas so all of the hit shapes are there but off. The work around seems to be editing the library to force only the Kinetic.HitCanvas to pixelRatio 1. The drawCanvas can then be left alone to automatically detect pixelRatio and will show retina quality.

Unfortunately I do not know where to add this in the min version of kinetic. Any idea?

kaplag avatar Jul 25 '14 14:07 kaplag

@kaplag, I haven't tried that solution cause I want to avoid editing the library itself if I can...for now, I'll just go with overriding the pixelRatio to be 1.

frnsys avatar Jul 25 '14 16:07 frnsys