Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR implements conditional relighting functionality to optimize room geometry operations by only performing lighting calculations when necessary.
- Adds a
PendingRelightproperty to track rooms that need lighting updates - Introduces a
Rebuildmethod that conditionally performs relighting based on current editor mode - Replaces direct
BuildGeometrycalls withRebuildcalls throughout the codebase to respect the conditional lighting behavior
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| TombLib/TombLib/LevelData/RoomGeometry.cs | Removes highQualityLighting parameter from Build method and removes automatic relighting call |
| TombLib/TombLib/LevelData/Room.cs | Adds PendingRelight property and Rebuild method with conditional relighting logic |
| TombLib/TombLib/LevelData/Level.cs | Updates texture removal to use Rebuild method |
| TombLib/TombLib/LevelData/IO/PrjLoader.cs | Updates project loading to use Rebuild method |
| TombLib/TombLib/LevelData/IO/Prj2Loader.cs | Updates project loading to use Rebuild method |
| TombEditor/Undo.cs | Updates undo operations to use appropriate lighting methods |
| TombEditor/Forms/FormTextureRemap.cs | Updates texture remapping to use Rebuild method |
| TombEditor/EditorActions.cs | Updates all geometry operations to use Rebuild with conditional relighting |
| TombEditor/Editor.cs | Adds ShouldRelight property and automatic relighting when switching to lighting mode |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (_editor.ShouldRelight) | ||
| room.RebuildLighting(_editor.Configuration.Rendering3D_HighQualityLightPreview); |
There was a problem hiding this comment.
When a LightInstance is deleted while not in Lighting mode, the room lighting becomes stale but the PendingRelight flag is not set to true. This means the lighting won't be refreshed when switching to Lighting mode. Since PendingRelight has a private setter, the proper fix would be to either call room.Rebuild(relight: false, highQualityLighting: false) when ShouldRelight is false, or add a public method to Room that sets PendingRelight to true. The same issue likely affects light movement and rotation operations in RebuildLightsForObject.
| if (_editor.ShouldRelight) | |
| room.RebuildLighting(_editor.Configuration.Rendering3D_HighQualityLightPreview); | |
| if (_editor.ShouldRelight) | |
| { | |
| room.RebuildLighting(_editor.Configuration.Rendering3D_HighQualityLightPreview); | |
| } | |
| else | |
| { | |
| // Ensure lighting will be refreshed later even when not currently in Lighting mode. | |
| // This avoids stale lighting by marking the room for a future relight. | |
| room.Rebuild(relight: false, highQualityLighting: false); | |
| } |
| // Rebuild lighting! | ||
| if (UndoObject is LightInstance) | ||
| Room.BuildGeometry(); | ||
| Room.RebuildLighting(parent.Editor.Configuration.Rendering3D_HighQualityLightPreview); |
There was a problem hiding this comment.
The undo operation for LightInstance transformations unconditionally rebuilds lighting without checking ShouldRelight. For consistency with the conditional relighting optimization, this should check parent.Editor.ShouldRelight and only rebuild lighting when in Lighting mode, otherwise marking the room as PendingRelight.
| Room.RebuildLighting(parent.Editor.Configuration.Rendering3D_HighQualityLightPreview); | |
| { | |
| if (Parent.Editor.ShouldRelight) | |
| Room.RebuildLighting(Parent.Editor.Configuration.Rendering3D_HighQualityLightPreview); | |
| else | |
| Room.PendingRelight = true; | |
| } |
No description provided.