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

Documenting OrthographicProjection. #5824

Closed
wants to merge 27 commits into from
Closed
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
11727c5
Documenting `BufferVec`.
May 6, 2022
9ef6d24
Replace RenderDevice by GPU.
Jun 21, 2022
5ffd580
State Pod as the direct requirement; and reflect reality of encase no…
Jun 21, 2022
da1f666
Say "system RAM" instead of "host memory" and "VRAM" instead of "devi…
Jun 21, 2022
e407b97
remove repetition of "data"
Jun 21, 2022
c50f80c
run cargo fmt
Jun 27, 2022
1dab056
Merge branch 'main' of github.com:bevyengine/bevy
Jun 28, 2022
b974929
Add documentation for UniformBuffer and DynamicUniformBuffer.
Jul 6, 2022
8a49be6
run cargo fmt
Jul 6, 2022
919c444
Merge branch 'bevyengine:main' into main
Jul 6, 2022
f77cd5a
Clarifications regarding dynamic vs. non-dynamic buffers. Also added…
Jul 6, 2022
ecfeac0
Merge branch 'main' of github.com:bzm3r/bevy
Jul 6, 2022
908bd6a
clarifying docs further.
Jul 8, 2022
d275294
say 'dynamic storage buffer', instead of only storage buffer, in dyna…
Jul 8, 2022
d34311f
addressing suggestions by @superdump (robswain)
Jul 11, 2022
e77771e
Update crates/bevy_render/src/render_resource/uniform_buffer.rs
Jul 20, 2022
8cb0f04
Update crates/bevy_render/src/render_resource/uniform_buffer.rs
Jul 20, 2022
d4c1761
Update crates/bevy_render/src/render_resource/storage_buffer.rs
Jul 20, 2022
a109dad
Update crates/bevy_render/src/render_resource/storage_buffer.rs
Jul 20, 2022
d5bf796
Update crates/bevy_render/src/render_resource/storage_buffer.rs
Jul 20, 2022
4ff1a55
Update crates/bevy_render/src/render_resource/uniform_buffer.rs
Jul 20, 2022
fb5001a
strip down docs a bit, to avoid repetition, and provide a cohesive se…
Jul 20, 2022
15f7033
allow doc_markdown clippy lint so that we can write WebGL2 without ba…
Jul 20, 2022
211ca7c
Merge branch 'bevyengine:main' into main
Jul 21, 2022
6c9754f
Merge branch 'main' of github.com:bzm3r/bevy
Aug 28, 2022
b2b87f8
Documenting and related concepts.
Aug 28, 2022
6881927
cargo fmt
Aug 28, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 35 additions & 1 deletion crates/bevy_render/src/camera/projection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -131,14 +131,16 @@ impl Default for PerspectiveProjection {
#[derive(Debug, Clone, Reflect, FromReflect, Serialize, Deserialize)]
#[reflect(Serialize, Deserialize)]
pub enum WindowOrigin {
/// `(0, 0, 0)` is at the center of the screen.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
/// `(0, 0, 0)` is at the center of the screen.
/// `(0, 0)` is at the center of the screen.

This affects only 2 dimensions.

Center,
/// `(0, 0, 0)` is at the bottom left of the screen.
BottomLeft,
}

#[derive(Debug, Clone, Reflect, FromReflect, Serialize, Deserialize)]
#[reflect(Serialize, Deserialize)]
pub enum ScalingMode {
/// Manually specify left/right/top/bottom values.
/// Manually specify `left`/`right`/`top`/`bottom` values.
/// Ignore window resizing; the image will stretch.
None,
/// Match the window size. 1 world unit = 1 pixel.
Expand All @@ -154,17 +156,49 @@ pub enum ScalingMode {
FixedHorizontal(f32),
}

/// A camera using `OrthographicProjection` displays the world within a rectangular prism (the "viewport", the "frustum", or
Copy link
Contributor

Choose a reason for hiding this comment

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

The description should be phrased in the terms of the type itself, not of the camera.

It could be something like: “Projects a 3D space onto a 2D surface using a rectangular prism.”, but I don't know if it is 100% correct since rendering is not my strong point.

The point is that the description of an item should not go outside its scope. You should also isolate the first sentence into its own paragraph.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also "viewport" and "frustum" are two different things, but are used as synonyms here.

/// "field of view"). Objects that do not lie in this prism are culled ("frustum culling"): in other words, they are not displayed.
Copy link
Contributor

Choose a reason for hiding this comment

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

Triangles that fall outside the frustum are naturally not displayed, since they can't be projected. Frustum culling is just an algorithm that optimizes this process by not even trying to render certain things that are certainly out of sight.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes this is unrelated to the projection component. This should probably be deleted.

/// The size of the field of view is specified in "world units".
///
/// The camera displays objects in its field of view within a 2D window on the computer screen. If the window is smaller
/// than the field of view, then some objects will naturally not appear on the screen. The relationship between the field of view
/// and the window is governed by [`window_origin`].
Comment on lines +164 to +165
Copy link
Contributor

@sarkahn sarkahn Aug 29, 2022

Choose a reason for hiding this comment

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

That last sentence feels a bit off to me - aside from it being a bit vague, I think the purpose of window_origin should be explained within the struct or a related function, not in the general overview.

///
/// Note also that the larger the field of view, the smaller the objects in the field of view will appear, and vice versa.
#[derive(Component, Debug, Clone, Reflect, FromReflect)]
#[reflect(Component, Default)]
pub struct OrthographicProjection {
/// The location of the left face of the camera's field of view, relative to the origin specified in [`window_origin`], in
Copy link
Contributor

Choose a reason for hiding this comment

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

"field of view" generally refers to the concept about how wide an angle the camera uses. Here we're talking about planes/faces, which is more about the frustum I think.

/// world units.
///
/// If [`scaling_mode`] is not `None`, then `left` will be updated as appropriate based on the size of the window.
pub left: f32,
/// The location of the right face of the camera's field of view, relative to the origin specified in [`window_origin`], in
/// world units.
///
/// If [`scaling_mode`] is not `None`, then `left` will be updated as appropriate based on the size of the window.
pub right: f32,
/// The location of the bottom face of the camera's field of view, relative to the origin specified in [`window_origin`], in
/// world units.
///
/// If [`scaling_mode`] is not `None`, then `right` will be updated as appropriate based on the size of the window.
pub bottom: f32,
/// The location of the top face of the camera's field of view, relative to the origin specified in [`window_origin`], in
/// world units.
///
/// If [`scaling_mode`] is not `None`, then `bottom` will be updated as appropriate based on the size of the window.
pub top: f32,
/// The location of the face (in the z-direction) nearest to the camera, in world units.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
/// The location of the face (in the z-direction) nearest to the camera, in world units.
/// The distance of the clipping plane (in the z-direction) nearest to the camera, in world units.

pub near: f32,
/// The location of the face (in the z-direction) farthest from the camera, in world units.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
/// The location of the face (in the z-direction) farthest from the camera, in world units.
/// The distance of the clipping plane (in the z-direction) farthest from the camera, in world units.

Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't "furthest" more common than "farthest"?

pub far: f32,
/// Specifies where `(0, 0, 0)` is located.
Copy link
Contributor

Choose a reason for hiding this comment

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

This does not really help comprehension I find. The "origin" is (0,0,0) so the comment is kind of redundant. It would be better to explain what this affects and how to configure it, maybe with an example.

pub window_origin: WindowOrigin,
/// Specifies how `left`, `right`, `bottom`, and `top` are updated given the window size.
pub scaling_mode: ScalingMode,
/// The value by which `left`, `right`, `bottom`, and `top` are multiplied (scaled). The smaller the `scale`, the smaller the
/// field of view, and thus, the *greater* the magnification of objects in the field of view. The larger the `scale`, the
/// larger the field of view, and thus the *smaller* the magnification of objects in the field of view.
Comment on lines +199 to +201
Copy link
Member

Choose a reason for hiding this comment

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

Maybe this is too specific, but it would be nice to note somewhere that this is what people will typically want to manipulate to achieve a "camera zoom" effect.

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we can put a doc alias here

pub scale: f32,
}

Expand Down