Skip to content

Add import support for EXT_lights_area#17146

Merged
sebavan merged 1 commit intoBabylonJS:masterfrom
MiiBond:mbond/EXT_lights_area
Sep 15, 2025
Merged

Add import support for EXT_lights_area#17146
sebavan merged 1 commit intoBabylonJS:masterfrom
MiiBond:mbond/EXT_lights_area

Conversation

@MiiBond
Copy link
Contributor

@MiiBond MiiBond commented Sep 12, 2025

Added importing support for first version of EXT_light_area extension.
KhronosGroup/glTF#2525

@bjsplat
Copy link
Collaborator

bjsplat commented Sep 12, 2025

Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s).
To prevent this PR from going to the changelog marked it with the "skip changelog" label.

Copy link
Member

@sebavan sebavan left a comment

Choose a reason for hiding this comment

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

Would be awesome to add a test

@bjsplat
Copy link
Collaborator

bjsplat commented Sep 12, 2025

@bjsplat
Copy link
Collaborator

bjsplat commented Sep 12, 2025

@bjsplat
Copy link
Collaborator

bjsplat commented Sep 12, 2025

@sebavan
Copy link
Member

sebavan commented Sep 12, 2025

@SergioRZMasson would be great to add the extra shapes, at least the disc ?

@sebavan sebavan requested a review from bghgary September 15, 2025 13:39
@sebavan
Copy link
Member

sebavan commented Sep 15, 2025

cc @bghgary for review

@sebavan
Copy link
Member

sebavan commented Sep 15, 2025

Let me merge and worst case @bghgary will review after :-)

@sebavan sebavan merged commit a386e27 into BabylonJS:master Sep 15, 2025
19 checks passed
Copy link
Contributor

@bghgary bghgary left a comment

Choose a reason for hiding this comment

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

Obviously, this is a late review. I have a small comment, and we really should put in a visual test to ensure this extension continues to work.

}

/**
* [Specification](https://github.com/KhronosGroup/glTF/blob/main/extensions/2.0/Vendor/EXT_lights_area/README.md)
Copy link
Contributor

Choose a reason for hiding this comment

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

Definitely should be marked experimental as the spec hasn't been merged yet. The link also doesn't work.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good call. I'll fix this and add a test when I update this importer to match the new spec.

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.

4 participants