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

Easier way to obtain object bounds #301

Open
aleokdev opened this issue Jun 19, 2024 · 5 comments
Open

Easier way to obtain object bounds #301

aleokdev opened this issue Jun 19, 2024 · 5 comments

Comments

@aleokdev
Copy link
Contributor

While refactoring my sokoban game to use this crate instead of my old fork, i realized a friction point that could be worked on.
Previously, to obtain an object's width or height, we could do the following:

object.width
object.height

However, since these members referred to deprecated members within the <object> tag, they were later removed. This means that to obtain the width/height of an object now, we need to match on the object's shape, check that it is an ellipse or rect, and obtain the width and height from there, something akin to:

let (width, height) = match object.shape {
    tiled::ObjectShape::Rect { width, height } | tiled::ObjectShape::Ellipse { width, height } => (width, height),
    _ => panic!(),
};

Doing this every time the user wants to obtain width/height of an object is a bit painful (and in this case also panics if the shape is anything but a rect or ellipse), so I propose a object.global_bounds() function of sorts that returns a rect including the position of the object as well as its actual width/height. I mean actual because previously width/height was not set for polygonal objects (since they are exclusively rect/ellipse members), but it could be useful to calculate them from the min/max point positions of the polygon.

@bjorn
Copy link
Member

bjorn commented Jun 20, 2024

It sounds definitely useful, but should these bounds also take into account object alignment and rotation then?

And I think bounding_rect might be a more fitting name, although the QGraphicsItem::boundingRect function does not include the item's position and rotation.

@aleokdev
Copy link
Contributor Author

OK, we could add a local_bounding_rect function that doesn't apply transformations for now, and then we could figure out rotations/position/etc on a global_bounding_rect function. Does that sound good?

@bjorn
Copy link
Member

bjorn commented Jun 21, 2024

I'm not sure about the verbosity of these names. Wouldn't a bounding_rect that just returns the local rectangle be fine, and we could call the global one aabb? But if you prefer the explicit local/global I'm fine with that too.

Btw, you initially proposed only a global_bounds, so I asked whether that should then also take into account alignment and rotation because I thought those things should probably go together. I didn't mean to propose we also add a version returning a local rectangle. Maybe we should wait with adding that until there's an actual need.

@aleokdev
Copy link
Contributor Author

Btw, you initially proposed only a global_bounds, so I asked whether that should then also take into account alignment and rotation because I thought those things should probably go together. I didn't mean to propose we also add a version returning a local rectangle. Maybe we should wait with adding that until there's an actual need.

I didn't think of the implications of having global in the name. While it is true that global bounds should include rotation and alignment, it would complicate the implementation. There are times where rotation isn't applied to an object and it is useful to know its bounds, for instance when you want to obtain the width/height of a rect object quickly without having to match like in the example above, so I think having local in the name is best to differentiate between a possible global implementation later down the line.

@bjorn
Copy link
Member

bjorn commented Jun 21, 2024

so I think having local in the name is best to differentiate between a possible global implementation later down the line.

Alright. What about alignment, though? I guess it is relevant even for a local bounding rectangle. For a rectangle with top-left alignment, the local bounding rect would have its top-left at (0,0), but for bottom-left alignment, that rectangle would need to have its top-left at (0,-height), right?

Of course the problem with alignment is that it depends not only on the type of object, but also on the tileset object alignment property as well as on the map orientation.

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

2 participants