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

Author Light component reference #546

Merged
merged 11 commits into from
Jun 23, 2021
Merged

Author Light component reference #546

merged 11 commits into from
Jun 23, 2021

Conversation

chanmosq
Copy link
Contributor

No description provided.

Signed-off-by: Chanelle Mosquera <chanmosq@amazon.com>
Signed-off-by: Chanelle Mosquera <chanmosq@amazon.com>
Signed-off-by: Chanelle Mosquera <chanmosq@amazon.com>
@chanmosq chanmosq self-assigned this Jun 10, 2021
Signed-off-by: Chanelle Mosquera <chanmosq@amazon.com>
@chanmosq chanmosq self-assigned this Jun 12, 2021
content/docs/user-guide/components/reference/atom/light.md Outdated Show resolved Hide resolved
content/docs/user-guide/components/reference/atom/light.md Outdated Show resolved Hide resolved
|-|-|-|-|
| **Enable shadow** | Enable shadow effects. | Boolean | `Disabled` |
| **Shadowmap size** | Set the width and height of the shadowmap. A higher size leads to a more detailed shadow effect.| `256`, `512`, `1024`, `2048` | `256` |
| **Shadow filter method** | Set the shadow filtering method to reduce aliasing in the shadow map. The supported methods are Percentage-Closer Filtering (PCF), Exponential Shadow Maps (ESM), and a combination of both. | `None`, `PCF`, `ESM`, `ESM+PCF` | `None` |
Copy link
Contributor

Choose a reason for hiding this comment

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

With regards to ESM+PCF, I would state that the way it works is that "ESM is used for the primary algorithm, but it falls back to PCF in areas where ESM would fail"

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 updated the description to

Set the shadow filtering method to reduce aliasing in the shadow map. The supported methods are Percentage-Closer Filtering (PCF) or Exponential Shadow Maps (ESM). ESM+PCF uses ESM, but falls back to PCF in areas where ESM might fail.

Signed-off-by: Chanelle Mosquera <chanmosq@amazon.com>
Copy link
Contributor

@invertednormal invertednormal 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 one thing that's missing is a more general explanation of what each type of light is. Doesn't need a long explanation, but just something that goes beyond the shape. This possibly could be combined with the current bullet list at the top to give a brief explanation of each type plus the things already pointed out.

Point (Sphere) - Emits light from the surface of a sphere. Best for emulating lights that emit in all directions like standard light bulbs.

Point (Simple Punctual) - Emits light from a single point in space. Less photorealistic than a sphere light, but cheaper.

Spot (Disk) - Emits light from a 2d circle in 3d space. It is a good choice to emulate can lights and spot lights. Shutters can be added to constrain the light to a cone.

Spot (Simple Punctual) - Emits light from a single point in space constrained to a cone. Less Photorealistic than a Spot (Disk) light, but cheaper.

Quad - Emits light from the surface of a rectangle in 3D space. Good for lighting a larger area with diffuse light. By default uses linearly transformed cosines for an accurate result, but also supports a fast approximation mode that is only about 5x as expensive as a point light.

Polygon - Emits light from an arbitrarily shaped 2d polygon in 3d space. This is the most expensive light type, but produces very realistic area lighting. The polygon can have up to 64 points, but increases with cost as more points as added.

Signed-off-by: Chanelle Mosquera <chanmosq@amazon.com>
@chanmosq chanmosq requested a review from paucom June 17, 2021 18:36
Signed-off-by: Chanelle Mosquera <chanmosq@amazon.com>
Comment on lines 40 to 46
| Point (sphere) | [Sphere Shape component](\docs\user-guide\components\reference\shape\sphere-shape.md) |
| Point (simple punctual) | - |
| Spot (disk) | [Disk Shape component](\docs\user-guide\components\reference\shape\disk-shape.md) |
| Spot (simple punctual) | - |
| Capsule | [Capsule Shape component](\docs\user-guide\components\reference\shape\capsule-shape.md) |
| Quad | [Quad Shape component](\docs\user-guide\components\reference\shape\quad-shape.md) |
| Polygon | [Polygon Prism Shape component](\docs\user-guide\components\reference\shape\polygon-prism-shape.md) |
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
| Point (sphere) | [Sphere Shape component](\docs\user-guide\components\reference\shape\sphere-shape.md) |
| Point (simple punctual) | - |
| Spot (disk) | [Disk Shape component](\docs\user-guide\components\reference\shape\disk-shape.md) |
| Spot (simple punctual) | - |
| Capsule | [Capsule Shape component](\docs\user-guide\components\reference\shape\capsule-shape.md) |
| Quad | [Quad Shape component](\docs\user-guide\components\reference\shape\quad-shape.md) |
| Polygon | [Polygon Prism Shape component](\docs\user-guide\components\reference\shape\polygon-prism-shape.md) |
| Point (sphere) | [Sphere Shape](\docs\user-guide\components\reference\shape\sphere-shape.md) |
| Point (simple punctual) | - |
| Spot (disk) | [Disk Shape](\docs\user-guide\components\reference\shape\disk-shape.md) |
| Spot (simple punctual) | - |
| Capsule | [Capsule Shape](\docs\user-guide\components\reference\shape\capsule-shape.md) |
| Quad | [Quad Shape](\docs\user-guide\components\reference\shape\quad-shape.md) |
| Polygon | [Polygon Prism Shape](\docs\user-guide\components\reference\shape\polygon-prism-shape.md) |

I recommend cutting "component" from the dependencies' link text in each row to minimize redundancy. If you think that it's important to keep "component," I recommend moving the word outside of the link text. (I see that the titles of the linked topics include "component," but I think that we should edit those titles to remove the redundant word "component" there, too.)

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've omitted the trailing "component" in each row. The pages in the component references have inconsistent styling: some have the trailing "component" in the page title, while others don't. However, we should opt to have "component" be in the title for better SEO (search engine optimization). The same styling follows for the Gem reference.

To summarize, page titles for the component and Gem reference should be:

  • Component
linkTitle: Component Name
title: Component Name Component
  • Gem
linkTitle: Gem Name
title: Gem Name Gem

I created a task for that here: #568

Copy link
Contributor

Choose a reason for hiding this comment

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

@chanmosq Thank you for pointing that out. I noticed that inconsistency and I agree with all of your points. I appreciate the correction for SEO considerations, as well as your creation of PR #568 for that cleanup effort.

| Property | Description | Values | Default |
|-|-|-|-|
| **Enable shadow** | Enable shadow effects. | Boolean | `Disabled` |
| **Shadowmap size** | Set the width and height of the shadowmap. A higher size leads to a more detailed shadow effect.| `256`, `512`, `1024`, `2048` | `256` |
Copy link
Contributor

@paucom paucom Jun 18, 2021

Choose a reason for hiding this comment

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

Suggested change
| **Shadowmap size** | Set the width and height of the shadowmap. A higher size leads to a more detailed shadow effect.| `256`, `512`, `1024`, `2048` | `256` |
| **Shadowmap size** | Set the width and height of the shadowmap. A greater size results in a more detailed shadow effect. | `256`, `512`, `1024`, `2048` | `256` |
  • Revised wording for accuracy, flow.
  • Is "shadowmap" a widely understood term? If not, consider adding a definition here, as is done for "attenuation" earlier.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The shadowmap texture contains information about the light and objects in an area of the scene. It is used for Shadow Mapping, a technique that renders shadows in the scene.

I've added this brief explanation in the description.

Copy link
Contributor

@paucom paucom left a comment

Choose a reason for hiding this comment

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

In addition to my various line-level comments, be sure to replace the .jpg images with .png images, per our style guidance. ("Use the PNG format (.png) for all screenshots.")

Signed-off-by: Chanelle Mosquera <chanmosq@amazon.com>
@chanmosq chanmosq requested a review from paucom June 21, 2021 17:48
Copy link
Contributor

@paucom paucom left a comment

Choose a reason for hiding this comment

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

Some small, non-blocking cleanup edits. Other than these suggested changes, I'm ready to approve.

Comment on lines 40 to 46
| Point (sphere) | [Sphere Shape component](\docs\user-guide\components\reference\shape\sphere-shape.md) |
| Point (simple punctual) | - |
| Spot (disk) | [Disk Shape component](\docs\user-guide\components\reference\shape\disk-shape.md) |
| Spot (simple punctual) | - |
| Capsule | [Capsule Shape component](\docs\user-guide\components\reference\shape\capsule-shape.md) |
| Quad | [Quad Shape component](\docs\user-guide\components\reference\shape\quad-shape.md) |
| Polygon | [Polygon Prism Shape component](\docs\user-guide\components\reference\shape\polygon-prism-shape.md) |
Copy link
Contributor

Choose a reason for hiding this comment

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

@chanmosq Thank you for pointing that out. I noticed that inconsistency and I agree with all of your points. I appreciate the correction for SEO considerations, as well as your creation of PR #568 for that cleanup effort.

content/docs/user-guide/components/reference/atom/light.md Outdated Show resolved Hide resolved
content/docs/user-guide/components/reference/atom/light.md Outdated Show resolved Hide resolved
content/docs/user-guide/components/reference/atom/light.md Outdated Show resolved Hide resolved
content/docs/user-guide/components/reference/atom/light.md Outdated Show resolved Hide resolved
content/docs/user-guide/components/reference/atom/light.md Outdated Show resolved Hide resolved
content/docs/user-guide/components/reference/atom/light.md Outdated Show resolved Hide resolved
Signed-off-by: Chanelle Mosquera <chanmosq@amazon.com>
@chanmosq chanmosq requested a review from paucom June 23, 2021 19:08
@paucom paucom closed this Jun 23, 2021
@paucom paucom reopened this Jun 23, 2021
@chanmosq chanmosq removed the request for review from micronAMZN June 23, 2021 19:26
@willihay willihay merged commit 47937a6 into o3de:main Jun 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants