-
-
Notifications
You must be signed in to change notification settings - Fork 36.1k
Fixing IESLoader and IESSpotLight #32339
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
base: dev
Are you sure you want to change the base?
Conversation
📦 Bundle sizeFull ESM build, minified and gzipped.
🌳 Bundle size after tree-shakingMinimal build including a renderer, camera, empty scene, and dependencies.
|
There was a problem hiding this 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 |
Copilot
AI
Nov 25, 2025
There was a problem hiding this comment.
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.
| // 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 |
| } | ||
|
|
||
| _self.candelaValues = candelaValues.concat( _self.candelaValues ); | ||
| _self.horAngles = horAngles.concat( _self.horAngles ); |
Copilot
AI
Nov 25, 2025
There was a problem hiding this comment.
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.
| _self.horAngles = horAngles.concat( _self.horAngles ); | |
| _self.horAngles = horAngles.concat( _self.horAngles ); | |
| _self.numHorAngles = _self.horAngles.length; |
| function _unrollTypeB() { | ||
|
|
||
| console.log( 'Type B : this type is not supported correctly, sorry.' ); |
Copilot
AI
Nov 25, 2025
There was a problem hiding this comment.
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.
| 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.' ); |
| } | ||
|
|
||
| _getIESValues( iesLamp, type ) { | ||
|
|
Copilot
AI
Nov 25, 2025
There was a problem hiding this comment.
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.
| /** | |
| * 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. | |
| */ |
| const vecLight = lightPosition( light ).sub( builder.context.positionWorld || positionWorld ).normalize(); | ||
|
|
||
| const vecTargetToLight = lightPosition( light ).sub( lightTargetPosition( light ) ).normalize(); | ||
| const vecTarget = lightTargetPosition( light ).normalize(); |
Copilot
AI
Nov 25, 2025
There was a problem hiding this comment.
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.
| const vecTarget = lightTargetPosition( light ).normalize(); | |
| const vecTarget = lightTargetPosition( light ).sub( lightPosition( light ) ).normalize(); |
| 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 ); |
Copilot
AI
Nov 25, 2025
There was a problem hiding this comment.
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:
- The
atanmethod is being called with two arguments, but this should beatan2. The single-argumentatanfunction returns values in the range [-π/2, π/2], whileatan2returns values in the full range [-π, π], which is necessary for properly computing the rotation angle around a full circle. - Adding
Math.PI * 2unconditionally 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 )afteratan2to normalize the angle to the range [0, 2π).
| } | ||
|
|
||
| } | ||
|
|
Copilot
AI
Nov 25, 2025
There was a problem hiding this comment.
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.
| /** | |
| * 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. | |
| */ |
| _self.inputWatts = Number( values[ 2 ] ); | ||
|
|
||
| } | ||
|
|
Copilot
AI
Nov 25, 2025
There was a problem hiding this comment.
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.
| /** | |
| * 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. | |
| */ |
|
|
||
| _unrollTypeC(); | ||
|
|
||
| } else if ( _self.gonioType == 3 ) { |
Copilot
AI
Nov 25, 2025
There was a problem hiding this comment.
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.
| } else if ( _self.gonioType == 3 ) { | |
| } else if ( _self.gonioType === 3 ) { |
| } | ||
|
|
||
| } | ||
|
|
Copilot
AI
Nov 25, 2025
There was a problem hiding this comment.
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.
| /** | |
| * 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. | |
| */ |
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.