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

Effective bounds of zero-width or zero-height skin/costume are incorrect #570

Open
cwillisf opened this issue Mar 19, 2020 · 2 comments
Open

Comments

@cwillisf
Copy link
Contributor

Expected Behavior

A Skin created for a costume should have the same bounds as that costume.

Actual Behavior

A Skin created for a Wx0 or 0xH (for any value of W or H) goes through the setEmptyImageData path which always leads to a 1x1 image (which is fine) and matching bounds (arguably not fine). The difference can be detected through fencing behavior and probably a few other ways.

Steps to Reproduce

These steps will work until we fix #434

  1. Create or edit a project which uses the pen extension
  2. Create or edit a costume with non-zero width and height. The default cat costume is fine.
  3. Add a perfectly horizontal or vertical line
  4. Delete everything from the costume EXCEPT for the perfectly horizontal or vertical line.
  5. Adjust the rotation center so that it is outside the model's bounds.
  6. Use pen down and forever { go to random position } to see the fencing behavior.

Example (thanks @fsih!):

@fsih
Copy link
Contributor

fsih commented Mar 19, 2020

Perhaps costumes should never have 0 width or height

@adroitwhiz
Copy link
Contributor

This led me down a bit of an investigation, but the TL;DR is that I have a patch that won't work until scratchfoundation/scratch-vm#2314 is merged (and preferably, setRotationCenter is removed).

  • Empty vector costumes are given a rotation center of [240, 180] (both in 2.0 and in 3.0 due to Empty vector costumes' rotation centers are incorrect scratch-paint#983), so always setting skins' rotation centers to the explicitly-passed ones won't work for 0x0 costumes.
  • This is handled at fence-time in 2.0, but doing something similar here would be kind of janky in my opinion (probably requiring some rotation-center/rectangle coordinate conversion, and possibly creating GC pressure by creating a new Rectangle every time a zero-size sprite is moved).
  • This can be fixed via an explicit check in setSVG, but this is broken because the VM sets the skin's rotation center every time its costume is switched to. That appears to be why Scratch 2.0 compat: don't modify rotation center of empty sprites #156 was merged.
  • Once the API for setting a drawable's rotation center is removed (drawables themselves haven't had rotation centers since late 2016), the setSVG check will work.

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

4 participants