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

Conversation

st-pasha
Copy link
Contributor

Description

Set size property for the SpriteWidget

Checklist

  • The title of my PR starts with a Conventional Commit prefix (fix:, feat:, docs: etc).
  • I have read the Contributor Guide and followed the process outlined for submitting PRs.
  • 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.

Breaking Change

  • [-] Yes, this is a breaking change.
    • (unless user code was relying on this size being zero)
  • No, this is not a breaking change.

Related Issues

Closes #1575

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?

@erickzanardo erickzanardo enabled auto-merge (squash) June 29, 2022 17:10
…flame-engine#1761)

This PR changes the signature of the closeToVector() helper function:

    Old: closeToVector(num x, num y, {double epsilon = 1e-15})
    New: closeToVector(Vector2 vector, [double epsilon = 1e-15])
/// ```
Matcher closeToVector(num x, num y, {double epsilon = 1e-15}) {
return _IsCloseTo(Vector2(x.toDouble(), y.toDouble()), epsilon);
Matcher closeToVector(Vector2 vector, [double epsilon = 1e-15]) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This should not be part of this PR! It is a breaking change of non-related feature.

Copy link
Member

Choose a reason for hiding this comment

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

I think that GitHub is confused, this was already merged in #1761

Copy link
Member

Choose a reason for hiding this comment

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

Now the PR looks like it should again.

@erickzanardo erickzanardo merged commit 82f75fc into flame-engine:main Jun 29, 2022
@st-pasha st-pasha deleted the ps.sprite-widget-size branch June 30, 2022 03:43
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SpriteWidget gets zero height
4 participants