[Merged by Bors] - Remove Need for Sprite Size Sync System #2632
Conversation
alice-i-cecile
added
A-Assets
|
This is definitely reasonable, but it creates some new problems: users will almost certainly want to query the current sprite size in userspace. Things like culling, bounding boxes, etc will also need per-sprite sizes. Users can manually read the texture size, but this involves more boilerplate than most people will want. |
|
Ah, I see, that makes sense. 🤔 Maybe this doesn't help much then. I'll think a bit more about the problem to see if I can come up with any other ways to improve the situation. |
|
OK, what do you think of this:
I think that might just make the situation a little more clear. I had always been confused by the resize mode thing until I looked at the implementation. That could easily be solved by documentation, but I think the solution above is more self explanatory. |
|
Alrighty I've thought about this for a bit and I think I'm actually in favor of merging the current implementation. I don't want to store "computed size" on the Sprite itself. It should probably live on something like a BoundingRect component. There is also the issue of rect size needing to be in sync with the transform scale, which complicates things even more. I'd rather just tackle that separately (when it is needed). Short term I think adopting the cheaper / simpler approach is the way to go. Users that need the actual width and height can look up the texture dimensions and scale it using the transform. This approach will be forward-compatible with whatever approach we adopt for bounding rects. |
|
@zicklag can you resolve merge conflicts? I did this locally but I cant push it because you've disallowed that. |
Replace the `Sprite::size` field with an optional `Sprite::custom_size` field and use the sprite image size for rendering unless `custom_size` is specified.
zicklag
force-pushed
the
sprite-size-redo
branch
from
ce6dae7
to
920e4eb
Compare
Ah, yeah, apparently that's what GitHub does when you use an organization account to fork. I wonder if they let you chnage that. 🤔 I think I can just start pushing to my Anyway, pushed an update! |
|
Ah thats good to know. Thanks! |
|
bors r+ |
# Objective - Prevent the need to have a system that synchronizes sprite sizes with their images ## Solution - Read the sprite size from the image asset when rendering the sprite - Replace the `size` and `resize_mode` fields of `Sprite` with a `custom_size: Option<Vec2>` that will modify the sprite's rendered size to be different than the image size, but only if it is `Some(Vec2)`
|
Build failed: |
|
bors r+ |
# Objective - Prevent the need to have a system that synchronizes sprite sizes with their images ## Solution - Read the sprite size from the image asset when rendering the sprite - Replace the `size` and `resize_mode` fields of `Sprite` with a `custom_size: Option<Vec2>` that will modify the sprite's rendered size to be different than the image size, but only if it is `Some(Vec2)`
|
Pull request successfully merged into pipelined-rendering. Build succeeded: |
bors
bot
changed the title
Objective
Solution
sizeandresize_modefields ofSpritewith acustom_size: Option<Vec2>that will modify the sprite's rendered size to be different than the image size, but only if it isSome(Vec2)