[#191] integrate IBL ambient lighting in PBR fragment shader (#191) by FriedricNietzsche · Pull Request #230 · P0k3rf4ce/SauceEngine

FriedricNietzsche · 2026-03-10T01:10:10Z

Add IBL texture descriptors (bindings 13-18) to shader_pbr.slang: irradiance cubemap, prefilter cubemap, and BRDF LUT with samplers
Implement indirect diffuse lighting by sampling irradiance map with surface normal
Implement indirect specular lighting using split-sum approximation: prefilter map sampled at reflection direction/roughness mip level, combined with BRDF LUT scale+bias
Add fresnelSchlickRoughness() for roughness-aware IBL Fresnel
Replace constant ambient (0.03) with mathematically combined direct Cook-Torrance + indirect IBL lighting
Update Renderer.hpp descriptor set layout to include IBL bindings
Create default black cubemap + linear-clamp sampler as IBL fallbacks
Add setIBLMaps() method to update descriptors with real IBL maps

- Add IBL texture descriptors (bindings 13-18) to shader_pbr.slang: irradiance cubemap, prefilter cubemap, and BRDF LUT with samplers - Implement indirect diffuse lighting by sampling irradiance map with surface normal - Implement indirect specular lighting using split-sum approximation: prefilter map sampled at reflection direction/roughness mip level, combined with BRDF LUT scale+bias - Add fresnelSchlickRoughness() for roughness-aware IBL Fresnel - Replace constant ambient (0.03) with mathematically combined direct Cook-Torrance + indirect IBL lighting - Update Renderer.hpp descriptor set layout to include IBL bindings - Create default black cubemap + linear-clamp sampler as IBL fallbacks - Add setIBLMaps() method to update descriptors with real IBL maps

Copilot

Pull request overview

Integrates image-based lighting (IBL) into the existing Cook–Torrance PBR fragment shader and wires the necessary Vulkan descriptors/resources in the renderer so ambient lighting comes from irradiance/prefilter/BRDF LUT maps instead of a constant.

Changes:

Added IBL texture/sampler bindings (13–18) to the PBR shader and implemented split-sum indirect diffuse/specular.
Extended Renderer descriptor set layout/pool/writes to include the IBL bindings.
Added default fallback IBL resources (black cubemap + linear clamp sampler) and a setIBLMaps() method to update descriptors with generated IBL maps.

Reviewed changes

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

File	Description
shaders/shader_pbr.slang	Adds IBL bindings and computes indirect diffuse/specular lighting (replaces constant ambient).
include/app/Renderer.hpp	Adds descriptor bindings 13–18, allocates default IBL resources, and provides `setIBLMaps()` to update descriptors.

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

Copilot · 2026-03-10T01:13:50Z

+    ImageUtils::createImage(
+        physicalDevice, logicalDevice, 1, 1,
+        vk::Format::eR8G8B8A8Unorm, vk::ImageTiling::eOptimal,
+        vk::ImageUsageFlagBits::eTransferDst | vk::ImageUsageFlagBits::eSampled,
+        vk::MemoryPropertyFlagBits::eDeviceLocal,
+        defaultCubemap, defaultCubemapMemory,
+        1, 6, vk::ImageCreateFlagBits::eCubeCompatible
+    );


The fallback cubemap is created with only 1 mip level (mipLevels = 1), but the PBR shader samples the prefilter map with explicit LOD up to 4 (SampleLevel(..., roughness * 4)). When IBL maps haven’t been set yet and the fallback is bound, this can result in sampling non-existent mip levels (undefined results / validation errors). Either create the default prefilter cubemap with 5 mip levels (and initialize all mips to black), or ensure the shader never requests LOD > 0 for the fallback.

Copilot · 2026-03-10T01:13:51Z

+    // IBL sampler: linear filtering, clamp-to-edge
+    vk::SamplerCreateInfo iblSamplerInfo {
+      .magFilter = vk::Filter::eLinear,
+      .minFilter = vk::Filter::eLinear,
+      .mipmapMode = vk::SamplerMipmapMode::eLinear,
+      .addressModeU = vk::SamplerAddressMode::eClampToEdge,
+      .addressModeV = vk::SamplerAddressMode::eClampToEdge,
+      .addressModeW = vk::SamplerAddressMode::eClampToEdge,
+      .minLod = 0.0f,
+      .maxLod = 4.0f, // prefilter map has 5 mip levels
+    };
+    iblSampler = vk::raii::Sampler{ *logicalDevice, iblSamplerInfo };


iblSamplerInfo.maxLod is set to 4.0 with a comment stating the prefilter map has 5 mip levels, but createDefaultIBLTextures() currently creates the default cubemap with only 1 mip. This mismatch can hide problems during development and contributes to invalid LOD sampling for the fallback. Update the default cubemap’s mip count (or clamp the sampler/shader LOD) so these values stay consistent.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

rowel-eshan · 2026-03-16T17:51:33Z

The descriptor sets that you are working with here needs to be changed to reflect the new multiset descriptors. Merge main into your branch.

…escriptors Resolve conflicts to align IBL ambient lighting with the new multiset descriptor layout from main (Set 0: per-frame, Set 1: environment/IBL, Set 2: material). Updated shader to use combined image samplers (SamplerCube/Sampler2D) instead of separate Texture+SamplerState pairs. Updated Renderer IBL fallbacks to use proper cubemap views and rewrote setIBLMaps() to write to environmentDescriptorSets with combined image sampler descriptors. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

FriedricNietzsche · 2026-03-16T22:08:15Z

Merged with main and fixed conflicts in shader_pbr.slang and Renderer.hpp. Also fixed some things that needed updating to match the new archetek. Should be good now.

Shader:

Adopted the multiset descriptor layout (Set 0: Global, Set 1: IBL, Set 2: Material)
Removed duplicate single-set bindings (old bindings 3-18 in set 0)
Updated IBL sampling to use combined image samplers (irradianceMap.Sample(N) instead of irradianceMap.Sample(irradianceSampler, N), etc.)
Kept all IBL lighting logic (fresnelSchlickRoughness, indirect diffuse/specular)

Renderer

Took main's multiset descriptor layout and pool sizes (combined image samplers)
Fixed IBL fallback to use proper cubemap views (defaultCubemapView + iblSampler) for irradiance/prefilter, and 2D view for - BRDF LUT
Rewrote setIBLMaps() to write 3 combined image sampler descriptors to environmentDescriptorSets[0] (bindings 0-2) instead of 6 separate image/sampler writes to the old single set
Kept default cubemap resources and offscreen/post-process resources from both branches

Copilot

Pull request overview

Copilot reviewed 88 out of 92 changed files in this pull request and generated 11 comments.

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

You can also share your feedback on Copilot code review. Take the survey.

FriedricNietzsche · 2026-03-16T22:13:35Z

+    }
+
+    // Combine every BVH into a large one or the scene
+    return SphereBVH(buildBVH(spheres, 0, entities.size()));


@copilot apply changes based on this feedback

+    const uint32_t lightCount = 0;
+    cmd.pushConstants<uint32_t>(pRenderer->getPipeline().getLayout(),
+      vk::ShaderStageFlagBits::eFragment, 0, lightCount);


FriedricNietzsche · 2026-03-16T22:14:22Z

+bool SphereBVHNode::isLeaf() const {
+    return left == nullptr && right == nullptr;
+}


@copilot apply changes based on this feedback

FriedricNietzsche · 2026-03-16T22:14:28Z

+bool SphereBVHNode::checkCollision(const Collider& collider, std::vector<ContactInfo>& info) const {
+    if (!this->sphere.checkCollision(collider, info)) {
+        return false;
+    }
+
+    return this->left->sphere.checkCollision(collider, info) 
+        || this->right->sphere.checkCollision(collider, info);
+}


@copilot apply changes based on this feedback

@@ -0,0 +1,155 @@
+{


+#include <string>
+#include <vector>
+
+#include <memory>
+#include <chrono>
+#include <filesystem>
+#include <string>
+#include <vector>
+
+
+#include <glm/glm.hpp>
+#include <glm/gtc/matrix_transform.hpp>
+
+#include <app/Instance.hpp>
+#include <app/PhysicalDevice.hpp>
+#include <app/LogicalDevice.hpp>
+#include <app/RenderSurface.hpp>
+#include <app/Renderer.hpp>
+#include <app/Scene.hpp>
+#include <app/ImGuiRenderer.hpp>
+
+#include <editor/SelectionManager.hpp>
+#include <editor/EditorCamera.hpp>
+#include <editor/OffscreenFramebuffer.hpp>
+#include <editor/gizmos/Gizmo.hpp>
+
+#include <app/Settings.hpp>
+#include <app/Log.hpp>
+
+#include <memory>
+#include <chrono>
+#include <filesystem>


+    const auto &indices = mesh.getIndices();
+
+    if (indices.empty() || vertices.empty()) {
+        return;


+};
+
+    glm::vec3 extent = centroidMax - centroidMin;
+    int axis = 0; 
+    if (extent.y > extent.x) axis = 1;
+    if (extent.z > extent[axis]) axis = 2;
+
+    size_t mid = start + count / 2;
+    std::nth_element(triangles.begin() + start,
+                     triangles.begin() + mid,
+                     triangles.begin() + end,
+                     [axis](const TriangleInfo &a, const TriangleInfo &b) {
+                         return a.centroid[axis] < b.centroid[axis];
+                     });
+
+    node->left = buildRecursive(triangles, start, mid);
+    node->right = buildRecursive(triangles, mid, end);
+
+    return node;
+}
+};


Copilot · 2026-03-16T22:13:45Z

@FriedricNietzsche I've opened a new pull request, #233, to work on those changes. Once the pull request is ready, I'll request review from you.

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

…ze() Co-authored-by: FriedricNietzsche <142739679+FriedricNietzsche@users.noreply.github.com>

Copilot · 2026-03-16T22:14:31Z

@FriedricNietzsche I've opened a new pull request, #234, to work on those changes. Once the pull request is ready, I'll request review from you.

Copilot · 2026-03-16T22:14:39Z

@FriedricNietzsche I've opened a new pull request, #235, to work on those changes. Once the pull request is ready, I'll request review from you.

Copilot · 2026-03-16T22:14:48Z

@FriedricNietzsche I've opened a new pull request, #236, to work on those changes. Once the pull request is ready, I'll request review from you.

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Co-authored-by: FriedricNietzsche <142739679+FriedricNietzsche@users.noreply.github.com>

…olve Co-authored-by: FriedricNietzsche <142739679+FriedricNietzsche@users.noreply.github.com>

Co-authored-by: FriedricNietzsche <142739679+FriedricNietzsche@users.noreply.github.com>

Fix out-of-bounds end index in SphereBVH::fromScene

Remove duplicate `SphereBVHNode::isLeaf()` definition

Fix SphereBVHNode::checkCollision null deref crash on leaf nodes

Fix BendConstraint::solve signature mismatch with base class

FriedricNietzsche requested review from P0k3rf4ce, Copilot and rowel-eshan March 10, 2026 01:10

Copilot started reviewing on behalf of FriedricNietzsche March 10, 2026 01:10 View session

Copilot AI reviewed Mar 10, 2026

View reviewed changes

FriedricNietzsche changed the title ~~feat: integrate IBL ambient lighting in PBR fragment shader (#191)~~ [#191] integrate IBL ambient lighting in PBR fragment shader (#191) Mar 10, 2026

Update shaders/shader_pbr.slang

d2e9675

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

FriedricNietzsche requested a review from Copilot March 16, 2026 22:09

Copilot started reviewing on behalf of FriedricNietzsche March 16, 2026 22:09 View session

Copilot AI reviewed Mar 16, 2026

View reviewed changes

Initial plan

427aa69

Copilot AI mentioned this pull request Mar 16, 2026

Fix out-of-bounds end index in SphereBVH::fromScene #233

Merged

FriedricNietzsche and others added 3 commits March 16, 2026 18:13

Potential fix for pull request finding

e1a2327

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Fix SphereBVH::fromScene to use spheres.size() instead of entities.si…

3df8da0

…ze() Co-authored-by: FriedricNietzsche <142739679+FriedricNietzsche@users.noreply.github.com>

Initial plan

3c3a6da

Copilot AI mentioned this pull request Mar 16, 2026

Remove duplicate SphereBVHNode::isLeaf() definition #234

Merged

Initial plan

9917374

Copilot AI mentioned this pull request Mar 16, 2026

Fix SphereBVHNode::checkCollision null deref crash on leaf nodes #235

Merged

Initial plan

1eef651

Copilot AI mentioned this pull request Mar 16, 2026

Fix BendConstraint::solve signature mismatch with base class #236

Merged

FriedricNietzsche and others added 2 commits March 16, 2026 18:14

Potential fix for pull request finding

956eeec

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Fix duplicate isLeaf() definition in SphereBVH.cpp

124f354

Co-authored-by: FriedricNietzsche <142739679+FriedricNietzsche@users.noreply.github.com>

Copilot AI and others added 6 commits March 16, 2026 22:18

Fix BendConstraint::solve signature to match base class Constraint::s…

79ceff7

…olve Co-authored-by: FriedricNietzsche <142739679+FriedricNietzsche@users.noreply.github.com>

Fix SphereBVHNode::checkCollision crash on leaf nodes

567fe3d

Co-authored-by: FriedricNietzsche <142739679+FriedricNietzsche@users.noreply.github.com>

Merge pull request #233 from P0k3rf4ce/copilot/sub-pr-230

3fbfedf

Fix out-of-bounds end index in SphereBVH::fromScene

Merge pull request #234 from P0k3rf4ce/copilot/sub-pr-230-again

d7c2fd7

Remove duplicate `SphereBVHNode::isLeaf()` definition

Merge pull request #235 from P0k3rf4ce/copilot/sub-pr-230-another-one

2993a5e

Fix SphereBVHNode::checkCollision null deref crash on leaf nodes

Merge pull request #236 from P0k3rf4ce/copilot/sub-pr-230-yet-again

b64e142

Fix BendConstraint::solve signature mismatch with base class

P0k3rf4ce changed the base branch from pbr-dev to main March 16, 2026 22:38

Conversation

FriedricNietzsche commented Mar 10, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

rowel-eshan commented Mar 16, 2026

Uh oh!

FriedricNietzsche commented Mar 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

FriedricNietzsche Mar 16, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

FriedricNietzsche Mar 16, 2026

Choose a reason for hiding this comment

Uh oh!

FriedricNietzsche Mar 16, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI commented Mar 16, 2026

Uh oh!

Copilot AI commented Mar 16, 2026

Uh oh!

Copilot AI commented Mar 16, 2026

Uh oh!

Copilot AI commented Mar 16, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

FriedricNietzsche commented Mar 16, 2026 •

edited

Loading