Skip to content

round x and y like Scratch 2 does#1453

Merged
rschamp merged 1 commit intoscratchfoundation:developfrom
mzgoddard:round-setxy
Aug 17, 2018
Merged

round x and y like Scratch 2 does#1453
rschamp merged 1 commit intoscratchfoundation:developfrom
mzgoddard:round-setxy

Conversation

@mzgoddard
Copy link
Contributor

Proposed Changes

Use Math.round instead of this._roundCoord.

Reason for Changes

https://github.com/LLK/scratch-flash/blob/develop/src/scratch/ScratchSprite.as#L180-L186

Scratch 2 rounds the X and Y to the nearest whole number.

Test Coverage

Projects like https://llk.github.io/scratch-gui/develop/#238746871 with fine lines and fractional positions needs rounded x and y values so that their lines are not offset from whole numbers. Offset lines from whole numbers in Scratch 3 render "inbetween pixels" and come out faded out or with striations.

https://scratch.mit.edu/projects/238746871/
https://llk.github.io/scratch-gui/develop/#238746871

@towerofnix
Copy link
Contributor

This is the only place in the code which uses the (private) function _roundCoord, so does it make sense to get rid of that function now?

@mrjacobbloom
Copy link

mrjacobbloom commented Aug 15, 2018

From some testing it looks like in 2.0, the x position reporter is precise to 8 digits and the x position monitor is precise to 6 digits. That specific rounding was added to fix project 117386152 (see #1150) -- does rounding to integer happen to the stored value or does it only round while rendering?

@rschamp rschamp merged commit a1249bd into scratchfoundation:develop Aug 17, 2018
rschamp added a commit to scratchfoundation/scratch-gui that referenced this pull request Aug 20, 2018
Roll back VM to before scratchfoundation/scratch-vm#1453. This broke some pen drawing. Resolves #2911.
rschamp added a commit to scratchfoundation/scratch-gui that referenced this pull request Aug 20, 2018
Roll back VM to before scratchfoundation/scratch-vm#1453. This broke some pen drawing. Resolves #2911.
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

Comments