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

Document OrthographicProjection #6177

Closed
wants to merge 2 commits into from
Closed

Conversation

xgbwei
Copy link
Contributor

@xgbwei xgbwei commented Oct 6, 2022

Objective

Solution

  • Provide explanations for OrthographicProjection and its fields.

@alice-i-cecile alice-i-cecile added C-Docs An addition or correction to our documentation A-Rendering Drawing game state to the screen labels Oct 6, 2022
@alice-i-cecile
Copy link
Member

Ping @bzm3r for review, since this was your PR that got adopted :)

@bzm3r
Copy link
Contributor

bzm3r commented Oct 6, 2022

@xgbwei thanks for adopting this PR! I was procrastinating on it quite hard. Overall, it's good, but I think you cut out too much in some places.

Also: it's okay for docs to be wordier, for the sake of clarity, rather than less wordy at the expense of possible lack of clarity.

@@ -161,17 +161,49 @@ pub enum ScalingMode {
FixedHorizontal(f32),
}

/// Project a 3D space onto a 2D surface using parallel lines, i.e., objects have the same
/// apparent size regardless of depth. The volume contained in the projection is called the view frustum.
Copy link
Contributor

Choose a reason for hiding this comment

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

We should say that the volume is a rectangular prism, so that the top, bottom, left, right, near, and far make sense to the reader in this context.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did consider this, but "parallel lines" is more accurate to the technical definition of orthographic projection.

Maybe something along the lines of "Since the viewport is rectangular, The view frustum is in the shape of a rectangular prism" at the end of the paragraph would work?

crates/bevy_render/src/camera/projection.rs Outdated Show resolved Hide resolved
pub window_origin: WindowOrigin,
pub scaling_mode: ScalingMode,
/// Scales the cross sectional area of the view frustum.
/// Scales the width and height of the view frustum.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This should be a bit more specific as the cross sectional area is a function of width and height, not the other way around.

///
/// Objects further than this will not be rendered.
pub far: f32,
/// Specifies where `(0, 0)` is located.
Copy link
Contributor Author

@xgbwei xgbwei Oct 6, 2022

Choose a reason for hiding this comment

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

I've checked the implementation for scaling and this is incorrect. See the newly added docs for WindowOrigin.

Maybe window_origin should be renamed to scaling_origin?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, I'm on board with that. We should consider renaming the type too.

///
/// If the window is smaller than the view frustum, some objects within the view frustum will not appear on the screen.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is there any way for the width/height of the view frustum to be different than the width/height of the window? The only case I can think of is if scaling_mode is None, but then contents in the view frustum will just be stretched to fill the window.

Copy link
Contributor

Choose a reason for hiding this comment

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

@xgbwei Will the contents just be stretched to fit the window? I thought some things will just lie outside the window if the view frustrum is larger than the window, for example (and thus those objects will not be visible)?

Copy link
Contributor

Choose a reason for hiding this comment

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

Is there any way for the width/height of the view frustum to be different than the width/height of the window? The only case I can think of is if scaling_mode is None, but then contents in the view frustum will just be stretched to fill the window.

The viewport field on the camera allows you to specify custom viewport dimensions, for things like multiple views. In my tiled camera crate I use it to force the game view to a multiple of some target resolution to avoid pixel artifacts when rendering low resolution images.

crates/bevy_render/src/camera/projection.rs Show resolved Hide resolved
@@ -138,7 +138,15 @@ impl Default for PerspectiveProjection {
#[derive(Debug, Clone, Reflect, FromReflect, Serialize, Deserialize)]
#[reflect(Serialize, Deserialize)]
pub enum WindowOrigin {
/// Scale the view frustum from the center.
Copy link
Contributor

Choose a reason for hiding this comment

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

Wow, thank you for looking into this! This is so much clearer.

Can you clarify the following?

    /// When resizing the window, object in the center of the screen remain in the center, while opposite
    /// faces of the view frustum expand/shrink equally.

I understand, that in contrast to BottomLeft, which keeps the bottom and left faces fixed, this will change all the faces equally to perform the scaling operation. But I am not sure I understand the significance of "objects in the center of the screen remain in the center".

What happens to objects at the center of the screen when using BottomLeft?

@xgbwei
Copy link
Contributor Author

xgbwei commented Oct 8, 2022

Moved to #6201

@xgbwei xgbwei closed this Oct 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Rendering Drawing game state to the screen C-Docs An addition or correction to our documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Lack of documentation for OrthographicProjection
4 participants