Skip to content

Conversation

@Wagyx
Copy link

@Wagyx Wagyx commented Nov 23, 2025

Hello,

This PR fixes some inconsistencies in the IESLoader and enables the use of asymmetric IES files.
Related topic are #24409 #25238
If you want to test it you may download any file from the IESLibrary.
As a check, you can view a fully physical implementation of direct lighting on LIV which I made by baking the light into textures.

@github-actions
Copy link

📦 Bundle size

Full ESM build, minified and gzipped.

Before After Diff
WebGL 358.3
86.96
358.3
86.96
+0 B
+0 B
WebGPU 620.54
174.18
620.76
174.26
+222 B
+82 B
WebGPU Nodes 619.14
173.92
619.37
174
+222 B
+82 B

🌳 Bundle size after tree-shaking

Minimal build including a renderer, camera, empty scene, and dependencies.

Before After Diff
WebGL 490.25
121.78
490.25
121.78
+0 B
+0 B
WebGPU 691.41
189.78
691.64
189.86
+227 B
+77 B
WebGPU Nodes 632.86
172.96
633.08
173.04
+227 B
+86 B

@sunag sunag requested a review from Copilot November 25, 2025 17:07
Copilot finished reviewing on behalf of sunag November 25, 2025 17:12
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes inconsistencies in the IESLoader and enables support for asymmetric IES files by adding azimuthal angle support to the spot light system. The changes introduce proper 2D texture sampling for IES data and add coordinate transformation functions to handle different IES goniometric types.

  • Added azimuthal angle parameter to spot light attenuation calculations
  • Refactored IESLoader to generate 2D textures with proper azimuthal and inclination coordinates
  • Implemented unroll functions to handle IES Types A, B, and C goniometric coordinate systems

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 23 comments.

File Description
src/nodes/lighting/SpotLightNode.js Added azimuthal angle calculation and parameter to getSpotAttenuation method to support asymmetric IES profiles
src/nodes/lighting/IESSpotLightNode.js Updated to use 2D texture lookup with both azimuthal and inclination coordinates, and pass azimuthal angle to parent class
examples/jsm/loaders/IESLoader.js Major refactoring to generate 2D IES textures, added coordinate transformation functions for different goniometric types, and fixed candela value calculation bug

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.


if ( deltaPhi === 0 ) // Outside range
return 0;
// compute the best resolution for the IES texture based on the minium sampling angle
Copy link

Copilot AI Nov 25, 2025

Choose a reason for hiding this comment

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

The comment misspells "minimum" as "minium". Consider fixing this typo for clarity.

Suggested change
// compute the best resolution for the IES texture based on the minium sampling angle
// compute the best resolution for the IES texture based on the minimum sampling angle

Copilot uses AI. Check for mistakes.
}

_self.candelaValues = candelaValues.concat( _self.candelaValues );
_self.horAngles = horAngles.concat( _self.horAngles );
Copy link

Copilot AI Nov 25, 2025

Choose a reason for hiding this comment

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

After concatenating new values to _self.candelaValues and _self.horAngles, the _self.numHorAngles should be updated to reflect the new array length. Currently, it remains unchanged, which will cause the rest of the code to use an incorrect count. Consider adding _self.numHorAngles = _self.horAngles.length; after line 341.

Suggested change
_self.horAngles = horAngles.concat( _self.horAngles );
_self.horAngles = horAngles.concat( _self.horAngles );
_self.numHorAngles = _self.horAngles.length;

Copilot uses AI. Check for mistakes.
Comment on lines +326 to +328
function _unrollTypeB() {

console.log( 'Type B : this type is not supported correctly, sorry.' );
Copy link

Copilot AI Nov 25, 2025

Choose a reason for hiding this comment

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

The _unrollTypeB function lacks documentation. Consider adding a JSDoc comment explaining what "Type B" refers to in the IES specification and what transformation this function performs on the goniometric data. Additionally, the console.log message indicates this type is "not supported correctly" - this warning should be documented more formally.

Suggested change
function _unrollTypeB() {
console.log( 'Type B : this type is not supported correctly, sorry.' );
/**
* Handles "Type B" goniophotometric data as defined in the IES photometric file specification.
*
* In the IES standard, "Type B" refers to a specific photometric distribution where the horizontal angles
* are measured in the plane of the luminaire, and the vertical angles are measured perpendicular to it.
* This function attempts to unroll or mirror the goniometric data for Type B distributions, but the current
* implementation is incomplete and may not produce correct results for all Type B IES files.
*
* @see https://ies.org/standards/ for more information on IES file types.
* @private
*/
function _unrollTypeB() {
console.warn( 'IESLoader: Type B goniophotometric data is not fully supported. The current implementation may produce incorrect results. Please verify the output or consider using Type A or C data if possible.' );

Copilot uses AI. Check for mistakes.
}

_getIESValues( iesLamp, type ) {

Copy link

Copilot AI Nov 25, 2025

Choose a reason for hiding this comment

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

The findIndex function lacks documentation. Consider adding a JSDoc comment explaining that this function performs a binary search-like operation to find the index of the largest angle value that is less than or equal to the given value, for use in interpolation.

Suggested change
/**
* Finds the index of the largest angle value in the array that is less than or equal to the given value.
* This is used to locate the interval for interpolation.
*
* @param {number[]} angles - Array of angle values (assumed sorted in ascending order).
* @param {number} value - The value to locate within the angles array.
* @returns {number} The index of the largest angle less than or equal to the value, or the last valid index.
*/

Copilot uses AI. Check for mistakes.
const vecLight = lightPosition( light ).sub( builder.context.positionWorld || positionWorld ).normalize();

const vecTargetToLight = lightPosition( light ).sub( lightTargetPosition( light ) ).normalize();
const vecTarget = lightTargetPosition( light ).normalize();
Copy link

Copilot AI Nov 25, 2025

Choose a reason for hiding this comment

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

The vecTarget is computed as lightTargetPosition( light ).normalize(), which normalizes the target position directly. This treats the target position as a direction vector from the origin, which may not be the intended behavior. If the target position is meant to represent an absolute position in world space (not relative to the origin), this normalization would be incorrect. Consider whether this should be lightTargetPosition( light ).sub( someOrigin ).normalize() or if you need a different calculation for the target direction.

Suggested change
const vecTarget = lightTargetPosition( light ).normalize();
const vecTarget = lightTargetPosition( light ).sub( lightPosition( light ) ).normalize();

Copilot uses AI. Check for mistakes.
const vecTarget = lightTargetPosition( light ).normalize();
const vecRight = vecTarget.cross( vecTargetToLight ).normalize();
const vecUp = vecTargetToLight.cross( vecRight ).normalize();
const rotationAngle = vecLight.dot( vecRight ).atan( vecLight.dot( vecUp ) ).add( Math.PI * 2 );
Copy link

Copilot AI Nov 25, 2025

Choose a reason for hiding this comment

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

There are two issues with this rotation angle calculation:

  1. The atan method is being called with two arguments, but this should be atan2. The single-argument atan function returns values in the range [-π/2, π/2], while atan2 returns values in the full range [-π, π], which is necessary for properly computing the rotation angle around a full circle.
  2. Adding Math.PI * 2 unconditionally will shift all angles into the range [2π, 4π). If the intention is to ensure the angle is positive, consider using .mod( Math.PI * 2 ) after atan2 to normalize the angle to the range [0, 2π).

Copilot uses AI. Check for mistakes.
}

}

Copy link

Copilot AI Nov 25, 2025

Choose a reason for hiding this comment

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

The _unrollTypeC function lacks documentation. Consider adding a JSDoc comment explaining what "Type C" refers to in the IES specification and what transformation this function performs on the goniometric data.

Suggested change
/**
* Unrolls or expands Type C goniometric data as defined in the IES specification.
*
* Type C photometric data is the most common format for luminaires with rotational symmetry,
* where horizontal angles represent azimuthal positions (0–360°) around the luminaire.
* This function ensures that the horizontal angles and corresponding candela values
* are properly expanded to cover the full 0–360° range, handling cases where the data
* is only partially specified (e.g., up to 90° or 180°). The transformation duplicates
* and mirrors data as needed to create a complete set of horizontal angles and candela values.
*/

Copilot uses AI. Check for mistakes.
_self.inputWatts = Number( values[ 2 ] );

}

Copy link

Copilot AI Nov 25, 2025

Choose a reason for hiding this comment

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

The _unrollTypeA function lacks documentation. Consider adding a JSDoc comment explaining what "Type A" refers to in the IES specification and what transformation this function performs on the goniometric data.

Suggested change
/**
* Adjusts goniometric data for IES "Type A" photometric files.
*
* In the IES specification, "Type A" refers to a goniophotometric measurement system
* where the lamp is oriented such that the 0-degree horizontal angle is aligned with
* the lamp's major axis. This function mirrors the horizontal angles and candela values
* to account for the symmetry of Type A data, and shifts all vertical angles by 90 degrees
* to align with the coordinate system expected by rendering engines.
*
* This transformation ensures that the photometric data is correctly oriented for visualization.
*/

Copilot uses AI. Check for mistakes.

_unrollTypeC();

} else if ( _self.gonioType == 3 ) {
Copy link

Copilot AI Nov 25, 2025

Choose a reason for hiding this comment

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

Using loose equality (==) for numeric comparisons can lead to unexpected behavior. Consider using strict equality (===) to ensure type-safe comparisons, especially when comparing with numeric literals.

Suggested change
} else if ( _self.gonioType == 3 ) {
} else if ( _self.gonioType === 3 ) {

Copilot uses AI. Check for mistakes.
}

}

Copy link

Copilot AI Nov 25, 2025

Choose a reason for hiding this comment

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

The unroll function lacks documentation. Consider adding a JSDoc comment explaining that this function transforms the IES goniometric data based on the gonioType (1, 2, or 3) to handle different coordinate systems specified in the IES format.

Suggested change
/**
* Transforms the IES goniometric data based on the gonioType (1, 2, or 3).
* This function handles the different coordinate systems specified in the IES format:
* - Type C (1): Most common, uses standard polar coordinates.
* - Type A (3): Used for automotive and other applications.
* - Type B (2): Less common, not fully supported.
* The function modifies the internal candela values and angle arrays to match
* the expected coordinate system for rendering.
*/

Copilot uses AI. Check for mistakes.
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.

1 participant