Sprite get_rect() does not return the correct Rect2 position

Godot version

3.5 stable, 4.0 Alpha 15

System information

Win

Issue description

In both Godot 3.5 stable and Godot 4 Alpha 15 (and earlier) if you try to get the Rect2 of a Sprite using get_rect(), the resulting Rect2() will have the correct size, but not the correct position.

Instead, the Rect2 position will be the Sprites offset - halve of it's size (if centered property is enabled). If centered property is not enabled, the returned Rect2 position will be exactly the Sprite offset. Which by default is Vector2(0,0).

I would expect the Rect2 position to be the Sprite position+offset-centered(halve the size).

Steps to reproduce

Add a Sprite and give it a texture
position it somewhere in the scene
execute print( $Sprite.get_rect() )

Minimal reproduction project

You are faster testing this without having to install a minimal project.

What does get_aabb() do in 3D (for comparison)? I don't think global coordinates should always be returned. If you need global coordinates, you can add position yourself (or better, global_position).

I would expect the Rect2 position to be the Sprite position+offset-centered(halve the size).

Then the result would make no sense if transform would be anything more than just a translation (e.g. if it would involve non-default rotation or scale).

Returning the local rect is intended, the sprite's rect is guaranteed to be locally axis aligned and thus the local rect is always properly representable by a Rect2.

Docs already state the rect returned from get_rect() is in local coordinates: https://github.com/godotengine/godot/blob/8fa9d1ae467632daa5f12b278eb616fd580a94ec/doc/classes/Sprite2D.xml#L16

To get the sprite's bounding rect in the parent coordinate space you can do:

(3.x) sprite.transform.xform(sprite.get_rect()).
(4.x) sprite.transform * sprite.get_rect().

[!NOTE]
Edit: My examples in the comments down below (from 2022) were written in 3.5, hence they use some_transform2D.xform(some_rect2) instead of some_transform2D * some_rect2 which is 4.x way of doing the same.

But again: it will be axis aligned in the parent coordinate space so it's not guaranteed to match the actual rect of the sprite. Simple example:

Godot_v3 5-stable_win64_ynJ2ofCqFZ

Rect in the local space (always matches the actual sprite rect):
Rect in the parent space (a bounding rect of the actual rect, only sometimes matches the actual rect):

I would expect the Rect2 that get_rect() returns to be indeed to be a local position (relative to the parent). But I would expect that to look like:

not like:

Meaning if I set the Sprite position to $Sprite.position = Vector2(100,0), I would expect the $Sprite.get_rect() to also have position Vector2(100,0) - Sprite.size/2 if centered is enabled ... regardless of the global_position of the Sprite.

If the Sprite is rotated I would also expect this to change the size and position of the Rect2. Again: all in local space, so relative to the parent.

Example

Create a Rect2 around these Sprites:

I expected this to be as simple as getting the the Sprite Rect2s and merging them together. Like so:

var new_rect
for i in $sprites.get_children():
    if new_rect == null:
        new_rect = i.get_rect()
    else:
        new_rect = new_rect.merge( i.get_rect())

I fail to see where the benefit would be of having this or similar typical operations to be any more complicated.

The bounding rect in the parent space is not exact. It has already some potentially lost information. So if get_rect() would work as you're suggesting (it would return the parent space bounding rect) then there would be no way to obtain the proper local rect from it. Meaning the local rect itself needs to be obtainable.

Example: if get_rect() would work as you're suggesting then sprite.transform.affine_inverse().xform(sprite.get_rect()) would result in the violet rect below: Godot_v3 5-stable_win64_bfM2QJcXVd LPpW1gmOFF

and there'd be no way to obtain the actual local rect from the parent bounding rect as there are infinitely many rects that have such bounding rect, e.g.: L93wFuncPQ iM6MZiIhls

Again, what you want which is obtaining parent space bounding rect is already easily achievable (sprite.transform.xform(sprite.get_rect())) so I don't think a dedicated method for it is needed. Also then the question would be why a dedicated method for parent space and not global space etc. You can handle all these by applying a proper transform to the local rect. If you think additional methods are needed (and some renaming?) you can open a proposal.

Example

Create a Rect2 around these Sprites:

I expected this to be as simple as getting the the Sprite Rect2s and merging them together. Like so:

What seems simple for your specific use case is not necessarily a correct way of doing things in a general case. Also I'd expect methods do work according to what the docs say, not according to my own expectations. I get there are bugs / incorrect docs but here things do work as intended.

var new_rect
for i in $sprites.get_children():
  if new_rect == null:
      new_rect = i.get_rect()
  else:
      new_rect = new_rect.merge( i.get_rect())
I fail to see where the benefit would be of having this or similar typical operations to be any more complicated.

I don't think it's much more complicated:

 var new_rect
 for sprite in $sprites.get_children():
    var bounding_rect = sprite.transform.xform(sprite.get_rect())
    if new_rect == null:
        new_rect = bounding_rect
    else:
        new_rect = new_rect.merge(bounding_rect)

(BTW GitHub supports GDScript syntax highlighting so you can use ```gdscript over ```swift)

@kleonc Thanks for the in depth reply, I appreciate it!

Also I'd expect methods do work according to what the docs say, not according to my own expectations.

My expectations are according to the docs which state that Rect2 is the AABB 2D counterpart. Both Rec2 and AABB purpose is to get the bounding box of an object. It's literally spelled out in the first line of the description:

2D axis-aligned bounding box.

As such both Rect2 and AABB have a position and a size, but they do not have rotation.

So the usecases you are describing are not usecases for Rect2 or AABB. This is not their purpose. When you use Rect2 or AABB, you always want

This is what a axis aligned bounding box is. and not:

This is not what a axis aligned bounding box is.

Again, what you want which is obtaining parent space bounding rect is already easily achievable sprite.transform.xform(sprite.get_rect())

Matrix transformations might come easy to you, but they are definitely not easy or intuitive for everyone. I am trying to understand and use them since years and still can't use them because I fail to have any intuitive understanding of them.

(BTW GitHub supports GDScript syntax highlighting so you can use gdscript overswift)

Thanks! That's good to know, will use gdscript highlighting from now on :)

and not:

This is not what a axis aligned bounding box is.

It is axis aligned, in the sprite's local coordinate system (you can see the axes it's aligned to). And it is a bounding box/rect of the object (the most precise bounding box as it is in the object's own coordinate space; thus it needs to be obtainable).

This is what a axis aligned bounding box is.

Indeed, it is axis aligned. But if the parent coordinate space would get e.g. rotated (and thus it would end up visually not axis aligned with the screen) then it would still be axis aligned in this specific coordinate space. obraz The same rect as above, still axis aligned (just looked from a different perspective).

Matrix transformations might come easy to you, but they are definitely not easy or intuitive for everyone. I am trying to understand and use them since years and still can't use them because I fail to have any intuitive understanding of them.

That sounds like a separate problem (in the engine if it's non-intuitive transformations API or something like that). Regarding what's reported here I'll just quote myself:

(...) the local rect itself needs to be obtainable. (...) If you think additional methods are needed (and some renaming?) you can open a proposal.

"local" to me means object to parent origin relationship. At least this is how it seems like it is handled in the rest of the engine. "global" to me means object to global origin relationship.

I don't think there needs to be separate proposals for what needs to be done here. I think this method is not returning the correct values according to what it's purpose is supposed to be. What is returned is not useful. What you seem to want to be returned is not what useful for an axis aligned bounding box.

If what you call "local rect" needs to be obtainable imho this needs to be obtained with a different method. In fact you should be already able to get that from the Image class. I don't see a reason to have both the Image class and get_rect() return the texture size of a sprite but have no method to get what would be actually useful for the purpose of axis aligned bounding boxes --> the main purpose for the Rect2 class.

"local" to me means object to parent origin relationship. At least this is how it seems like it is handled in the rest of the engine.

Counterexamples: CanvasItem.get_local_mouse_position, Node2D.to_local, Line2D.points and so on (literally anything in the local coordinate space of the object). Can you list where in the engine local is referred to according to your understanding?

I don't think there needs to be separate proposals for what needs to be done here. I think this method is not returning the correct values according to what it's purpose is supposed to be. What is returned is not useful. What you seem to want to be returned is not what useful for an axis aligned bounding box.

Again: it's already returning an axis aligned bounding box, as I've already explained. It does exactly what it's supposed to do. And of course it can be useful, depending on the use case (to me you seem to be fixed up only to your use case). For example to check whether the mouse is within the sprite's bounds (no matter how the sprite itself is transformed!). Also what you're suggesting for it to return doesn't seem to be useful for the use case you've mentioned. Simple example (if it would return the parent space bounding box): XjAM1QkV1o These rects aren't useful for obtaining the bounding rect in the Node2D's space.

And to answer your potential suggestion for it to return "global" bounding rect instead: it again would fail depending on the use case. Simple example, the same World2D shown from two different perspectives: q9n1uxJc8o 11TKgfSEOZ (Icon is rotating and in the left Viewport a Camera2D is added to it as a child (and camera has rotating enabled))

What you seem to do not understand is that axis aligned bounding box is aligned to, well, some axes of some specific coordinate space/system. And how this coordinate space end up being shown/visualized is a completely separated thing. Which in this specific case means that sprite can't know how it ends up being shown to you, it can't know beforehand where the screen axes will be (and thus it can not be aligned to these).

If what you call "local rect" needs to be obtainable imho this needs to be obtained with a different method.

No, that's basically what get_rect method is for.

In fact you should be already able to get that from the Image class. I don't see a reason to have both the Image class and get_rect() return the texture size of a sprite

Image is completely unrelated here, not sure why would you come up with it. If anything, I guess you're referring to Texture. But again, it's unrelated. Sprite.get_rect takes into account all sprite's internals like offset, centering, region rect, etc. All it's doing is returning the actual sprite's rect, as the method's name suggests (and as already thoroughly explained by me, sprite's exact rect is properly representable by Rect2 only in the sprite's local space (yes, because Rect2 is axis aligned)).

but have no method to get what would be actually useful for the purpose of axis aligned bounding boxes --> the main purpose for the Rect2 class.

Again, you seem to want a new method (but I'm not sure how you'd expect it to work) and it's up to a proposal. And again, get_rect() does return an axis aligned bounding box (whether you've already understood it or not). And such bounding box can be used to calculate axis aligned bounding boxes in different coordinate spaces (so it is useful even for your use case).

Anyway, feels like I keep repeating myself and you keep ignoring the explanations / keep being fixed up to what you're suggesting (although I'm not sure if you yourself are sure what exactly you're suggesting). So at this point I kinda see no point in trying to explain things any further.

The problem is that people don't seem to specify whether the aabb they mean is in global or local. Might be worth it to have two functions then?

@kleonc Thank you again for this in depth reply with all those helpful visual examples, I do really appreciate it because you definitely have a far better understanding on the sourrounding math and inner Godot working than I do. But I also feel like I was not able to convey my point to you yet, so I keep trying to explain my thoughts if you don't mind, but I will try to use more precise language:

Counterexamples: CanvasItem.get_local_mouse_position, Node2D.to_local, Line2D.points and so on (literally anything in the local coordinate space of the object). Can you list where in the engine local is referred to according to your understanding?

I don't think these are counter examples. They are all in useful local coordinate spaces.

When I try to get the local mouse position CanvasItem.get_local_mouse_position will return a vector from that canvas item to the mouse position. This makes sense because it is in the local space from the CanvasItem and is totally useful!

Likewise when I want to get the vector from any Node2D to any global_position, Node2D.to_local will do just that. Again makes sense and is useful because it is in the local space of the Node2D.

However $Sprite.get_rect() does imho not follow the same paradigm. It does not return anything equally useful. Neither does it return Rect2 with a size and position relative to the parent (local space of the parent), which would be useful, nor does it return the Rect2 size and position relative to the global origin (global space), which arguably might be even more useful.

What $Sprite.get_rect() currently returns is only useful if you then use complicated chain of matrix transforms. The Rect2 itself does not contain a rotation. You have to get that info from somewhere else. I mean of course it does not have rotation, it's not supposed to! Rect2 is meant to be performant and fast, it's meant to be used with other Rect2s with the same axis alignment!

$Sprite.get_rect().size is always the same as Image.get_size(). Don't you see how the pointless this is? I currently can't easily use get_rect() for what Rect2s are typically designed to do, but what it currently does, is already covered in a different much more suitable method.

Again: it's already returning an axis aligned bounding box

Well yes, but the axis it is aligned to is totally useless for the most common purpose of a Rect2.

And to answer your potential suggestion for it to return "global" bounding rect instead: it again would fail depending on the use case. Simple example, the same World2D shown from two different perspectives:

Assuming the green/red position marker represents the Viewport origin: This does not look like it's failing to me. This looks exactly like I would a $Sprite.get_global_rect() to work. Actually $Sprite.get_rect() too, since the Sprite is the direct child of the Viewport.

But maybe this is just me having a wrong idea about things. I have not yet fully understood everything about the Transform conversions between various viewports. Likely because this often ends up in complicated matrix math.

No, that's basically what get_rect method is for.

I disagree. I think what $Sprite.get_rect() currently returns is hardly ever useful. If you and other think it is useful for some occasions, I think it should be it's own method and called something along the lines of $Sprite.get_offset_rect() or maybe better $Sprite.get_internal_rect(). At least then the Godot user would know by looking at the method name the Rect2 they can expect from this method is relative from the Sprite origin to the texture rect position. And it would make sense to me

Again, you seem to want a new method (but I'm not sure how you'd expect it to work) and it's up to a proposal.

No not really. I want this method with everything it does, but I want the Rect2 it returns to be aligned to a different axis (either sprite parent or root viewport) because I think it would be a million times more useful in everyday use.

Anyway, feels like I keep repeating myself and you keep ignoring the explanations / keep being fixed up to what you're suggesting (although I'm not sure if you yourself are sure what exactly you're suggesting). So at this point I kinda see no point in trying to explain things any further.

Thank you for taking the time to explain everything in such detail. I do think I learned a lot from it, so your effort definitely was not in vain. It's possible my arguments and points come from a lack of mathematical understanding or the inner workings of game engines, but the way I see it, taking the point of a user-usecase perspective who is totally oblivious to the inner working of the engine is a valid position to argument. Users want to get things done. The engine should make common usecases easy and straight forward regardless of the users understanding. Common usecases should not require engine level understanding imho.

Thanks for reading.

The problem is that people don't seem to specify whether the aabb they mean is in global or local. Might be worth it to have two functions then?

@Zireael07 I think the main thing that needs clarifying here is that sprite itself is a rectangle. Forget coordinate spaces for a while, sprite itself is a rectangle. And in the Sprite's (node) own coordinate space the actual sprite rect is always axis aligned. Meaning the local rect is the exact rect of the sprite. Of course aabb of an already axis aligned rect is that rect itself. Meaning the local rect is at the same time: the exact rect of the sprite, and the aabb of the sprite (in the local space). And it's the exact rect of a sprite that needs to be obtainable (so you can do whatever you want depending on the use case). The fact that it's the local aabb at the same time is kinda just a bonus, I'd say it's not the reason why this method is provided.

As I've already said: >If you think additional methods are needed (and some renaming?) you can open a proposal.

I don't have anything against get_rect -> get_local_rect rename, it should make things clearer. And if there would be a demand for something like get_global_rect method (just a shorthand for global_transform.xform(get_local_rect())) then I wouldn't really mind. But be aware that if someone doesn't understand transforms etc. then it could still lead to confusion, e.g. one could think that these would be equivalent:

get_local_rect().has_point(get_local_mouse_position())
get_global_rect().has_point(get_global_mouse_position())

but these aren't the same. The first would check whether mouse is actually over the sprite's rect and the second would check whether it's over the global aabb of the sprite (so not necessarily over the sprite's rect). Hence in case such method is wanted I'd rather suggest naming it something like get_global_bounding_rect to avoid potential confusion. But that's just my opinion, feel free to open any proposals you like.

I don't think these are counter examples.

@golddotasksquestions These are counter examples to your own claim that in the engine local is used according to your understanding. Your own words: >"local" to me means object to parent origin relationship. At least this is how it seems like it is handled in the rest of the engine.

These examples just show that your claim is not true. Also you seem to just skipped over my question: >Can you list where in the engine local is referred to according to your understanding?

They are all in useful local coordinate spaces. (...) However $Sprite.get_rect() does imho not follow the same paradigm. It does not return anything equally useful.

And so get_rect is, it's in the exact same space as get_local_mouse_position. Hence you can use them together. But for some reason you keep claiming it's not useful because it's not useful for you. And more importantly local rect is not achievable from parent/global rect (yes, I've already mentioned it) so it needs to be provided by the engine. Unless you're suggesting to remove this method and make users need to look into the source code and replicate the logic in case they do need the local rect.

What $Sprite.get_rect() currently returns is only useful if you then use complicated chain of matrix transforms. (...)

Counter example: get_rect().has_point(get_local_mouse_position()). No matrix transformations whatsoever.

$Sprite.get_rect().size is always the same as Image.get_size(). (...)

That's not true. You keep making me doubt whether you've read what I've already written. But if you need visualizations instead of a text, here you go (the local rect is shown): Kj4ge2JyNR (and yes, I needed to use get_rect to be able to draw the sprite's border, I wonder if that's a valid use case for you?:thinking:)

Again: it's already returning an axis aligned bounding box

Well yes, but the axis it is aligned to is totally useless for the most common purpose of a Rect2.

Refer to my answer to @Zireael07. What's important is providing the exact rect of the sprite, not the sprite's local aabb.

Assuming the green/red position marker represents the Viewport origin: This does not look like it's failing to me. This looks exactly like I would a $Sprite.get_global_rect() to work.

No, it's still the version showing the parent space aabb. So it's showing the axes of the SpriteRectTest node's coord space. Viewport space's would be like this (added manually so not precise): vVVZarm

Actually $Sprite.get_rect() too, since the Sprite is the direct child of the Viewport.

It's not, you can see the scene tree hierarchy.

I think what $Sprite.get_rect() currently returns is hardly ever useful.

Again, not useful to you doesn't mean not useful. YLPq1HJnGr

If you and other think it is useful for some occasions, I think it should be it's own method and called something along the lines of $Sprite.get_offset_rect() or maybe better $Sprite.get_internal_rect(). At least then the Godot user would know by looking at the method name the Rect2 they can expect from this method is relative from the Sprite origin to the texture rect position. And it would make sense to me

Such namings would be even more confusing. It's the actual/exact rect of the sprite, so just get_rect makes sense. But as I've already mentioned changing it to get_local_rect would probably make things clearer.

Again, you seem to want a new method (but I'm not sure how you'd expect it to work) and it's up to a proposal.

No not really. I want this method with everything it does, but I want the Rect2 it returns to be aligned to a different axis (either sprite parent or root viewport) because I think it would be a million times more useful in everyday use.

Again, get_rect works correctly. Whether you want renaming / additional method is up to a proposal. Renaming of course can't be done in 3.x and if you want to get it into 4.0 then I suggest making it fast as beta is close and post beta it will be most likely too late for such breaking change.

Thank you for taking the time to explain everything in such detail. I do think I learned a lot from it, so your effort definitely was not in vain. It's possible my arguments and points come from a lack of mathematical understanding or the inner workings of game engines, but the way I see it, taking the point of a user-usecase perspective who is totally oblivious to the inner working of the engine is a valid position to argument. Users want to get things done. The engine should make common usecases easy and straight forward regardless of the users understanding. Common usecases should not require engine level understanding imho.

Sure thing a different point of view is nice. But if a user comes and tells something like: _"Hey, for my use case (which I think is the most common use case!) I need X value to have only two decimal places. So I think get_X method should return it like that, rounded to just two decimal places."_ then it doesn't mean the user's suggestion is what should be done. Of course that example is super dumb and intentionally exaggerated by me. But if the exact value of X is providable then the user should be able to obtain it (and do whatever else with it). And I keep telling you that get_rect does exactly what it's supposed to (it returns the exact sprite's rect). I don't see this method being removed, it's useful/needed. What you want is (besides potential get_rect renaming) an additional utility method, please stop denying it. :slightly_smiling_face:

Thanks for reading.

Also thanks! But I'm really done in here. I doubt I can clarify things any further, I keep repeating myself on and on. So in case you still don't understand something here: thanks for re-reading what I've already written. :wink:

Actually

sprite.transform.xform(sprite.get_rect())

does not work.

Did you mean

sprite.transform * sprite.get_rect()

I was also surprised by get_rect() returning the sprite internal rect. I guess it makes sense, because there's no get_size and position points to some arbitrary anchor point. Not necessarily to one of the vertices.

I came here to find a solution but only found a wrong answer. Would you mind updating your post with the correct information? (And the 6 repetitions thereof?)

Actually
sprite.transform.xform(sprite.get_rect())
does not work.

Did you mean
sprite.transform * sprite.get_rect()
?

@Micha0 Oh, see the dates of the comments, it was reported for both 3.5 stable and 4.0 Alpha 15. If I'm using Transform2D.xform in the examples then they're were written in 3.5. See 3.5 docs for Transform2D.xform. So seems I did meant to use what I've used, that's not a mistake / wrong answer.

But indeed in 4.0+ Transform2D.xform is replaced by relevant Transform2D.operator *(OtherMultiplicableType) multiplication operators. E.g. Transform2D.operator *(Rect2). So you're correct that sprite.transform * sprite.get_rect() is the way to do the same in 4.0+, yes. :slightly_smiling_face:

I came here to find a solution but only found a wrong answer. Would you mind updating your post with the correct information? (And the 6 repetitions thereof?)

I've added a note for clarification.

godotengine / godot