Skip to content

fix: Use all four corners in PositionComponent.toAbsoluteRect - #4109

Merged
ufrshubham merged 5 commits into
mainfrom
devkage/fix-to-absolute-rect-rotated-ancestor
Oct 13, 2026
Merged

ufrshubham merged 5 commits into
mainfrom
devkage/fix-to-absolute-rect-rotated-ancestor

Conversation

@ufrshubham

@ufrshubham ufrshubham commented Oct 10, 2026 •

Copy link
Copy Markdown
Member

Description

PositionComponent.toAbsoluteRect() has a shortcut for components whose own angle is 0: it only
projects the top left and the bottom right corner. That is not enough when an ancestor is rotated by
an angle that is not a multiple of a quarter turn (like 45°), because the other two corners can then
stick out of the rectangle. The result was a rectangle that is too small, and
CameraComponent.canSee(), which uses it, could return false for a component that is partly
visible.

toAbsoluteRect() now always uses all four corners. toRect() keeps the shortcut, because it is in
the coordinate space of the parent, where only the rotation of the component itself matters.

Tests are added for toAbsoluteRect() with a rotated ancestor, for canSee() with a rotated
ancestor, and a test that toRect() ignores the rotation of the parent.

This was found in a review comment on #4107:
#4107 (comment)

Before the fix:

to_absolute_rect_bug.mp4

After the fix:

to_absolute_rect_fixed.mp4

Checklist

  • I have followed the Contributor Guide when preparing my PR.
  • I have updated/added tests for ALL new/updated/fixed functionality.
  • I have updated/added relevant documentation in docs and added dartdoc comments with ///.
  • [-] I have updated/added relevant examples in examples or docs.

Breaking Change?

  • Yes, this PR is a breaking change.
  • No, this PR is not a breaking change.

Related Issues

Closes #4108

A child without a rotation of its own was measured with only two corners. That is too small when an ancestor is rotated by an angle that is not a multiple of a quarter turn, and it made CameraComponent.canSee return false for components that are partly visible.
Copilot AI balanced review requested due to automatic review settings October 10, 2026 13:41

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

The focused implementation correctly resolves #4108 and includes coverage for the affected behavior.

0 open findings

What changed in this PR

Fixes world-space bounding rectangles for components beneath rotated ancestors, resolving incorrect camera visibility checks.

Changes:

  • Computes absolute bounds from all four corners.
  • Preserves the optimized parent-space calculation.
  • Adds regression tests for bounding and camera visibility.
File Description
packages/​flame/​lib/​src/​components/​position_component.dart Corrects absolute bounding-box calculation.
packages/​flame/​test/​components/​position_component_test.dart Tests rotated-ancestor bounds and parent-space behavior.
packages/​flame/​test/​camera/​camera_component_test.dart Tests visibility beneath a rotated ancestor.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@ufrshubham ufrshubham changed the title fix: Use all four corners in PositionComponent.toAbsoluteRect fix: Use all four corners in PositionComponent.toAbsoluteRect Oct 10, 2026

@luanpotter luanpotter left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

Comment thread packages/flame/lib/src/components/position_component.dart Outdated
Replaces the nested min and max calls in PositionComponent._boundsOfCorners with the existing fromVertices extension, as suggested in the review. It is about 37 ns slower per call in an AOT build (48 ns against 11 ns), which is small next to projecting the four corners.
@ufrshubham
ufrshubham requested a review from luanpotter October 12, 2026 17:00
@ufrshubham
ufrshubham enabled auto-merge (squash) October 13, 2026 19:22
@ufrshubham
ufrshubham merged commit 4e17520 into main Oct 13, 2026
8 checks passed
@ufrshubham
ufrshubham deleted the devkage/fix-to-absolute-rect-rotated-ancestor branch October 13, 2026 19:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

toAbsoluteRect and canSee are wrong for a child of an ancestor that is rotated by a non-right angle

4 participants