Skip to content

Comments

Round drawable position#339

Merged
thisandagain merged 1 commit intoscratchfoundation:developfrom
mzgoddard:round-position
Oct 29, 2018
Merged

Round drawable position#339
thisandagain merged 1 commit intoscratchfoundation:developfrom
mzgoddard:round-position

Conversation

@mzgoddard
Copy link
Contributor

Resolves

Fixes scratchfoundation/scratch-vm#1453

Proposed Changes

Round the sprite position received from the VM.

Reason for Changes

Scratch 2 rendered sprite position is at whole numbers, while its vm position may be rational numbers.

Reviewing https://github.com/LLK/scratch-flash/blob/develop/src/scratch/ScratchSprite.as#L180-L186 again I see the error I made in scratchfoundation/scratch-vm#1453. x and y in ScratchSprite.as are the rendering position. scratchX and scratchY are the vm position. x and y are defined by the Sprite class that ScratchSprite inherits from its parent ScratchObj.

Test Coverage

I haven't identified a project this improves yet. In theory it should fix projects with sprites with a 1-pixel blur because it is rendered with a rational position instead of a whole number position. It should also fix some high-precision issues with touching object and color. It may be one cause for these problems and not fix all instances of these issues.

Scratch 2 rendered sprite position is at whole numbers, while its vm
position may be rational numbers.
@mzgoddard mzgoddard requested a review from cwillisf August 24, 2018 22:31
@thisandagain thisandagain added this to the August 2018 milestone Aug 27, 2018
@thisandagain thisandagain changed the title round drawable position Round drawable position Aug 27, 2018
Copy link
Contributor

@cwillisf cwillisf left a comment

Choose a reason for hiding this comment

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

👍

@thisandagain
Copy link
Contributor

@cwillisf @mzgoddard Should this be merged for testing?

@fsih
Copy link
Contributor

fsih commented Oct 24, 2018

@cwillisf @mzgoddard is there a reason we haven't merged this?

@thisandagain thisandagain merged commit 9ab14e3 into scratchfoundation:develop Oct 29, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants