Skip to content
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

Fix LOD selection for image-based lighting #12070

Merged
merged 4 commits into from
Jul 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
##### Fixes :wrench:

- Updated geometric self-shadowing function to improve direct lighting on models using physically-based rendering. [#12063](https://github.com/CesiumGS/cesium/pull/12063)
- Fixed environment map LOD selection in image-based lighting. [#12070](https://github.com/CesiumGS/cesium/pull/12070)

### 1.119 - 2024-07-01

Expand Down
2 changes: 1 addition & 1 deletion packages/engine/Source/Renderer/AutomaticUniforms.js
Original file line number Diff line number Diff line change
Expand Up @@ -1454,7 +1454,7 @@ const AutomaticUniforms = {
* // Example: For a given roughness and NdotV value, find the material's BRDF information in the red and green channels
* float roughness = 0.5;
* float NdotV = dot(normal, view);
* vec2 brdfLut = texture(czm_brdfLut, vec2(NdotV, 1.0 - roughness)).rg;
* vec2 brdfLut = texture(czm_brdfLut, vec2(NdotV, roughness)).rg;
*/
czm_brdfLut: new AutomaticUniform({
size: 1,
Expand Down
11 changes: 5 additions & 6 deletions packages/engine/Source/Scene/OctahedralProjectedCubeMap.js
Original file line number Diff line number Diff line change
Expand Up @@ -337,11 +337,10 @@ OctahedralProjectedCubeMap.prototype.update = function (frameState) {
},
});

// We only need up to 6 mip levels to avoid artifacts.
const length = Math.min(cubeMapBuffers.length, 6);
this._maximumMipmapLevel = length - 1;
const cubeMaps = (this._cubeMaps = new Array(length));
const mipTextures = (this._mipTextures = new Array(length));
const mipLevels = cubeMapBuffers.length;
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it be worth capping this to a higher number than 6?

Copy link
Contributor Author

@jjhembd jjhembd Jul 9, 2024

Choose a reason for hiding this comment

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

I tried a few values with mixed results. My choice of no cap was based on 2 factors:

  1. The higher-numbered LODs are the lower-resolution ones, so including them adds very little memory overhead.
  2. If a user tries the same environment map at different resolutions, they would expect to see differences only in the high-res details of the reflections (mip level 0). However, if we cap the max LOD, different input resolutions will result in a different mip being used for the background low-res part of the reflection.

this._maximumMipmapLevel = mipLevels - 1;
const cubeMaps = (this._cubeMaps = new Array(mipLevels));
const mipTextures = (this._mipTextures = new Array(mipLevels));
const originalSize = cubeMapBuffers[0].positiveX.width * 2.0;
const uniformMap = {
originalSize: function () {
Expand All @@ -350,7 +349,7 @@ OctahedralProjectedCubeMap.prototype.update = function (frameState) {
};

// First we project each cubemap onto a flat octahedron, and write that to a texture.
for (let i = 0; i < length; ++i) {
for (let i = 0; i < mipLevels; ++i) {
// Swap +Y/-Y faces since the octahedral projection expects this order.
const positiveY = cubeMapBuffers[i].positiveY;
cubeMapBuffers[i].positiveY = cubeMapBuffers[i].negativeY;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ vec3 czm_sampleOctahedralProjectionWithFiltering(sampler2D projectedMap, vec2 te
* @returns {vec3} The color of the cube map at the direction.
*/
vec3 czm_sampleOctahedralProjection(sampler2D projectedMap, vec2 textureSize, vec3 direction, float lod, float maxLod) {
float currentLod = floor(lod + 0.5);
float currentLod = floor(lod);
Copy link
Contributor

Choose a reason for hiding this comment

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

I agree that the fix is resulting in better appearances for sure. But any idea why there was a 0.5 increment in the first place?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this dates back to before we were interpolating the mip level. In this previous version we were simply rounding to the nearest mip, which required the 0.5 increment. 006d859 added trilinear filtering but neglected to remove the 0.5 when selecting the "currentLod".

float nextLod = min(currentLod + 1.0, maxLod);

vec3 colorCurrentLod = czm_sampleOctahedralProjectionWithFiltering(projectedMap, textureSize, direction, currentLod);
Expand Down
65 changes: 31 additions & 34 deletions packages/engine/Specs/Scene/OctahedralProjectedCubeMapSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -120,33 +120,32 @@ describe(
});
}

it("creates a packed texture with the right dimensions", function () {
it("creates a packed texture with the right dimensions", async function () {
if (!OctahedralProjectedCubeMap.isSupported(context)) {
return;
}

octahedralMap = new OctahedralProjectedCubeMap(environmentMapUrl);
const frameState = createFrameState(context);

return pollToPromise(function () {
await pollToPromise(function () {
octahedralMap.update(frameState);
return octahedralMap.ready;
}).then(function () {
expect(octahedralMap.texture.width).toEqual(770);
expect(octahedralMap.texture.height).toEqual(512);
expect(octahedralMap.maximumMipmapLevel).toEqual(5);
});
expect(octahedralMap.texture.width).toEqual(770);
expect(octahedralMap.texture.height).toEqual(512);
expect(octahedralMap.maximumMipmapLevel).toEqual(7);
});

it("correctly projects the given cube map and all mip levels", function () {
it("correctly projects the given cube map and all mip levels", async function () {
if (!OctahedralProjectedCubeMap.isSupported(context)) {
return;
}

octahedralMap = new OctahedralProjectedCubeMap(environmentMapUrl);
const frameState = createFrameState(context);

return pollToPromise(function () {
await pollToPromise(function () {
// We manually call update and execute the commands
// because calling scene.renderForSpecs does not
// actually execute these commands, and we need
Expand All @@ -155,34 +154,32 @@ describe(
executeCommands(frameState);

return octahedralMap.ready;
}).then(function () {
const directions = {
positiveX: new Cartesian3(1, 0, 0),
negativeX: new Cartesian3(-1, 0, 0),
positiveY: new Cartesian3(0, 1, 0),
negativeY: new Cartesian3(0, -1, 0),
positiveZ: new Cartesian3(0, 0, 1),
negativeZ: new Cartesian3(0, 0, -1),
};

for (
let mipLevel = 0;
mipLevel < octahedralMap.maximumMipmapLevel;
mipLevel++
) {
for (const key in directions) {
if (directions.hasOwnProperty(key)) {
const direction = directions[key];

expectCubeMapAndOctahedralMapEqual(
octahedralMap,
direction,
mipLevel
);
}
});
const directions = {
positiveX: new Cartesian3(1, 0, 0),
negativeX: new Cartesian3(-1, 0, 0),
positiveY: new Cartesian3(0, 1, 0),
negativeY: new Cartesian3(0, -1, 0),
positiveZ: new Cartesian3(0, 0, 1),
negativeZ: new Cartesian3(0, 0, -1),
};

// The projection is less accurate for the last mip levels,
// where the input cubemap only has a few samples.
const lastAccurateMip = octahedralMap.maximumMipmapLevel - 2;
for (let mipLevel = 0; mipLevel < lastAccurateMip; mipLevel++) {
for (const key in directions) {
if (directions.hasOwnProperty(key)) {
const direction = directions[key];

expectCubeMapAndOctahedralMapEqual(
octahedralMap,
direction,
mipLevel
);
}
}
});
}
});

it("caches projected textures", function () {
Expand Down