From 84bbd77ba6804cb7286c3b2c0d8e707d49badce4 Mon Sep 17 00:00:00 2001 From: ggetz Date: Fri, 16 Aug 2024 09:59:50 -0400 Subject: [PATCH] Cleanup --- CHANGES.md | 2 +- .../Source/DataSources/ModelGraphics.js | 2 - .../Source/DataSources/ModelVisualizer.js | 1 - packages/engine/Source/Renderer/CubeMap.js | 25 ++++--- .../engine/Source/Scene/Cesium3DTileset.js | 9 ++- .../Scene/DynamicEnvironmentMapManager.js | 75 ++++++++++++++----- packages/engine/Source/Scene/Model/Model.js | 11 +-- 7 files changed, 82 insertions(+), 43 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index d2ea5251c441..fa24d2daecf6 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -18,7 +18,7 @@ ##### Breaking Changes :mega: - Custom specular environment maps in `ImageBasedLighting` now require either a WebGL2 context or a WebGL1 context that supports the [`EXT_shader_texture_lod` extension](https://registry.khronos.org/webgl/extensions/EXT_shader_texture_lod/). -- `ImageBasedLighting.luminanceAtZenith` has been removed. Use newFoo instead. +- `ImageBasedLighting.luminanceAtZenith` has been removed. Use `DynamicEnvironmentMapManager` instead. ### 1.120 - 2024-08-01 diff --git a/packages/engine/Source/DataSources/ModelGraphics.js b/packages/engine/Source/DataSources/ModelGraphics.js index 3d238410c72f..1622dafae0e2 100644 --- a/packages/engine/Source/DataSources/ModelGraphics.js +++ b/packages/engine/Source/DataSources/ModelGraphics.js @@ -411,8 +411,6 @@ ModelGraphics.prototype.merge = function (source) { source.imageBasedLightingFactor ); - // TODO environmentMapManager - this.lightColor = defaultValue(this.lightColor, source.lightColor); this.distanceDisplayCondition = defaultValue( this.distanceDisplayCondition, diff --git a/packages/engine/Source/DataSources/ModelVisualizer.js b/packages/engine/Source/DataSources/ModelVisualizer.js index 681b812d990c..393345358aee 100644 --- a/packages/engine/Source/DataSources/ModelVisualizer.js +++ b/packages/engine/Source/DataSources/ModelVisualizer.js @@ -263,7 +263,6 @@ ModelVisualizer.prototype.update = function (time) { time, defaultImageBasedLightingFactor ); - // TODO: environmentMapManager let lightColor = Property.getValueOrUndefined( modelGraphics._lightColor, time diff --git a/packages/engine/Source/Renderer/CubeMap.js b/packages/engine/Source/Renderer/CubeMap.js index a789da3d0908..b5ad1479f12c 100644 --- a/packages/engine/Source/Renderer/CubeMap.js +++ b/packages/engine/Source/Renderer/CubeMap.js @@ -246,10 +246,11 @@ function CubeMap(options) { } /** - * TODO - * @param {*} cubeMap - * @param {*} frameState - * @returns + * Copy an existing texture to a cubemap face. + * @param {FrameState} frameState The current rendering frameState + * @param {Texture} texture Texture being copied + * @param {CubeMap.FaceName} face The face to which to copy + * @param {number} [mipLevel=0] The mip level at which to copy */ CubeMap.prototype.copyFace = function (frameState, texture, face, mipLevel) { const context = frameState.context; @@ -266,7 +267,7 @@ CubeMap.prototype.copyFace = function (frameState, texture, face, mipLevel) { 0, texture.width, texture.height, - mipLevel + defaultValue(mipLevel, 0) ); framebuffer._unBind(); framebuffer.destroy(); @@ -484,10 +485,10 @@ Object.defineProperties(CubeMap.prototype, { }); /** - * TODO - * @param {CubeMap.FaceName} face - * @param {Cartesian3} result - * @returns + * Get a vector representing the cubemap face direction + * @param {CubeMap.FaceName} face The relevant face + * @param {Cartesian3} [result] The object onto which to store the result. + * @returns {Cartesian3} The vector representing the cubemap face direction */ CubeMap.getDirection = function (face, result) { switch (face) { @@ -652,9 +653,11 @@ CubeMap.prototype.generateMipmap = function (hint) { }; /** - * TODO + * Create a vertex array that can be used for cubemap shaders. + * @param {Context} context The rendering context + * @returns {VertexArray} The created vertex array */ -CubeMap.createVertexArray = function (context, face) { +CubeMap.createVertexArray = function (context) { const geometry = BoxGeometry.createGeometry( BoxGeometry.fromDimensions({ dimensions: new Cartesian3(2.0, 2.0, 2.0), diff --git a/packages/engine/Source/Scene/Cesium3DTileset.js b/packages/engine/Source/Scene/Cesium3DTileset.js index 2575c5d3485f..269fcffdf7b1 100644 --- a/packages/engine/Source/Scene/Cesium3DTileset.js +++ b/packages/engine/Source/Scene/Cesium3DTileset.js @@ -105,6 +105,7 @@ import DynamicEnvironmentMapManager from "./DynamicEnvironmentMapManager.js"; * @property {object} [pointCloudShading] Options for constructing a {@link PointCloudShading} object to control point attenuation based on geometric error and lighting. * @property {Cartesian3} [lightColor] The light color when shading models. When undefined the scene's light color is used instead. * @property {ImageBasedLighting} [imageBasedLighting] The properties for managing image-based lighting for this tileset. + * @param {DynamicEnvironmentMapManager.ConstructorOptions} [options.environmentMapOptions] The properties for managing dynamic environment maps on this model. * @property {boolean} [backFaceCulling=true] Whether to cull back-facing geometry. When true, back face culling is determined by the glTF material's doubleSided property; when false, back face culling is disabled. * @property {boolean} [enableShowOutline=true] Whether to enable outlines for models using the {@link https://github.com/KhronosGroup/glTF/tree/master/extensions/2.0/Vendor/CESIUM_primitive_outline|CESIUM_primitive_outline} extension. This can be set to false to avoid the additional processing of geometry at load time. When false, the showOutlines and outlineColor options are ignored. * @property {boolean} [showOutline=true] Whether to display the outline for models using the {@link https://github.com/KhronosGroup/glTF/tree/master/extensions/2.0/Vendor/CESIUM_primitive_outline|CESIUM_primitive_outline} extension. When true, outlines are displayed. When false, outlines are not displayed. @@ -828,8 +829,9 @@ function Cesium3DTileset(options) { this._shouldDestroyImageBasedLighting = true; } - // TODO: options - this._environmentMapManager = new DynamicEnvironmentMapManager(); + this._environmentMapManager = new DynamicEnvironmentMapManager( + options.environmentMapOptions + ); /** * The light color when shading models. When undefined the scene's light color is used instead. @@ -1864,10 +1866,9 @@ Object.defineProperties(Cesium3DTileset.prototype, { }, /** - * TODO: The properties for managing image-based lighting on this model. + * The properties for managing dynamic environment maps on this model. Affects lighting. * * @memberof Model.prototype - * @private * @readonly * * @type {DynamicEnvironmentMapManager} diff --git a/packages/engine/Source/Scene/DynamicEnvironmentMapManager.js b/packages/engine/Source/Scene/DynamicEnvironmentMapManager.js index 6ca37f567c17..2c682ff57595 100644 --- a/packages/engine/Source/Scene/DynamicEnvironmentMapManager.js +++ b/packages/engine/Source/Scene/DynamicEnvironmentMapManager.js @@ -33,7 +33,15 @@ import DynamicAtmosphereLightingType from "./DynamicAtmosphereLightingType.js"; * * Options for the DynamicEnvironmentMapManager constructor * + * @property {boolean} [enabled=true] If true, the environment map and related properties will continue to update. * @property {number} [mipmapLevels=10] The number of mipmap levels to generate for specular maps. More mipmap levels will produce a higher resolution specular reflection. + * @property {number} [maximumSecondsDifference=1800] The maximum amount of elapsed seconds before a new environment map is created + * @property {number} [maximumPositionEpsilon=CesiumMath.EPSILON1] The maximum difference in position before a new environment map is created. Small differences in position will not visibly affect results. + * @property {number} [intensity=2.0] The intensity of the light. This should be adjusted relative to the value of {@link Scene.light} intensity. + * @property {number} [gamma=1.0] The gamma correction to apply to the range of light. 1.0 uses the unmodified incoming light color. + * @property {number} [brightness=1.0] The brightness of light. 1.0 uses the unmodified incoming environment color. Less than 1.0 makes the light darker while greater than 1.0 makes it brighter. + * @property {number} [saturation=0.8] The saturation of the light. 1.0 uses the unmodified incoming environment color. Less than 1.0 reduces the saturation while greater than 1.0 increases it. + * @property {Color} [groundColor=DynamicEnvironmentMapManager.AVERAGE_EARTH_GROUND_COLOR] Solid color used to represent the ground. */ /** @@ -89,30 +97,52 @@ function DynamicEnvironmentMapManager(options) { * @type {boolean} * @default true */ - this.enabled = true; + this.enabled = defaultValue(options.enabled, true); /** * The maximum amount of elapsed seconds before a new environment map is created. * @type {number} - * @default 180 + * @default 1800 */ - this.maximumSecondsDifference = 60 * 30; + this.maximumSecondsDifference = defaultValue( + options.maximumSecondsDifference, + 60 * 30 + ); + + /** + * The maximum difference in position before a new environment map is created. Small differences in position will not visibly affect results. + * @type {number} + * @default CesiumMath.EPSILON1 + */ + this.maximumPositionEpsilon = defaultValue( + options.maximumPositionEpsilon, + CesiumMath.EPSILON1 + ); + + /** + * The intensity of the light. This should be adjusted relative to the value of {@link Scene.light} intensity. + * @type {number} + * @default 2.0 + * @see {DirectionalLight.intensity} + * @see {SunLight.intensity} + */ + this.intensity = defaultValue(options.intensity, 2.0); /** - * The gamma correction to apply to the range of light. 1.0 uses the unmodified incoming light color. + * The gamma correction to apply to the range of light. 1.0 uses the unmodified incoming light color. * @type {number} * @default 1.0 */ - this.gamma = 1.0; + this.gamma = defaultValue(options.gamma, 1.0); /** - * The brightness of light. 1.0 uses the unmodified incoming environment color. Less than 1.0 + * The brightness of light. 1.0 uses the unmodified incoming environment color. Less than 1.0 * makes the light darker while greater than 1.0 makes it brighter. * * @type {number} * @default 1.0 */ - this.brightness = 1.0; + this.brightness = defaultValue(options.brightness, 1.0); /** * The saturation of the light. 1.0 uses the unmodified incoming environment color. Less than 1.0 reduces the @@ -121,21 +151,17 @@ function DynamicEnvironmentMapManager(options) { * @type {number} * @default 0.8 */ - this.saturation = 0.8; - - /** - * The intensity of the light. - * @type {number} - * @default 2.0 - */ - this.intensity = 2.0; + this.saturation = defaultValue(options.saturation, 0.8); /** * Solid color used to represent the ground. * @type {Color} * @default Color.fromCssColorString("#6E6259").withAlpha(0.3) average ground color on earth, a warm grey */ - this.groundColor = Color.fromCssColorString("#6E6259").withAlpha(0.3); + this.groundColor = defaultValue( + options.groundColor, + DynamicEnvironmentMapManager.AVERAGE_EARTH_GROUND_COLOR + ); } Object.defineProperties(DynamicEnvironmentMapManager.prototype, { @@ -163,7 +189,11 @@ Object.defineProperties(DynamicEnvironmentMapManager.prototype, { }, set: function (value) { if ( - Cartesian3.equalsEpsilon(value, this._position, CesiumMath.EPSILON8) + Cartesian3.equalsEpsilon( + value, + this._position, + this.maximumPositionEpsilon + ) ) { return; } @@ -347,7 +377,6 @@ function updateRadianceMap(manager, frameState) { } if (manager._radianceCommandsDirty) { - // TODO: Do we need both of these dirty flags? let fs = manager._radianceMapFS; if (!defined(fs)) { fs = new ShaderSource({ @@ -609,7 +638,7 @@ function updateSphericalHarmonicCoefficients(manager, frameState) { * Do not call this function directly. *

* @private - * @return {boolean} TODO + * @return {boolean} true is shaders should be updated. */ DynamicEnvironmentMapManager.prototype.update = function (frameState) { if (!this.enabled || !defined(this._position)) { @@ -732,4 +761,12 @@ DynamicEnvironmentMapManager.prototype.destroy = function () { return destroyObject(this); }; +/** + * Average hue of ground color on earth. + * @type {Color} + */ +DynamicEnvironmentMapManager.AVERAGE_EARTH_GROUND_COLOR = Object.freeze( + Color.fromCssColorString("#25211E") +); + export default DynamicEnvironmentMapManager; diff --git a/packages/engine/Source/Scene/Model/Model.js b/packages/engine/Source/Scene/Model/Model.js index e626c8395cf7..117014a87de8 100644 --- a/packages/engine/Source/Scene/Model/Model.js +++ b/packages/engine/Source/Scene/Model/Model.js @@ -152,6 +152,7 @@ import pickModel from "./pickModel.js"; * @privateParam {ClippingPolygonCollection} [options.clippingPolygons] The {@link ClippingPolygonCollection} used to selectively disable rendering the model. * @privateParam {Cartesian3} [options.lightColor] The light color when shading the model. When undefined the scene's light color is used instead. * @privateParam {ImageBasedLighting} [options.imageBasedLighting] The properties for managing image-based lighting on this model. + * @privateParam {DynamicEnvironmentMapManager.ConstructorOptions} [options.environmentMapOptions] The properties for managing dynamic environment maps on this model. Affects lighting. * @privateParam {boolean} [options.backFaceCulling=true] Whether to cull back-facing geometry. When true, back face culling is determined by the material's doubleSided property; when false, back face culling is disabled. Back faces are not culled if the model's color is translucent. * @privateParam {Credit|string} [options.credit] A credit for the data source, which is displayed on the canvas. * @privateParam {boolean} [options.showCreditsOnScreen=false] Whether to display the credits of this model on screen. @@ -394,9 +395,10 @@ function Model(options) { : new ImageBasedLighting(); this._shouldDestroyImageBasedLighting = !defined(options.imageBasedLighting); - // TODO: Options this._environmentMapManager = undefined; - const environmentMapManager = new DynamicEnvironmentMapManager(); + const environmentMapManager = new DynamicEnvironmentMapManager( + options.environmentMapOptions + ); DynamicEnvironmentMapManager.setOwner( environmentMapManager, this, @@ -1406,10 +1408,9 @@ Object.defineProperties(Model.prototype, { }, /** - * TODO: The properties for managing image-based lighting on this model. + * The properties for managing dynamic environment maps on this model. Affects lighting. * * @memberof Model.prototype - * @private * @readonly * * @type {DynamicEnvironmentMapManager} @@ -1826,7 +1827,6 @@ Model.prototype.update = function (frameState) { const environmentMapManager = this._environmentMapManager; if (this._ready && environmentMapManager.owner === this) { - // TODO: Probs don't want to do this every frame environmentMapManager.position = this._boundingSphere.center; if (environmentMapManager.update(frameState)) { @@ -2837,6 +2837,7 @@ Model.prototype.destroyModelResources = function () { * @param {ClippingPolygonCollection} [options.clippingPolygons] The {@link ClippingPolygonCollection} used to selectively disable rendering the model. * @param {Cartesian3} [options.lightColor] The light color when shading the model. When undefined the scene's light color is used instead. * @param {ImageBasedLighting} [options.imageBasedLighting] The properties for managing image-based lighting on this model. + * @param {DynamicEnvironmentMapManager.ConstructorOptions} [options.environmentMapOptions] The properties for managing dynamic environment maps on this model. * @param {boolean} [options.backFaceCulling=true] Whether to cull back-facing geometry. When true, back face culling is determined by the material's doubleSided property; when false, back face culling is disabled. Back faces are not culled if the model's color is translucent. * @param {Credit|string} [options.credit] A credit for the data source, which is displayed on the canvas. * @param {boolean} [options.showCreditsOnScreen=false] Whether to display the credits of this model on screen.