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

Expand convex hull bounds by half a pixel per side #529

Conversation

adroitwhiz
Copy link
Contributor

@adroitwhiz adroitwhiz commented Nov 28, 2019

Resolves

Resolves #528

Proposed Changes

This PR expands convex hull bounds by the radius of half a pixel on each side of the bounding box.

Reason for Changes

Say you have a costume like this:
image

The pixels of said costume can be treated as square color samples taken at the pixels' centers:
image

The convex hull algorithm calculates the hull based on the pixels' centers, not taking area into account:
convhull

As such, bounds calculated from the convex hull need to be enlarged by the "radius" of each pixel.
If pixels were circular, this would be half the "diameter" (aka the drawable's Scratch-space scale), but since they're square, it's actually (√2 / 2) * the diameter.

This will probably change:

If the "if on edge, bounce" behavioral change is a compatibility issue, I can redo this once there's architecture in place for separating a sprite's "logical" bounds and texture bounds, so the old tight bounds can be used for bouncing and the new expanded ones can be used for stamping.

EDIT: The current "if on edge, bounce" behavior is extremely inconsistent both with itself and 2.0 (you can get this sprite to begin phasing through the walls if you move it into a corner) so this hopefully shouldn't affect compatibility any more than the current behavior already has.

A semi-related issue you may run into when investigating this further is that getConvexHullPointsForDrawable does touching tests at the sprite's "logical size", so double-resolution bitmap costumes (in 3.0, that means all of them) are sampled at half their actual resolution.

This means that, incidentally, double-resolution bitmap sprites with odd-numbered resolution dimensions (like the black circle I linked above in 349149479) appear to be properly fenced currently, but will be fenced half a texel too far inwards by this change. This is because their "logical" sizes are their resolutions divided by 2, and we've started rounding that size up. However, bitmap sprites with even-numbered resolution dimensions were fenced too far outwards, and this change fixes that.

// The bounds must be extended by the 'radius' of each sample on every side.
// Since nearest-neighbor sampling gives pixels a square area, the radius is sqrt(2) / 2 = 0.707...
// at its longest (from the center of the pixel to one of its corners).
const halfPixel = 0.7071067811865476 * scale;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
const halfPixel = 0.7071067811865476 * scale;
const halfPixel = Math.SQRT1_2 * scale;

@adroitwhiz
Copy link
Contributor Author

Waiting for all the other convex hull issues (#479 and #592) to be resolved before redoing this

@adroitwhiz
Copy link
Contributor Author

Closing in favor of #724

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.

1x1 sprites will not stamp after a "say" block is executed
2 participants