Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inconsistent use of Math.floor() breaks canvas, mouse #9

Closed
joshtynjala opened this issue Jan 2, 2011 · 7 comments
Closed

Inconsistent use of Math.floor() breaks canvas, mouse #9

joshtynjala opened this issue Jan 2, 2011 · 7 comments

Comments

@joshtynjala
Copy link

Canvas drawing (at least in Firefox 3.6; it seemed to work without issue in Chrome 8) and the bounds defined by areaMap can become wildly offset from where they are expected if an object repeatedly moves to new x,y positions that are not integers. I'm doing programmatic animation over time that frequently results in sprite positions being set to non-integer values.

The cause seems to be from comparisons between the modified values returned by pos() and other values that remain unmodified.

A fix that seems to work for me is to move the Math.floor() calls into the setters:

this.defineSetter('x', function(v) { this._attr('_x', Math.floor(v)); });

That may create other issues, though. If at all possible, I think that it would be best to allow non-integer values everywhere, but I'm not experienced enough with canvas to know if this is possible cross-browser.

@louisstow
Copy link
Member

Thanks for the catch. I was worried if something like this would happen. To my knowledge, canvas doesn't like non-integer values when calling drawImage().

It was my intention to leave them as floats (for want of a better word) until drawing and then convert. Will take a look where pos() is being used.

@louisstow
Copy link
Member

Actually do you have a live example I could look at or even email me a zip? Would make it a lot quicker and fixable very shortly.

@joshtynjala
Copy link
Author

The code below should do it. You just need some images for background.png and mover.png that are the specified sizes. Make sure background.png is dark enough to contrast well with a blank canvas.

When the mover image moves back to the left, it will leave artifacts in its trail. I discovered while trying to build this that it's actually the background must appear at a non-integer position, rather than mover. Maybe the rectangle used to clear the canvas is different than the rectangle used to redraw the background?

By the way, this doesn't reproduce for me in Chrome, but I could see the bug in Firefox, Safari, and Opera.

Crafty.init(50, 320, 240);
Crafty.canvas();
Crafty.sprite("images/background.png",
{
    background: [0, 0, 321, 243]
});
Crafty.sprite("images/mover.png",
{
    mover: [0, 0, 25, 25]
});

var bg = Crafty.e("2D, canvas, background");
bg.x = (Crafty.viewport.width - bg.w) / 2;
bg.y = (Crafty.viewport.width - bg.w) / 2;
console.log("bg x: " + bg.x + ", bg y: " + bg.y);

var mover = Crafty.e("2D, canvas, mover");
var modifier = 5;
mover.bind("enterframe", function()
{
    mover.x += modifier;
    if(mover.x >= Crafty.viewport.width - mover.w || mover.x < 0)
    {
        modifier *= -1;
    }
});

@joshtynjala
Copy link
Author

Oops. I accidentally closed this issue when I commented. I don't see a way to reopen, so I think you'll need to do it. Sorry about that.

@louisstow
Copy link
Member

So I think the problem was with trying to draw elements out of the canvas bounds. Have now fixed it by modifying the drawImage values to only draw what's visible. Will upload when a few other bugs are fixed and will release as a new version.

                    //if out of bounds from the canvas, crop
        if(pos._x < 0) {
            //negative minus negative
            co._x -= pos._x;
            pos._x = 0;
        }

        if(pos._y < 0) {
            co._y -= pos._y;
            pos._y = 0;
        }

        if(pos._x + pos._w > Crafty.viewport.width) {
            co._w += (Crafty.viewport.width - pos._x + pos._w);
            pos._w = co._w;
        }

        if(pos._y + pos._h > Crafty.viewport.height) {
            co._h += (Crafty.viewport.height - pos._y + pos._h);
            pos._h = co._h;
        }

Thanks!

@louisstow
Copy link
Member

My mistake. That code was wrong. The problem was you were giving the sprite() function the wrong values. Here are the correct values:

Crafty.sprite("bg.png", {
    background: [0, 0, 321, 240]
});

@joshtynjala
Copy link
Author

I specifically designed the test case to center an image so that Crafty would need to work with partial pixels, both vertically and horizontally. 321x243 is the correct dimensions for the image I used. Crafty's stage is a bit smaller at 320x240. The background, in that case, would be positioned at -0.5, -1.5.

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants