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.

Sign up for GitHub

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

Reintroduce light intensity scale factor removed in THREE r165 after WebGLRenderer.useLegacyLights deprecation (fix #5556, #5546) #5577

Closed
wants to merge 1 commit into from
Closed

Reintroduce light intensity scale factor removed in THREE r165 after WebGLRenderer.useLegacyLights deprecation (fix #5556, #5546) #5577

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 2 additions & 2 deletions docs/components/light.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ Directional lights are the most efficient type for adding realtime shadows to a
<a-light type="directional" light="castShadow:true;" position="1 1 1" intensity="0.5" shadow-camera-automatic="#objects"></a-light>
```

The `shadow-camera-automatic` configuration maps to `light.shadowCameraAutomatic` which tells the light to automatically update the shadow camera to be the minimum size and position to encompass the target elements.
The `shadow-camera-automatic` configuration maps to `light.shadowCameraAutomatic` which tells the light to automatically update the shadow camera to be the minimum size and position to encompass the target elements.

### Hemisphere

Expand Down Expand Up @@ -129,7 +129,7 @@ lit.

| Property | Description | Default Value |
|-------------|------------------------------------------------------------------------------------------------------------|---------------|
| decay | Amount the light dims along the distance of the light. | 1.0 |
| decay | Amount the light dims along the distance of the light. | 2.0 |
| distance | Distance where intensity becomes 0. If `distance` is `0`, then the point light does not decay with distance. | 0.0 |

### Spot
Expand Down
8 changes: 6 additions & 2 deletions src/components/light.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ module.exports.Component = registerComponent('light', {
color: {type: 'color', if: {type: ['ambient', 'directional', 'hemisphere', 'point', 'spot']}},
envMap: {default: '', if: {type: ['probe']}},
groundColor: {type: 'color', if: {type: ['hemisphere']}},
decay: {default: 1, if: {type: ['point', 'spot']}},
decay: {default: 2, if: {type: ['point', 'spot']}},
Copy link
Contributor

Choose a reason for hiding this comment

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

Why did you change decay?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is @mrxz suggestion in #5546 (comment)

distance: {default: 0.0, min: 0, if: {type: ['point', 'spot']}},
intensity: {default: 1.0, min: 0, if: {type: ['ambient', 'directional', 'hemisphere', 'point', 'spot', 'probe']}},
penumbra: {default: 0, min: 0, max: 1, if: {type: ['spot']}},
Expand Down Expand Up @@ -281,7 +281,11 @@ module.exports.Component = registerComponent('light', {
var distance = data.distance;
var groundColor = new THREE.Color(data.groundColor);
groundColor = groundColor.getHex();
var intensity = data.intensity;
// WebGLRenderer.useLegacyLights has been removed in THREE r165
// Lights intensity had an implicit scaleFactor of PI now removed.
// It's reintroduced here since it's easier to think about integers vs multiples of PI.
var intensityScaleFactor = Math.PI;
var intensity = data.intensity * intensityScaleFactor;
Copy link
Contributor

Choose a reason for hiding this comment

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

It shouldn't change the intensity for scene that used renderer="physicallyCorrectLights: true" already.

Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure I understand the comment. Without the factor scene is dark. What should we do?

Copy link
Contributor

Choose a reason for hiding this comment

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

We have aframe experiences with renderer="physicallyCorrectLights:true" and light intensity that was carefully set, those scenes are not dark, we don't want the intensity to change for those.

var type = data.type;
var target = data.target;
var light = null;
Expand Down
Loading