Phaser.Ellipse.contains() broken

[spayton]

It looks like the recent update to this function broke it (imported from PIXI it seems).

Here's how to demonstrate the problem. Start with the current dev phaser loaded in the browser and open the dev console.

Create an ellipse by typing:

testEllipse = new Phaser.Ellipse(0, 0, 200, 100);

then test if the most upper-left coordinate is within the ellipse by typing:

Phaser.Ellipse.contains(testEllipse,0,0);

it will return true - which is wrong (a corner pixel is not within the ellipse)

by comparision, you can add the old code and re-test - paste this chunk into the console:

Phaser.Ellipse.containsOLD = function (a, x, y) {

    if (a.width <= 0 || a.height <= 0) {
        return false;
    }

    //  Normalize the coords to an ellipse with center 0,0 and a radius of 0.5
    var normx = ((x - a.x) / a.width) - 0.5;
    var normy = ((y - a.y) / a.height) - 0.5;

    normx *= normx;
    normy *= normy;

    return (normx + normy < 0.25);

};

and type the following to repeat the test using the old 'contains' function:

Phaser.Ellipse.containsOLD(testEllipse,0,0);

this time the return is false - which is right, ie the corner pixel is not in the ellipse.

Either fix the new calculation method or revert to the old method.

[pnstickne]

I'm fine with a local Phaser patch - I suspect I'm not alone.

Care to create a PR?

[photonstorm]

I'd like to keep differences to Pixi as minimal as possible, but I'm no longer worried about maintaining absolute parity - so am happy to have this fixed locally in Phaser if you'd like to create a PR.

[spayton]

I've just checked the current dev branch and it behaves similarly broken as expected. Tested with the following in the browser console with just PIXI loaded:

testEllipse = new PIXI.Ellipse(0, 0, 200, 100);
testEllipse.contains(0,0)

So, both libs seems to require a fix one way or another. Would it be best to fix in PIXI first then import the mods into Phaser - that way alignment is maintained and both get fixed?

[photonstorm]

If you're going to submit a PR then feel free to do it against both. I can't say when Pixi will merge it as they're starting off on some major rewrite, but I'll merge it into Phaser dev instantly.

[spayton]

Ok, before reverting I'll test the old version properly, make sure it works in differing cases and not just my simple use where the ellipse xy position = 0.

[photonstorm]

Just wondering how you got on with this? Should we close the issue off?

[spayton]

Argh, I put this aside for a bit with the intention to see why PIXI was also doing this, I did a little digging then got sidetracked. The fix I suggested (just reverting back) is fine but I did see that there is room for a little optimisation too. I’ve been running with the broken function overridden for now.

I’ll do it now.

[photonstorm]

Thanks for the code. Now in dev :)