Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix: Specify size for the SpriteWidget #1760

Merged
merged 4 commits into from
Jun 29, 2022
Merged

fix: Specify size for the SpriteWidget #1760

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
02a1cc2
fix: Specify size for the SpriteWidget
st-pasha Jun 29, 2022
ff9632c
Merge branch 'main' into ps.sprite-widget-size
erickzanardo Jun 29, 2022
01a88c4
refactor!: Matcher closeToVector() now accepts Vector2 as an argument…
st-pasha Jun 29, 2022
853c479
Merge branch 'main' into ps.sprite-widget-size
spydon Jun 29, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion packages/flame/lib/src/widgets/sprite_widget.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,10 @@ class InternalSpriteWidget extends StatelessWidget {
@override
Widget build(_) {
return Container(
child: CustomPaint(painter: SpritePainter(sprite, anchor, angle: angle)),
child: CustomPaint(
painter: SpritePainter(sprite, anchor, angle: angle),
size: sprite.srcSize.toSize(),
Copy link
Member

Choose a reason for hiding this comment

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

This will prevent the user to render the sprite bigger than its original size right?

I think that a better approach here would be to use the BoxFit enum. Or, maybe something like that. wdyt?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This could be considered a feature request: adding explicit size parameter to SpriteWidget, and then taking it into account within SpriteRenderer.
Current PR only addresses the problem that SpriteWidget is currently rendered at size 0, which is problematic if someone tries to do layout.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, I get that, but this will solve one issue, while adding a new one, where all sprite widgets will always have the size of the image. And it will be impossible to render it bigger than its original size.

So we need an alternative here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So, i'm looking at RenderCustomPaint render-box, and it says that it considers the provided size as the preferred size, meaning it will attempt to find the size that is as close as possible to the given size, while still satisfying the parent's constraints. Meaning that if constrains say that minSize=maxSize=100, then that will be size of the widget regardless of what its size property says.

I've tried this with the SpriteWidgetExample, and it can still scale the sprite both up and down.

Copy link
Member

Choose a reason for hiding this comment

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

So, i'm looking at RenderCustomPaint render-box, and it says that it considers the provided size as the preferred size, meaning it will attempt to find the size that is as close as possible to the given size, while still satisfying the parent's constraints. Meaning that if constrains say that minSize=maxSize=100, then that will be size of the widget regardless of what its size property says.

I've tried this with the SpriteWidgetExample, and it can still scale the sprite both up and down.

Interesting! So my concern will not happen! Thanks for checking on that. LGTM!

Copy link
Member

Choose a reason for hiding this comment

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

Interesting indeed, can someone open an issue for setting custom sizes?

),
);
}
}