[Merged by Bors] - Make ScalingMode more flexible #3253

Daniikk1012 · 2021-12-05T08:33:43Z

Adds ability to specify scaling factor for WindowSize, size of the fixed axis for FixedVertical and FixedHorizontal and a new ScalingMode that is a mix of FixedVertical and FixedHorizontal

The issue

Currently, only available options are to:

Have one of the axes fixed to value 1
Have viewport size match the window size
Manually adjust viewport size

In most of the games these options are not enough and more advanced scaling methods have to be used

Solution

The solution is to provide additional parameters to current scaling modes, like scaling factor for WindowSize. Additionally, a more advanced Auto mode is added, which dynamically switches between behaving like FixedVertical and FixedHorizontal depending on the window's aspect ratio.

DJMcNab · 2021-12-05T08:53:16Z

How are these new fields different to the scale field which already exists?

That being said, the auto mode seems useful.

Daniikk1012 · 2021-12-05T08:55:58Z

These fields are there because scale is usually used for zooming, so it is better to separate "zoom" scale and "viewport" scale.

Add additional fields to existing modes and add an additional Auto mode

Daniikk1012 · 2021-12-17T14:03:10Z

Right now, in this pull request there is Auto mode, which uses the smallest possible viewport size that is at least the provided size while keeping the aspect ratio. Here is the question: can a viewport that uses the biggest possible viewport size that is at most the provided size be useful and should it be added as well?

crates/bevy_render/src/camera/projection.rs

alice-i-cecile

I like the addition of Auto quite a bit.

However I don't like the addition of an additional way to scale the window viewport. Can you strip down this PR?

Daniikk1012 · 2022-05-16T18:24:01Z

What about the parameters in FixedVertical and FixedHorizontal? There it is not just the scale, but the desired length of a specific side, should that be removed as well?

alice-i-cecile · 2022-05-17T04:00:14Z

Hmm. I personally think we should keep those; I can see the appeal there.

Daniikk1012 · 2022-05-17T11:36:50Z

Apparently, right now FixedHorizontal and FixedVertical will give you different total widh/height depending on the window_origin field of the projection (Bounds would be -1.0..1.0 if WindowOrigin::Center is used, but 0.0..1.0 if WindowOrigin::BottomLeft is used). In this implementation however, it does not work that way, so I added additional commit to account for this.

crates/bevy_render/src/camera/bundle.rs

crates/bevy_render/src/camera/projection.rs

alice-i-cecile · 2022-05-17T17:50:40Z

Excellent, I'm happy with this now.

crates/bevy_render/src/camera/projection.rs

alice-i-cecile · 2022-05-18T15:20:38Z

@Daniikk1012 can you resolve the comment threads that you've addressed? It makes it much easier to follow for future reviewers.

inodentry

I'd like to approve these changes. It improves on what we had previously, in multiple ways. I feel this PR is in a good shape now.

Any additional fixes/improvements/nitpicks can always come via additional PRs, of course. :)

alice-i-cecile · 2022-05-30T15:00:21Z

bors r+

alice-i-cecile · 2022-05-30T16:19:12Z

@Daniikk1012 can you please rebase this so it can be merged?

alice-i-cecile · 2022-05-30T18:15:25Z

bors r+

bors · 2022-05-30T18:35:54Z

Merge conflict.

alice-i-cecile · 2022-05-30T20:08:22Z

bors r+

bors · 2022-05-30T20:39:23Z

Merge conflict.

alice-i-cecile · 2022-05-30T21:04:08Z

Apparently more merge conflicts. You have my sympathies @Daniikk1012...

Thanks for your patience through all this.

Daniikk1012 · 2022-05-31T03:20:54Z

Sorry, I still do not understand how to work with git properly (My first time resolving a merge conflict). This should work now

alice-i-cecile · 2022-05-31T17:13:57Z

Git can be a bit of a pain; it's not just you :) This looks good now, thanks!

bors r+

Adds ability to specify scaling factor for `WindowSize`, size of the fixed axis for `FixedVertical` and `FixedHorizontal` and a new `ScalingMode` that is a mix of `FixedVertical` and `FixedHorizontal` # The issue Currently, only available options are to: * Have one of the axes fixed to value 1 * Have viewport size match the window size * Manually adjust viewport size In most of the games these options are not enough and more advanced scaling methods have to be used ## Solution The solution is to provide additional parameters to current scaling modes, like scaling factor for `WindowSize`. Additionally, a more advanced `Auto` mode is added, which dynamically switches between behaving like `FixedVertical` and `FixedHorizontal` depending on the window's aspect ratio. Co-authored-by: Daniikk1012 <49123959+Daniikk1012@users.noreply.github.com>

bors · 2022-05-31T17:30:23Z

Pull request successfully merged into main.

Build succeeded:

Adds ability to specify scaling factor for `WindowSize`, size of the fixed axis for `FixedVertical` and `FixedHorizontal` and a new `ScalingMode` that is a mix of `FixedVertical` and `FixedHorizontal` # The issue Currently, only available options are to: * Have one of the axes fixed to value 1 * Have viewport size match the window size * Manually adjust viewport size In most of the games these options are not enough and more advanced scaling methods have to be used ## Solution The solution is to provide additional parameters to current scaling modes, like scaling factor for `WindowSize`. Additionally, a more advanced `Auto` mode is added, which dynamically switches between behaving like `FixedVertical` and `FixedHorizontal` depending on the window's aspect ratio. Co-authored-by: Daniikk1012 <49123959+Daniikk1012@users.noreply.github.com>

github-actions bot added the S-Needs-Triage This issue needs to be labelled label Dec 5, 2021

alice-i-cecile added A-Windowing Platform-agnostic interface layer to run your app in C-Usability A targeted quality-of-life change that makes Bevy easier to use S-Needs-Review and removed S-Needs-Triage This issue needs to be labelled labels Dec 5, 2021

Daniikk1012 closed this Dec 14, 2021

Make ScalingMode more flexible

07eabd5

Add additional fields to existing modes and add an additional Auto mode

Daniikk1012 reopened this Dec 14, 2021

DJMcNab requested a review from inodentry December 14, 2021 15:10

cart removed the S-Needs-Review label Dec 16, 2021

alice-i-cecile added M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide and removed S-Needs-Migration-Guide labels Jan 20, 2022

alice-i-cecile reviewed May 16, 2022

View reviewed changes

crates/bevy_render/src/camera/projection.rs Outdated Show resolved Hide resolved

alice-i-cecile requested changes May 16, 2022

View reviewed changes

Daniikk1012 and others added 3 commits May 17, 2022 17:00

Remove parameter from ScalingMode::WindowSize

7c43647

Merge branch 'bevyengine:main' into main

8ae1ef4

Fix uses of FixedHorizontal and FixedVertical

323923a

alice-i-cecile reviewed May 17, 2022

View reviewed changes

crates/bevy_render/src/camera/bundle.rs Show resolved Hide resolved

alice-i-cecile reviewed May 17, 2022

View reviewed changes

crates/bevy_render/src/camera/projection.rs Outdated Show resolved Hide resolved

Specify units in the comments

c0c8af7

alice-i-cecile approved these changes May 17, 2022

View reviewed changes

inodentry reviewed May 18, 2022

View reviewed changes

crates/bevy_render/src/camera/projection.rs Outdated Show resolved Hide resolved

crates/bevy_render/src/camera/projection.rs Show resolved Hide resolved

Daniikk1012 added 2 commits May 18, 2022 21:02

Replace early return with match arm

0a23755

Round viewport size when WindowSize is used

070c480

inodentry approved these changes May 25, 2022

View reviewed changes

alice-i-cecile 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 May 25, 2022

Merge remote-tracking branch 'upstream/main'

9e95242

bors bot changed the title ~~Make ScalingMode more flexible~~ [Merged by Bors] - Make ScalingMode more flexible May 31, 2022

bors bot closed this May 31, 2022

SUPERCILEX mentioned this pull request Jun 8, 2022

Stuff to do for Bevy 0.8 SUPERCILEX/grocery_bagger_9000#1

Closed

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Merged by Bors] - Make ScalingMode more flexible #3253

[Merged by Bors] - Make ScalingMode more flexible #3253

Daniikk1012 commented Dec 5, 2021

DJMcNab commented Dec 5, 2021

Daniikk1012 commented Dec 5, 2021

Daniikk1012 commented Dec 17, 2021

alice-i-cecile left a comment

Daniikk1012 commented May 16, 2022

alice-i-cecile commented May 17, 2022

Daniikk1012 commented May 17, 2022 •

edited

Loading

alice-i-cecile commented May 17, 2022

alice-i-cecile commented May 18, 2022

inodentry left a comment

alice-i-cecile commented May 30, 2022

alice-i-cecile commented May 30, 2022

alice-i-cecile commented May 30, 2022

bors bot commented May 30, 2022

alice-i-cecile commented May 30, 2022

bors bot commented May 30, 2022

alice-i-cecile commented May 30, 2022 •

edited

Loading

Daniikk1012 commented May 31, 2022

alice-i-cecile commented May 31, 2022

bors bot commented May 31, 2022

[Merged by Bors] - Make ScalingMode more flexible #3253

[Merged by Bors] - Make ScalingMode more flexible #3253

Conversation

Daniikk1012 commented Dec 5, 2021

The issue

Solution

DJMcNab commented Dec 5, 2021

Daniikk1012 commented Dec 5, 2021

Daniikk1012 commented Dec 17, 2021

alice-i-cecile left a comment

Choose a reason for hiding this comment

Daniikk1012 commented May 16, 2022

alice-i-cecile commented May 17, 2022

Daniikk1012 commented May 17, 2022 • edited Loading

alice-i-cecile commented May 17, 2022

alice-i-cecile commented May 18, 2022

inodentry left a comment

Choose a reason for hiding this comment

alice-i-cecile commented May 30, 2022

alice-i-cecile commented May 30, 2022

alice-i-cecile commented May 30, 2022

bors bot commented May 30, 2022

alice-i-cecile commented May 30, 2022

bors bot commented May 30, 2022

alice-i-cecile commented May 30, 2022 • edited Loading

Daniikk1012 commented May 31, 2022

alice-i-cecile commented May 31, 2022

bors bot commented May 31, 2022

Daniikk1012 commented May 17, 2022 •

edited

Loading

alice-i-cecile commented May 30, 2022 •

edited

Loading