feat: Add a method to adapt the camera bounds to the world

[Skyost]

Description

This PR adds a method to the CameraComponent class that allows it to set its bounds according to the current world. In fact, this was the behavior of the old camera object.

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 #2601.

[Skyost]

@spydon I see, but calculating the worldSize may be a bit more difficult if the shape is not a Rectangle, isn't it ? Also, does strictBounds sounds good to you ?

[spydon]

@spydon I see, but calculating the worldSize may be a bit more difficult if the shape is not a Rectangle, isn't it ? Also, does strictBounds sounds good to you ?

Maybe:

/// When [considerViewport] is true none of the viewport can go outside
/// of the bounds, when it is false only the viewfinder anchor is considered.
void setBounds(Shape? bounds. {bool considerViewport = false}}) {

[Skyost]

Okay, I've made the changes you've suggested :

if (considerViewport && bounds != null) {
  final halfViewportSize = viewport.size / 2;
  bounds = Rectangle.fromCenter(
    center: bounds.center,
    size: Vector2(bounds.support(Vector2(1, 0)), bounds.support(Vector2(0, 1))) - halfViewportSize,
  );
}

One drawback is that the bounds are converted to a Rectangle. I don't know if it's possible to just "adapt" the given shape to the viewport as Shape is an abstract class.

[spydon]

One drawback is that the bounds are converted to a Rectangle. I don't know if it's possible to just "adapt" the given shape to the viewport as Shape is an abstract class.

Much better! Except that it doesn't compile. 😅
I think we should check at least support circular viewports and circular bounds in the future too, but maybe that could be a follow-up.

[Skyost]

Yep, it still wip. More commits to follow 🙂

[Skyost]

Alright, so I've added the following method :

Shape? _calculateViewportAwareBounds(Shape originalBounds) {
  final worldSize = Vector2(
    originalBounds.support(Vector2(1, 0)).x,
    originalBounds.support(Vector2(0, 1)).y,
  );
  final halfViewportSize = viewport.size / 2;
  if (originalBounds is Rectangle) {
    return Rectangle.fromCenter(
      center: originalBounds.center,
      size: worldSize - halfViewportSize,
    );
  } else if (originalBounds is RoundedRectangle) {
    final halfSize = (worldSize - halfViewportSize) / 2;
    return RoundedRectangle.fromPoints(
      originalBounds.center - halfSize,
      originalBounds.center + halfSize,
      originalBounds.radius,
    );
  } else if (originalBounds is Circle) {
    return Circle(
      originalBounds.center,
      worldSize.x - max(halfViewportSize.x, halfViewportSize.y),
    );
  }
  return null;
}

Which is being used in setBounds :

if (considerViewport && bounds != null) {
  bounds = _calculateViewportAwareBounds(bounds) ?? bounds;
}

Tell me what you think about it 🙂

[spydon]

Tell me what you think about it 🙂

Looks good! I think we need to take the zoom into consideration too though, because this doesn't work when the zoom level is changed right?

[Skyost]

Let's do that !

[Skyost]

@spydon Just checked the source code and it seems that BoundedPositionBehavior doesn't take any viewfinder.transformation into account. Maybe this belongs to a separate PR ?

[spydon]

@spydon Just checked the source code and it seems that BoundedPositionBehavior doesn't take any viewfinder.transformation into account. Maybe this belongs to a separate PR ?

The BoundedPositionBehavior doesn't need to take any viewfinder transformations into account, since it only checks on a position and a bounds. This PR on the other hand that adds bounds aware of the viewport needs to take into consideration when the CameraComponent.visibleWorldRect changes (i.e. when the viewport size changes after a resize or the zoom level of the viewfinder makes a smaller/bigger part of the world visible), otherwise if you for example first play the game in landscape mode and then flip it to portrait the bounds won't be correctly calculated.

[Skyost]

Alright. Maybe the best approach is to add a new behavior to the viewfinder so that it can update the bounds based on the visible rectangle.

That's what I did in the latest commit. I haven't implemented the "update" part, but can you tell me what you think about it please ?

[spydon]

Alright. Maybe the best approach is to add a new behavior to the viewfinder so that it can update the bounds based on the visible rectangle.

That's what I did in the latest commit. I haven't implemented the "update" part, but can you tell me what you think about it please ?

The idea is good, but the bounds should not have to be recalculated on every tick, but only when the transforms that are affecting it are changing. You can add listeners to those transforms and run the update method when they trigger.

[Skyost]

The idea is good, but the bounds should not have to be recalculated on every tick, but only when the transforms that are affecting it are changing. You can add listeners to those transforms and run the update method when they trigger.

Yes, I was thinking about that. The thing is, from what I know, these transforms don't have any method that allow to register a listener. I'll see what's the best approach.

[spydon]

Yes, I was thinking about that. The thing is, from what I know, these transforms don't have any method that allow to register a listener. I'll see what's the best approach.

The FixedResolutionViewport has it (called transform), we could just put that field directly on the Viewport and override it in the subclasses. And the viewfinder already has it.

[Skyost]

Yes, what I mean is I didn't know that the Transform2D class had a addListener method or something like that. But I just seen that it overrides ChangeNotifier.

[Skyost]

Okay, the problem with listening for Transformation2D updates, it that when there is a FollowBehavior, the following line,

owner.position = delta..add(owner.position);

triggers an update at every tick.

[spydon]

Okay, the problem with listening for Transformation2D updates, it that when there is a FollowBehavior, the following line,

owner.position = delta..add(owner.position);

triggers an update at every tick.

scale is exposed as a NotifyingVector2 inside of Transform2D so that you can listen to only scale changes, are there more things from the transform that you need?

[Skyost]

Oh yes, sure.

[spydon]

Starting to look good!
Just some tests missing, the easiest would be to do golden tests for this I think, you can get some inspiration from here for example:
https://github.com/flame-engine/flame/blob/main/packages/flame/test/camera/viewports/fixed_aspect_ratio_viewport_test.dart

[spydon]

Any updates on this @Skyost? :)

[Skyost]

Hey @spydon,

Sorry, I've never written any golden test in my life and have been unactive for the past two weeks. I'll try to make some progress today 🙂

[Skyost]

I've added a test that checks whether the viewport has been correctly updated. It's not a golden test, but I think it does the trick. Do you think it needs more tests ? If so, what parts of the ViewportAwareBoundsBehavior class need to be tested ?

[spydon]

Lgtm, the test doesn't really ensure things are rendered correctly, but since the other camera behaviour doesn't have a goldens test either I think we can let this pass for now.