Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

[Merged by Bors] - Flip UI image #6292

Closed
wants to merge 11 commits into from

Conversation

ickshonpe
Copy link
Contributor

@ickshonpe ickshonpe commented Oct 18, 2022

Objective

Fixes #3225, Allow for flippable UI Images

Solution

Add flip_x and flip_y fields to UiImage, and swap the UV coordinates accordingly in ui_prepare_nodes.

Changelog

  • Changes UiImage to a struct with texture, flip_x, and flip_y fields.
  • Adds flip_x and flip_y fields to ExtractedUiNode.
  • Changes extract_uinodes to extract the flip_x and flip_y values from UiImage.
  • Changes prepare_uinodes to swap the UV coordinates as required.
  • Changes UiImage derefs to texture field accesses.

    Adds the ImageFlip component.
    Adds an ImageFlip field called flip to UiImageBundle.
    Adds flip_x and flip_y fields to ExtractedUiNode.
    Changes extract_uinodes to extract the ImageFlip component values.
    Changes prepare_uinodes to swap the UV coords depending on the values of the ExtractedUiNode's flip_x and flip_y fields.
@alice-i-cecile alice-i-cecile added A-UI Graphical user interfaces, styles, layouts, and widgets C-Usability A simple quality-of-life change that makes Bevy easier to use C-Breaking-Change A breaking change to Bevy's public API that needs to be noted in a migration guide labels Oct 18, 2022
Copy link
Member

@alice-i-cecile alice-i-cecile left a comment

Choose a reason for hiding this comment

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

I'm on board; this is a clean implementation of a useful feature.

I'd probably prefer a singular Flip component, which is reused across sprites and UI images, but I won't block on that.

@ickshonpe
Copy link
Contributor Author

ickshonpe commented Oct 19, 2022

I'd probably prefer a singular Flip component, which is reused across sprites and UI images, but I won't block on that.

Yes, I'm trying to keep things as close to sprites as possible (with the least breaking changes).

I chose FlipImage rather than Flip because a lot of people already find the UI confusing, so I wanted to be extra clear about what it's flipping. Most of the ambiguity is in the Style component though so maybe Flip is fine. Or it could be FlipTexture.

@mockersf
Copy link
Member

mockersf commented Oct 19, 2022

I don't think it should be a separate component. What happens if I add the FlipImage component to a PbrBundle? Is the image file used for texture flipped?

I would prefer to have two fields flip_x and flip_y in an existing UI component, maybe UiImage

@ickshonpe
Copy link
Contributor Author

ickshonpe commented Nov 1, 2022

I don't think it should be a separate component. What happens if I add the FlipImage component to a PbrBundle? Is the image file used for texture flipped?

I would prefer to have two fields flip_x and flip_y in an existing UI component, maybe UiImage

I agree that would be better, it's just that I was worried it'd break a lot of existing code that expects UiImage to be a newtype.

ImageMode is only used as a marker component for the image widget at the moment, the flip_x/y fields could be added to it as an alternative? Or would it be better to just have the changes to ExtractedUiNode and ui_prepare_nodes in this PR, and have another PR for the interface?

@alice-i-cecile
Copy link
Member

I agree that would be better, it's just that I was worried it'd break a lot of existing code that expects UiImage to be a newtype.

Yep, no worries about breaking existing code here :) This sort of migration is very easy. Not immediately sure about your other question though.

Copy link
Contributor

@Weibye Weibye left a comment

Choose a reason for hiding this comment

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

Clean and straightforward!

@rparrett rparrett added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Nov 7, 2022
@cart
Copy link
Member

cart commented Nov 14, 2022

bors r+

bors bot pushed a commit that referenced this pull request Nov 14, 2022
# Objective
Fixes  #3225, Allow for flippable UI Images

## Solution
Add flip_x and flip_y fields to UiImage, and swap the UV coordinates accordingly in ui_prepare_nodes.

## Changelog
* Changes UiImage to a struct with texture, flip_x, and flip_y fields.
* Adds flip_x and flip_y fields to ExtractedUiNode.
* Changes extract_uinodes to extract the flip_x and flip_y values from UiImage.
* Changes prepare_uinodes to swap the UV coordinates as required.
* Changes UiImage derefs to texture field accesses.
@bors
Copy link
Contributor

bors bot commented Nov 14, 2022

Build failed (retrying...):

bors bot pushed a commit that referenced this pull request Nov 14, 2022
# Objective
Fixes  #3225, Allow for flippable UI Images

## Solution
Add flip_x and flip_y fields to UiImage, and swap the UV coordinates accordingly in ui_prepare_nodes.

## Changelog
* Changes UiImage to a struct with texture, flip_x, and flip_y fields.
* Adds flip_x and flip_y fields to ExtractedUiNode.
* Changes extract_uinodes to extract the flip_x and flip_y values from UiImage.
* Changes prepare_uinodes to swap the UV coordinates as required.
* Changes UiImage derefs to texture field accesses.
@bors bors bot changed the title Flip UI image [Merged by Bors] - Flip UI image Nov 14, 2022
@bors bors bot closed this Nov 14, 2022
taiyoungjang pushed a commit to taiyoungjang/bevy that referenced this pull request Dec 15, 2022
# Objective
Fixes  bevyengine#3225, Allow for flippable UI Images

## Solution
Add flip_x and flip_y fields to UiImage, and swap the UV coordinates accordingly in ui_prepare_nodes.

## Changelog
* Changes UiImage to a struct with texture, flip_x, and flip_y fields.
* Adds flip_x and flip_y fields to ExtractedUiNode.
* Changes extract_uinodes to extract the flip_x and flip_y values from UiImage.
* Changes prepare_uinodes to swap the UV coordinates as required.
* Changes UiImage derefs to texture field accesses.
alradish pushed a commit to alradish/bevy that referenced this pull request Jan 22, 2023
# Objective
Fixes  bevyengine#3225, Allow for flippable UI Images

## Solution
Add flip_x and flip_y fields to UiImage, and swap the UV coordinates accordingly in ui_prepare_nodes.

## Changelog
* Changes UiImage to a struct with texture, flip_x, and flip_y fields.
* Adds flip_x and flip_y fields to ExtractedUiNode.
* Changes extract_uinodes to extract the flip_x and flip_y values from UiImage.
* Changes prepare_uinodes to swap the UV coordinates as required.
* Changes UiImage derefs to texture field accesses.
ItsDoot pushed a commit to ItsDoot/bevy that referenced this pull request Feb 1, 2023
# Objective
Fixes  bevyengine#3225, Allow for flippable UI Images

## Solution
Add flip_x and flip_y fields to UiImage, and swap the UV coordinates accordingly in ui_prepare_nodes.

## Changelog
* Changes UiImage to a struct with texture, flip_x, and flip_y fields.
* Adds flip_x and flip_y fields to ExtractedUiNode.
* Changes extract_uinodes to extract the flip_x and flip_y values from UiImage.
* Changes prepare_uinodes to swap the UV coordinates as required.
* Changes UiImage derefs to texture field accesses.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-UI Graphical user interfaces, styles, layouts, and widgets C-Breaking-Change A breaking change to Bevy's public API that needs to be noted in a migration guide C-Usability A simple quality-of-life change that makes Bevy easier to use S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow for flippable UI Images
6 participants