Rework of edges

[salam99823]

Changes:

Added:

• feature glam-latest for those who use glam.
• public function new to take Edges from any data.
• private BinImage structure to represent an image.

Moved:

• structure Edges moved to lib.rs.
• tests moved to separate file.
• distance function to utils.

Moved to BinImage implementation

• translate_vec renamed to translate.
• xy_translate renamed to translate_point.
• get_at renamed to get.

Removed:

• hashbrown, mashmap and thiserror from dependencies.
• march_edges function.
• unused enum Error and module error.
• translate_vec, march_edges functions from public API.
• EdgesDisplay structure.

Other

• dependency on bevy replaced with bevy_mathand bevy_render.

[shnewto]

Hi @salam99823 thanks for the PR! I'll be able to give it a proper look weekend.

[salam99823]

Hi @shnewto, I'm currently thinking about how to solve this problem. I think can add an areas argument and make a convention that each area can only have one object, then can take the farthest points and the closest (where the distance between them is 1) points to each other. Then the points can be sorted in drawing order. Or can write function for find these areas.

[salam99823]

I also think that the Edges struct could be removed or deprecated, and its methods moved outside and BinImage made public. Like this

/// If there's only one sprite / object in the image, this returns just one, with
/// coordinates translated to either side of (0, 0)
#[must_use]
pub fn single_image_edge_translated<T: Into<BinImage>>(image: T) -> Vec<Vec2> {
    image_edges(image, true).into_iter().flatten().collect()
}

// ...

/// Takes `Edges` and a boolean to indicate whether to translate
/// the points you get back to either side of (0, 0) instead of everything in positive x and y.
#[must_use]
pub fn image_edges<T: Into<BinImage>>(image: T, translate: bool) -> Vec<Vec<Vec2>> {
    let image: BinImage = image.into();
    // Marching squares adjacent, walks all the pixels in the provided data and keeps track of
    // any that have at least one transparent / zero value neighbor then, while sorting into drawing
    // order, groups them into sets of connected pixels
    let edge_points = (0..image.height() * image.width())
        .map(|i| (i / image.height(), i % image.height()))
        .map(|(x, y)| UVec2::new(x, y))
        .filter(|p| image.get(*p))
        .filter(|p| (0..8).contains(&image.get_neighbors(*p).iter().filter(|i| **i).count()))
        .collect();

    points_to_drawing_order(edge_points)
        .into_iter()
        .map(|group| {
            let group = group.into_iter().map(|p| p.as_vec2()).collect();
            if translate {
                image.translate(group)
            } else {
                group
            }
        })
        .collect()
}

#[must_use]
pub fn translate(image: impl Image, v: Vec<Vec2>) -> Vec<Vec2> {
    image.translate(v)
}

impl Image for BinImage {
    fn width(&self) -> u32 {
        self.width()
    }
    fn height(&self) -> u32 {
        self.height()
    }
}

pub trait Image {
    fn width(&self) -> u32;
    fn height(&self) -> u32;

    fn translate_point(&self, p: Vec2) -> Vec2 {
        Vec2::new(
            p.x - (self.width() as f32 / 2.0 - 1.0),
            (self.height() as f32 / 2.0 - 1.0) - p.y,
        )
    }

    fn translate(&self, v: Vec<Vec2>) -> Vec<Vec2> {
        v.into_iter().map(|p| self.translate_point(p)).collect()
    }
}

[shnewto]

@salam99823 I've been thinking some about that issue as well, I've been wondering whether we could get a little stricter about what constitutes a connected edge for drawing order by checking whether the on/off coordinates of a neighboring pixel continue the current pixel's edge in the correct coordinates. something like:

• if the current pixel has a single empty neighbor to the north
• the next edge coordinate must be a neighbor to the north west or the north east
  or
• if the current pixel has empty neighbors to the north west, north, and north east
• the next edge coordinate must be a neighbor to the west or the east

I think that approach might actually reduce the complexity of the current approach that's doing some acrobatics to get the edges it's able to currently.

[shnewto]

wrt whether to deprecate the Edges struct I see what you're getting at but I sort of think what you've called BinImage could just become the Edges object itself. Conceptually I understand why you've chosen BinImage and am open to the name if you feel very strongly, but in my head, the BinImage is internal once the user has called new. After that, they're using an object for properties of edges of things inside an image rather than the properties of an full image itself.

[salam99823]

Deprecating the Edges structure is an optional proposal. I don't mind leaving BinImage internal.

[salam99823]

Interesting solution, I may try to do this.

[shnewto]

Hey @salam99823 how are you feeling about the changes so far? i.e ready for earnest review or still working through some things? I'm in no rush, just don't want to let anything sit if it doesn't need to. Very interested in your revisions to edge collecting!

[salam99823]

I consider the work finished.

[shnewto]

This is a really fantastic contribution @salam99823. I really appreciate all the work you put into this crate and the quality of the code you committed. Thank you!!

[salam99823]

I am very glad that I was able to contribute to this crate. Now i work on bevy_collider_gen.