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 scaling mode to FixedHorizontal #4055

Closed
wants to merge 2 commits into from

Conversation

kirusfg
Copy link
Contributor

@kirusfg kirusfg commented Feb 27, 2022

Objective

Solution

  • This was due to wrong scaling mode being used. This PR simply changes WindowSize scaling mode to FixedHorizontal.
  • Divide the scale by 2, see the note

Additional Note

As per discussion here, the orthographic scale has to be divided by 2.

@github-actions github-actions bot added the S-Needs-Triage This issue needs to be labelled label Feb 27, 2022
@alice-i-cecile alice-i-cecile added A-Rendering Drawing game state to the screen A-Assets Load files from disk to use for things like images, models, and sounds C-Bug An unexpected or incorrect behavior and removed S-Needs-Triage This issue needs to be labelled labels Feb 27, 2022
Copy link
Contributor

@superdump superdump left a comment

Choose a reason for hiding this comment

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

The changes look good to me, I just don't know if anyone uses gltf for 2D and sprites so the change from CAMERA_2D to CAMERA_3D could be controversial. It makes sense to me though.

@superdump superdump requested a review from mockersf March 3, 2022 22:39
@mockersf
Copy link
Member

mockersf commented Mar 5, 2022

I don't think it really makes sense to use gltf for 2D...

@mockersf mockersf 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 Mar 5, 2022
@kirusfg kirusfg force-pushed the fix/gltf-orthographic-camera branch 2 times, most recently from 9ca3718 to ccad2ea Compare March 22, 2022 18:30
@superdump superdump added this to the Bevy 0.7 milestone Apr 8, 2022
Copy link
Member

@mockersf mockersf left a comment

Choose a reason for hiding this comment

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

I think we should not specify left/right/top/bottom when scaling_mode is ScalingMode::FixedHorizontal.

@kirusfg kirusfg force-pushed the fix/gltf-orthographic-camera branch from ccad2ea to bd2b6b0 Compare April 8, 2022 11:14
Co-authored-by: François <mockersf@gmail.com>
Copy link
Contributor

@superdump superdump left a comment

Choose a reason for hiding this comment

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

LGTM. And I saw the discussion in Discord and that it has been tested, exporting a scene with an orthographic camera from current Blender to glTF and it looks correct.

@mockersf
Copy link
Member

mockersf commented Apr 8, 2022

bors r+

bors bot pushed a commit that referenced this pull request Apr 8, 2022
# Objective

- Fixes the issue with orthographic camera imported from glTF not displaying anything (mentioned in #4005).

## Solution

- This was due to wrong scaling mode being used. This PR simply changes WindowSize scaling mode to FixedHorizontal.

## Important Note

Currently, othographic scale in Blender, three.js, and possibly other software does not translate to Bevy (via glTF) because their developers have [misinterpreted the spec](KhronosGroup/glTF#1663 (comment)). The camera parameters have been clarified in glTF 2.0, which was released on October of 2021. In Blender 3.0.1 this issue has **not** been fixed yet. If you are importing orthographic cameras from Blender, you have to divide the scale by 2.
@bors bors bot changed the title Change scaling mode to FixedHorizontal [Merged by Bors] - Change scaling mode to FixedHorizontal Apr 8, 2022
@bors bors bot closed this Apr 8, 2022
@kirusfg kirusfg deleted the fix/gltf-orthographic-camera branch April 8, 2022 17:44
aevyrie pushed a commit to aevyrie/bevy that referenced this pull request Jun 7, 2022
# Objective

- Fixes the issue with orthographic camera imported from glTF not displaying anything (mentioned in bevyengine#4005).

## Solution

- This was due to wrong scaling mode being used. This PR simply changes WindowSize scaling mode to FixedHorizontal.

## Important Note

Currently, othographic scale in Blender, three.js, and possibly other software does not translate to Bevy (via glTF) because their developers have [misinterpreted the spec](KhronosGroup/glTF#1663 (comment)). The camera parameters have been clarified in glTF 2.0, which was released on October of 2021. In Blender 3.0.1 this issue has **not** been fixed yet. If you are importing orthographic cameras from Blender, you have to divide the scale by 2.
ItsDoot pushed a commit to ItsDoot/bevy that referenced this pull request Feb 1, 2023
# Objective

- Fixes the issue with orthographic camera imported from glTF not displaying anything (mentioned in bevyengine#4005).

## Solution

- This was due to wrong scaling mode being used. This PR simply changes WindowSize scaling mode to FixedHorizontal.

## Important Note

Currently, othographic scale in Blender, three.js, and possibly other software does not translate to Bevy (via glTF) because their developers have [misinterpreted the spec](KhronosGroup/glTF#1663 (comment)). The camera parameters have been clarified in glTF 2.0, which was released on October of 2021. In Blender 3.0.1 this issue has **not** been fixed yet. If you are importing orthographic cameras from Blender, you have to divide the scale by 2.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Assets Load files from disk to use for things like images, models, and sounds A-Rendering Drawing game state to the screen C-Bug An unexpected or incorrect behavior 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.

4 participants