Skip to content

Commit

Permalink
Use fround() in FlxSprite.draw() instead of FlxObject.updateMotion()
Browse files Browse the repository at this point in the history
  • Loading branch information
@Gama11
Gama11 committed Feb 10, 2014
1 parent 85ecb60 commit 54f5780
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 8 deletions.
6 changes: 0 additions & 6 deletions flixel/FlxObject.hx
View file
Expand Up @@ -295,12 +295,6 @@ class FlxObject extends FlxBasic
*/
private inline function updateMotion():Void
{
// Hack to deal with floating point precision issues in flash
#if flash
x = Math.fround(x);
y = Math.fround(y);
#end

var delta:Float;
var velocityDelta:Float;

Expand Down
5 changes: 3 additions & 2 deletions flixel/FlxSprite.hx
View file
Expand Up @@ -768,8 +768,9 @@ class FlxSprite extends FlxObject
#if flash
if (simpleRender)
{
_flashPoint.x = Math.floor(_point.x);
_flashPoint.y = Math.floor(_point.y);
// use fround() to deal with floating point precision issues in flash
_flashPoint.x = Math.floor(Math.fround(_point.x));
_flashPoint.y = Math.floor(Math.fround(_point.y));

This comment has been minimized.

Sign in to view

Copy link
@gamedevsam

gamedevsam Feb 10, 2014

Contributor

Do we need both Math.floor and Math.fround here? I would imagine one would be enough?

This comment has been minimized.

Sign in to view

Copy link
@Gama11

Gama11 Feb 10, 2014

Author Member

Well, it does fix the jittering issue when standing on this platform in @Eiyeron's game. I'm not sure it's the best solution, but it works.

For some reason, y is alternating between 105 and 104.9999999999 as you can see in the debugger's watch window, which leads to Math.floor() rounding down to 14 in some cases. I guess it might be nicer to use Math.round() here?

This comment has been minimized.

Sign in to view

Copy link
@Gama11

Gama11 Feb 10, 2014

Author Member

I guess this is similar to this fix in flixel 2.55.

This comment has been minimized.

Sign in to view

Copy link
@Gama11

Gama11 Feb 10, 2014

Author Member

The Math.floor() might be unnecessary...

This comment has been minimized.

Sign in to view

Copy link
@gamedevsam

gamedevsam Feb 10, 2014

Contributor

It's worth testing. It would be a much nicer solution.

This comment has been minimized.

Sign in to view

Copy link
@Gama11

Gama11 Feb 10, 2014

Author Member

There don't seem to be any immediate sideeffects from removing it... But I'm not sure why the Math.floor() is there in the first place, it's not in the original flixel. It might be a replacement for the ugly ? + 0.001 : - 0.001 logic it had... @Beeblerox?

This comment has been minimized.

Sign in to view

Copy link
@Beeblerox

Beeblerox Feb 11, 2014

Member

i don't remember the reason this change was made. So i can't say about any side effects. only testing will show if it has flaws

This comment has been minimized.

Sign in to view

Copy link
@gamedevsam

gamedevsam Feb 11, 2014

Contributor

I say remove it and wait to see if we get a jittering bug or something related to that.

This comment has been minimized.

Sign in to view

Copy link
@Beeblerox

Beeblerox Feb 11, 2014

Member

yes, it is what i wanted to say :)

This comment has been minimized.

Sign in to view

Copy link
@Gama11

Gama11 Feb 11, 2014

Author Member

Alright.

This comment has been minimized.

Sign in to view

Copy link
@JoeCreates

JoeCreates Apr 10, 2014

Member

Simply rounding did cause issues.

camera.buffer.copyPixels(framePixels, _flashRect, _flashPoint, null, null, true);
}
Expand Down

4 comments on commit 54f5780

@Eiyeron
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not rounding it only for rendering?

@Eiyeron
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nevermind* i'm on a phone. Sorry.

Will this fix the camera too? Some were issuing camera jittering.

@Gama11
Copy link
Member Author

@Gama11 Gama11 commented on 54f5780 Feb 10, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No... funnily enough, the initial fix in updateMotion() kind of fixed the camera jittering (#797).

@Eiyeron
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the camera why not frounding the target's position so?

Please sign in to comment.