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] - Change UI coordinate system to have origin at top left corner #6000

Closed
wants to merge 13 commits into from

Conversation

mahulst
Copy link
Contributor

@mahulst mahulst commented Sep 17, 2022

Objective

Fixes #5572

Solution

Approach is to invert the Y-axis of the UI Camera by changing the UI projection matrix to render the UI upside down.

After that I'm trying to fix all issues, that pop up:

  • interaction expected the "old" position
  • images and text were displayed upside-down
  • baseline of text was based on the top of the glyph instead of bottom

... probably a lot more.


Result when running examples:

Button example

main branch:
button main
this pr:
button pr

Text example

m
text main
ain branch:

this pr:
text pr fixed

Text debug example

main branch:
text_debug main

this pr:
text_debug pr fixed

Transparency UI example

main branch:
transparency_ui main

this pr:
transperency_ui pr

UI example

ui example
main branch:
ui main

this pr:
ui pr fixed

Changelog

UI coordinate system and cursor position was changed from bottom left origin, y+ up to top left origin, y+ down.

Migration Guide

All flex layout should be inverted (ColumnReverse => Column, FlexStart => FlexEnd, WrapReverse => Wrap)
System where dealing with cursor position should be changed to account for cursor position being based on the top left instead of bottom left

Copy link
Contributor

@tim-blackbird tim-blackbird left a comment

Choose a reason for hiding this comment

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

In the examples, FlexDirection::ColumnReverse should be replaced with FlexDirection::Column :)

crates/bevy_ui/src/focus.rs Outdated Show resolved Hide resolved
@djeedai djeedai added A-UI Graphical user interfaces, styles, layouts, and widgets M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide labels Sep 19, 2022
@mahulst mahulst force-pushed the feature/invert-y-ui branch 2 times, most recently from 4ca9759 to ba1318e Compare September 25, 2022 11:02
@mahulst mahulst force-pushed the feature/invert-y-ui branch from ba1318e to 8429b6d Compare September 25, 2022 11:05
@mahulst mahulst force-pushed the feature/invert-y-ui branch from a4f1c72 to 9aa2503 Compare October 7, 2022 11:48
@mahulst mahulst requested a review from tim-blackbird October 7, 2022 11:52
@mahulst
Copy link
Contributor Author

mahulst commented Oct 7, 2022

I think the PR is ready to go now. All examples working as they should.

@alice-i-cecile alice-i-cecile requested review from inodentry and Weibye and removed request for tim-blackbird October 7, 2022 18:03
@alice-i-cecile
Copy link
Member

@inodentry @Weibye @TimJentzsch can you review this over the weekend? I'd very much like to merge this on Monday before any other UI PRs.

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.

To the best of my ability this looks good. Great work on this!

examples/ui/text.rs Show resolved Hide resolved
Copy link
Contributor

@TimJentzsch TimJentzsch left a comment

Choose a reason for hiding this comment

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

Thanks a lot for working on this, this will make Bevy UI a lot less confusing!

I left a few comments regarding the AlignSelf::FlexStart. The default value is AlignSelf::Auto, but it might visually the same.

I'd suggest that we try to remove the explicitly set values and see if it still looks similar, if yes I'd be in favor of removing them to make the examples a bit shorter.

crates/bevy_ui/src/render/mod.rs Show resolved Hide resolved
examples/ui/text.rs Outdated Show resolved Hide resolved
examples/ui/text_debug.rs Outdated Show resolved Hide resolved
examples/ui/ui.rs Outdated Show resolved Hide resolved
examples/ui/ui.rs Show resolved Hide resolved
examples/ui/text.rs Show resolved Hide resolved
@TimJentzsch
Copy link
Contributor

In the changelog / migration guide I would suggest to clearly specify that the UI coordinate system was bottom left origin, y+ up and got changed to top left origin, y+ down; to avoid confusion :)

@mahulst
Copy link
Contributor Author

mahulst commented Oct 9, 2022

@TimJentzsch Changelog was updated, and I removed the redundant styles where it made no changes.

Copy link
Contributor

@TimJentzsch TimJentzsch left a comment

Choose a reason for hiding this comment

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

Nice work!

@alice-i-cecile alice-i-cecile removed 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 Oct 10, 2022
@alice-i-cecile alice-i-cecile added this to the Bevy 0.9 milestone Oct 10, 2022
@mahulst
Copy link
Contributor Author

mahulst commented Oct 10, 2022

The games/game_menu.rs example is broken: the menu item order needs to be inverted (or the FlexLayout strategy swapped. Once that's done this LGTM.

Argh! I'll look at this in the morning.
Also github says you suggested changes but I don't see any. Don't know what thats about...

@inodentry
Copy link
Contributor

"suggested changes" is just one of the 3 "status" options you can choose when leaving a PR review. It doesn't refer to anything specific.

When you review a PR, it asks you to select what "kind" your review is: just leaving comments, an endorsement/approval, or wanting changes. It's just to help communicate how the reviewer feels about your PR.

@mahulst
Copy link
Contributor Author

mahulst commented Oct 11, 2022

"suggested changes" is just one of the 3 "status" options you can choose when leaving a PR review. It doesn't refer to anything specific.

When you review a PR, it asks you to select what "kind" your review is: just leaving comments, an endorsement/approval, or wanting changes. It's just to help communicate how the reviewer feels about your PR.

Ah right 🤦 I was expecting a code suggestion or something.

@alice-i-cecile example has been updated! Does this need to be merged by cart?

@alice-i-cecile
Copy link
Member

Nope, I can merge this :) It would be controversial, but @cart gave his blessing in the linked issue. The actual implementation isn't the tricky part, so this has already passed through the critical review step.

@alice-i-cecile alice-i-cecile added the C-Usability A targeted quality-of-life change that makes Bevy easier to use label Oct 11, 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've tested the examples again, and they're all working for me.

bors r+

bors bot pushed a commit that referenced this pull request Oct 11, 2022
# Objective
Fixes #5572

## Solution

Approach is to invert the Y-axis of the UI Camera by changing the UI projection matrix to render the UI upside down.

After that I'm trying to fix all issues, that pop up:
- interaction expected the "old" position
- images and text were displayed upside-down
- baseline of text was based on the top of the glyph instead of bottom

... probably a lot more.

---

Result when running examples:
<details>
    <summary>Button example</summary>

main branch:
![button main](https://user-images.githubusercontent.com/4232644/190856087-61dd1d98-42b5-4238-bd97-149744ddfeba.png)
this pr:
![button pr](https://user-images.githubusercontent.com/4232644/190856097-3f4bc97a-ed15-4e97-b7f1-2b2dd6bb8b14.png)

</details>

<details>
    <summary>Text example</summary>

m
![text main](https://user-images.githubusercontent.com/4232644/192142831-4cf19aa1-f49a-485e-af7b-374d6f5c396c.png)
ain branch: 


this pr:
![text pr fixed](https://user-images.githubusercontent.com/4232644/192142829-c433db3b-32e1-4ee8-b493-0b4a4d9c8e70.png)


</details>

<details>
    <summary>Text debug example</summary>

main branch:
![text_debug main](https://user-images.githubusercontent.com/4232644/192142822-940aefa6-e502-410b-8da4-5570f77b5df2.png)

this pr:
![text_debug pr fixed](https://user-images.githubusercontent.com/4232644/194547010-8c968f5c-5a71-4ffc-871d-790c06d48016.png)

</details>

<details>
    <summary>Transparency UI example</summary>

main branch:
![transparency_ui main](https://user-images.githubusercontent.com/4232644/190856172-328c60fe-3622-4598-97d5-2f1595db13b3.png)


this pr:
![transperency_ui pr](https://user-images.githubusercontent.com/4232644/190856179-a2dafb99-41ea-45a9-9dd6-400fa3ef24b9.png)

</details>

<details>
    <summary>UI example</summary>

**ui example**
main branch:
![ui main](https://user-images.githubusercontent.com/4232644/192142812-e20ba31a-6841-46d9-a785-4198cf22dc99.png)

this pr:
![ui pr fixed](https://user-images.githubusercontent.com/4232644/192142788-cc0b74e0-7710-4faa-b5a2-60270a5da77c.png)

</details>

## Changelog
UI coordinate system and cursor position was changed from bottom left origin, y+ up to top left origin, y+ down.

## Migration Guide
All flex layout should be inverted (ColumnReverse => Column, FlexStart => FlexEnd, WrapReverse => Wrap)
System where dealing with cursor position should be changed to account for cursor position being based on the top left instead of bottom left
@bors bors bot changed the title Change UI coordinate system to have origin at top left corner [Merged by Bors] - Change UI coordinate system to have origin at top left corner Oct 11, 2022
@bors bors bot closed this Oct 11, 2022
This was referenced Oct 16, 2022
james7132 pushed a commit to james7132/bevy that referenced this pull request Oct 19, 2022
…gine#6000)

# Objective
Fixes bevyengine#5572

## Solution

Approach is to invert the Y-axis of the UI Camera by changing the UI projection matrix to render the UI upside down.

After that I'm trying to fix all issues, that pop up:
- interaction expected the "old" position
- images and text were displayed upside-down
- baseline of text was based on the top of the glyph instead of bottom

... probably a lot more.

---

Result when running examples:
<details>
    <summary>Button example</summary>

main branch:
![button main](https://user-images.githubusercontent.com/4232644/190856087-61dd1d98-42b5-4238-bd97-149744ddfeba.png)
this pr:
![button pr](https://user-images.githubusercontent.com/4232644/190856097-3f4bc97a-ed15-4e97-b7f1-2b2dd6bb8b14.png)

</details>

<details>
    <summary>Text example</summary>

m
![text main](https://user-images.githubusercontent.com/4232644/192142831-4cf19aa1-f49a-485e-af7b-374d6f5c396c.png)
ain branch: 


this pr:
![text pr fixed](https://user-images.githubusercontent.com/4232644/192142829-c433db3b-32e1-4ee8-b493-0b4a4d9c8e70.png)


</details>

<details>
    <summary>Text debug example</summary>

main branch:
![text_debug main](https://user-images.githubusercontent.com/4232644/192142822-940aefa6-e502-410b-8da4-5570f77b5df2.png)

this pr:
![text_debug pr fixed](https://user-images.githubusercontent.com/4232644/194547010-8c968f5c-5a71-4ffc-871d-790c06d48016.png)

</details>

<details>
    <summary>Transparency UI example</summary>

main branch:
![transparency_ui main](https://user-images.githubusercontent.com/4232644/190856172-328c60fe-3622-4598-97d5-2f1595db13b3.png)


this pr:
![transperency_ui pr](https://user-images.githubusercontent.com/4232644/190856179-a2dafb99-41ea-45a9-9dd6-400fa3ef24b9.png)

</details>

<details>
    <summary>UI example</summary>

**ui example**
main branch:
![ui main](https://user-images.githubusercontent.com/4232644/192142812-e20ba31a-6841-46d9-a785-4198cf22dc99.png)

this pr:
![ui pr fixed](https://user-images.githubusercontent.com/4232644/192142788-cc0b74e0-7710-4faa-b5a2-60270a5da77c.png)

</details>

## Changelog
UI coordinate system and cursor position was changed from bottom left origin, y+ up to top left origin, y+ down.

## Migration Guide
All flex layout should be inverted (ColumnReverse => Column, FlexStart => FlexEnd, WrapReverse => Wrap)
System where dealing with cursor position should be changed to account for cursor position being based on the top left instead of bottom left
bors bot pushed a commit that referenced this pull request Oct 24, 2022
# Objective

- Clipping (visible in the UI example with text scrolling) is funky 
- Fixes #6287 

## Solution

- Fix UV calculation:
  - correct order for values (issue introduced in #6000)

  - add the `y` values instead of subtracting them now that vertical order is reversed
  - take scale factor into account (bug already present before reversing the order)
- While around clipping, I changed clip to only mutate when changed

No more funkiness! 😞 

<img width="696" alt="Screenshot 2022-10-23 at 22 44 18" src="https://user-images.githubusercontent.com/8672791/197417721-30ad4150-5264-427f-ac82-e5265c1fb3a9.png">
james7132 pushed a commit to james7132/bevy that referenced this pull request Oct 28, 2022
…gine#6000)

# Objective
Fixes bevyengine#5572

## Solution

Approach is to invert the Y-axis of the UI Camera by changing the UI projection matrix to render the UI upside down.

After that I'm trying to fix all issues, that pop up:
- interaction expected the "old" position
- images and text were displayed upside-down
- baseline of text was based on the top of the glyph instead of bottom

... probably a lot more.

---

Result when running examples:
<details>
    <summary>Button example</summary>

main branch:
![button main](https://user-images.githubusercontent.com/4232644/190856087-61dd1d98-42b5-4238-bd97-149744ddfeba.png)
this pr:
![button pr](https://user-images.githubusercontent.com/4232644/190856097-3f4bc97a-ed15-4e97-b7f1-2b2dd6bb8b14.png)

</details>

<details>
    <summary>Text example</summary>

m
![text main](https://user-images.githubusercontent.com/4232644/192142831-4cf19aa1-f49a-485e-af7b-374d6f5c396c.png)
ain branch: 


this pr:
![text pr fixed](https://user-images.githubusercontent.com/4232644/192142829-c433db3b-32e1-4ee8-b493-0b4a4d9c8e70.png)


</details>

<details>
    <summary>Text debug example</summary>

main branch:
![text_debug main](https://user-images.githubusercontent.com/4232644/192142822-940aefa6-e502-410b-8da4-5570f77b5df2.png)

this pr:
![text_debug pr fixed](https://user-images.githubusercontent.com/4232644/194547010-8c968f5c-5a71-4ffc-871d-790c06d48016.png)

</details>

<details>
    <summary>Transparency UI example</summary>

main branch:
![transparency_ui main](https://user-images.githubusercontent.com/4232644/190856172-328c60fe-3622-4598-97d5-2f1595db13b3.png)


this pr:
![transperency_ui pr](https://user-images.githubusercontent.com/4232644/190856179-a2dafb99-41ea-45a9-9dd6-400fa3ef24b9.png)

</details>

<details>
    <summary>UI example</summary>

**ui example**
main branch:
![ui main](https://user-images.githubusercontent.com/4232644/192142812-e20ba31a-6841-46d9-a785-4198cf22dc99.png)

this pr:
![ui pr fixed](https://user-images.githubusercontent.com/4232644/192142788-cc0b74e0-7710-4faa-b5a2-60270a5da77c.png)

</details>

## Changelog
UI coordinate system and cursor position was changed from bottom left origin, y+ up to top left origin, y+ down.

## Migration Guide
All flex layout should be inverted (ColumnReverse => Column, FlexStart => FlexEnd, WrapReverse => Wrap)
System where dealing with cursor position should be changed to account for cursor position being based on the top left instead of bottom left
james7132 pushed a commit to james7132/bevy that referenced this pull request Oct 28, 2022
# Objective

- Clipping (visible in the UI example with text scrolling) is funky 
- Fixes bevyengine#6287 

## Solution

- Fix UV calculation:
  - correct order for values (issue introduced in bevyengine#6000)

  - add the `y` values instead of subtracting them now that vertical order is reversed
  - take scale factor into account (bug already present before reversing the order)
- While around clipping, I changed clip to only mutate when changed

No more funkiness! 😞 

<img width="696" alt="Screenshot 2022-10-23 at 22 44 18" src="https://user-images.githubusercontent.com/8672791/197417721-30ad4150-5264-427f-ac82-e5265c1fb3a9.png">
Pietrek14 pushed a commit to Pietrek14/bevy that referenced this pull request Dec 17, 2022
…gine#6000)

# Objective
Fixes bevyengine#5572

## Solution

Approach is to invert the Y-axis of the UI Camera by changing the UI projection matrix to render the UI upside down.

After that I'm trying to fix all issues, that pop up:
- interaction expected the "old" position
- images and text were displayed upside-down
- baseline of text was based on the top of the glyph instead of bottom

... probably a lot more.

---

Result when running examples:
<details>
    <summary>Button example</summary>

main branch:
![button main](https://user-images.githubusercontent.com/4232644/190856087-61dd1d98-42b5-4238-bd97-149744ddfeba.png)
this pr:
![button pr](https://user-images.githubusercontent.com/4232644/190856097-3f4bc97a-ed15-4e97-b7f1-2b2dd6bb8b14.png)

</details>

<details>
    <summary>Text example</summary>

m
![text main](https://user-images.githubusercontent.com/4232644/192142831-4cf19aa1-f49a-485e-af7b-374d6f5c396c.png)
ain branch: 


this pr:
![text pr fixed](https://user-images.githubusercontent.com/4232644/192142829-c433db3b-32e1-4ee8-b493-0b4a4d9c8e70.png)


</details>

<details>
    <summary>Text debug example</summary>

main branch:
![text_debug main](https://user-images.githubusercontent.com/4232644/192142822-940aefa6-e502-410b-8da4-5570f77b5df2.png)

this pr:
![text_debug pr fixed](https://user-images.githubusercontent.com/4232644/194547010-8c968f5c-5a71-4ffc-871d-790c06d48016.png)

</details>

<details>
    <summary>Transparency UI example</summary>

main branch:
![transparency_ui main](https://user-images.githubusercontent.com/4232644/190856172-328c60fe-3622-4598-97d5-2f1595db13b3.png)


this pr:
![transperency_ui pr](https://user-images.githubusercontent.com/4232644/190856179-a2dafb99-41ea-45a9-9dd6-400fa3ef24b9.png)

</details>

<details>
    <summary>UI example</summary>

**ui example**
main branch:
![ui main](https://user-images.githubusercontent.com/4232644/192142812-e20ba31a-6841-46d9-a785-4198cf22dc99.png)

this pr:
![ui pr fixed](https://user-images.githubusercontent.com/4232644/192142788-cc0b74e0-7710-4faa-b5a2-60270a5da77c.png)

</details>

## Changelog
UI coordinate system and cursor position was changed from bottom left origin, y+ up to top left origin, y+ down.

## Migration Guide
All flex layout should be inverted (ColumnReverse => Column, FlexStart => FlexEnd, WrapReverse => Wrap)
System where dealing with cursor position should be changed to account for cursor position being based on the top left instead of bottom left
Pietrek14 pushed a commit to Pietrek14/bevy that referenced this pull request Dec 17, 2022
# Objective

- Clipping (visible in the UI example with text scrolling) is funky 
- Fixes bevyengine#6287 

## Solution

- Fix UV calculation:
  - correct order for values (issue introduced in bevyengine#6000)

  - add the `y` values instead of subtracting them now that vertical order is reversed
  - take scale factor into account (bug already present before reversing the order)
- While around clipping, I changed clip to only mutate when changed

No more funkiness! 😞 

<img width="696" alt="Screenshot 2022-10-23 at 22 44 18" src="https://user-images.githubusercontent.com/8672791/197417721-30ad4150-5264-427f-ac82-e5265c1fb3a9.png">
ItsDoot pushed a commit to ItsDoot/bevy that referenced this pull request Feb 1, 2023
…gine#6000)

# Objective
Fixes bevyengine#5572

## Solution

Approach is to invert the Y-axis of the UI Camera by changing the UI projection matrix to render the UI upside down.

After that I'm trying to fix all issues, that pop up:
- interaction expected the "old" position
- images and text were displayed upside-down
- baseline of text was based on the top of the glyph instead of bottom

... probably a lot more.

---

Result when running examples:
<details>
    <summary>Button example</summary>

main branch:
![button main](https://user-images.githubusercontent.com/4232644/190856087-61dd1d98-42b5-4238-bd97-149744ddfeba.png)
this pr:
![button pr](https://user-images.githubusercontent.com/4232644/190856097-3f4bc97a-ed15-4e97-b7f1-2b2dd6bb8b14.png)

</details>

<details>
    <summary>Text example</summary>

m
![text main](https://user-images.githubusercontent.com/4232644/192142831-4cf19aa1-f49a-485e-af7b-374d6f5c396c.png)
ain branch: 


this pr:
![text pr fixed](https://user-images.githubusercontent.com/4232644/192142829-c433db3b-32e1-4ee8-b493-0b4a4d9c8e70.png)


</details>

<details>
    <summary>Text debug example</summary>

main branch:
![text_debug main](https://user-images.githubusercontent.com/4232644/192142822-940aefa6-e502-410b-8da4-5570f77b5df2.png)

this pr:
![text_debug pr fixed](https://user-images.githubusercontent.com/4232644/194547010-8c968f5c-5a71-4ffc-871d-790c06d48016.png)

</details>

<details>
    <summary>Transparency UI example</summary>

main branch:
![transparency_ui main](https://user-images.githubusercontent.com/4232644/190856172-328c60fe-3622-4598-97d5-2f1595db13b3.png)


this pr:
![transperency_ui pr](https://user-images.githubusercontent.com/4232644/190856179-a2dafb99-41ea-45a9-9dd6-400fa3ef24b9.png)

</details>

<details>
    <summary>UI example</summary>

**ui example**
main branch:
![ui main](https://user-images.githubusercontent.com/4232644/192142812-e20ba31a-6841-46d9-a785-4198cf22dc99.png)

this pr:
![ui pr fixed](https://user-images.githubusercontent.com/4232644/192142788-cc0b74e0-7710-4faa-b5a2-60270a5da77c.png)

</details>

## Changelog
UI coordinate system and cursor position was changed from bottom left origin, y+ up to top left origin, y+ down.

## Migration Guide
All flex layout should be inverted (ColumnReverse => Column, FlexStart => FlexEnd, WrapReverse => Wrap)
System where dealing with cursor position should be changed to account for cursor position being based on the top left instead of bottom left
ItsDoot pushed a commit to ItsDoot/bevy that referenced this pull request Feb 1, 2023
# Objective

- Clipping (visible in the UI example with text scrolling) is funky 
- Fixes bevyengine#6287 

## Solution

- Fix UV calculation:
  - correct order for values (issue introduced in bevyengine#6000)

  - add the `y` values instead of subtracting them now that vertical order is reversed
  - take scale factor into account (bug already present before reversing the order)
- While around clipping, I changed clip to only mutate when changed

No more funkiness! 😞 

<img width="696" alt="Screenshot 2022-10-23 at 22 44 18" src="https://user-images.githubusercontent.com/8672791/197417721-30ad4150-5264-427f-ac82-e5265c1fb3a9.png">
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-Usability A targeted quality-of-life change that makes Bevy easier to use M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Change UI coordinate system to have origin at top left corner
7 participants