From 76a01c10b0a7cf18a69748486b90e12118d7b594 Mon Sep 17 00:00:00 2001 From: Harel M Date: Thu, 22 Aug 2024 13:08:57 +0300 Subject: [PATCH] Fix issues with setting sky to undefined (#4587) * Fix issues with undefining a style * Update changelog * Fix test name --- CHANGELOG.md | 3 ++- src/render/painter.ts | 2 +- src/style/sky.ts | 9 ++++++++ src/style/style.test.ts | 18 ++++++++++++++++ src/style/style.ts | 23 ++++++++++++--------- src/ui/map.ts | 8 +++++-- src/util/test/util.ts | 4 ++-- test/examples/sky-with-fog-and-terrain.html | 6 ++++++ 8 files changed, 57 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b3da31c264..afd5fb8583 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,7 +12,8 @@ - Fix 3D map freezing when camera is adjusted against map bounds. ([#4537](https://github.com/maplibre/maplibre-gl-js/issues/4537)) - Fix `getStyle()` to return a clone so the object cannot be internally changed ([#4488](https://github.com/maplibre/maplibre-gl-js/issues/4488)) - Prefer local glyph rendering for all CJKV characters, not just those in the CJK Unified Ideographs, Hiragana, Katakana, and Hangul Syllables blocks. ([#4560](https://github.com/maplibre/maplibre-gl-js/pull/4560))) -- - _...Add new stuff here..._ +- Fix issues with setting sky to undefined ([#4587](https://github.com/maplibre/maplibre-gl-js/pull/4587))) +- _...Add new stuff here..._ ## 4.5.2 diff --git a/src/render/painter.ts b/src/render/painter.ts index 268d53cf6e..59f9896ac6 100644 --- a/src/render/painter.ts +++ b/src/render/painter.ts @@ -409,7 +409,7 @@ export class Painter { this.clearStencil(); // draw sky first to not overwrite symbols - if (this.style.stylesheet?.sky) drawSky(this, this.style.sky); + if (this.style.sky) drawSky(this, this.style.sky); this._showOverdrawInspector = options.showOverdrawInspector; this.depthRangeFor3D = [0, 1 - ((style._order.length + 2) * this.numSublayers * this.depthEpsilon)]; diff --git a/src/style/sky.ts b/src/style/sky.ts index 261407d9fc..2550f1b117 100644 --- a/src/style/sky.ts +++ b/src/style/sky.ts @@ -55,11 +55,20 @@ export class Sky extends Evented { this._transitionable = new Transitionable(properties); this.setSky(sky); this._transitioning = this._transitionable.untransitioned(); + this.recalculate(new EvaluationParameters(0)); } setSky(sky?: SkySpecification, options: StyleSetterOptions = {}) { if (this._validate(validateSky, sky, options)) return; + if (!sky) { + sky = { + 'sky-color': 'transparent', + 'horizon-color': 'transparent', + 'fog-color': 'transparent', + }; + } + for (const name in sky) { const value = sky[name]; if (name.endsWith(TRANSITION_SUFFIX)) { diff --git a/src/style/style.test.ts b/src/style/style.test.ts index 6789921fc9..5bc79c228c 100644 --- a/src/style/style.test.ts +++ b/src/style/style.test.ts @@ -2592,6 +2592,24 @@ describe('Style#serialize', () => { expect(style.getSky()).toBeUndefined(); }); + test('sky should be defined even after setting it to undefined and back', async () => { + const sky: SkySpecification = { + 'horizon-fog-blend': 0.5, + 'fog-color': '#fff' + }; + const styleJson = createStyleJSON({sky}); + const style = new Style(getStubMap()); + style.loadJSON(styleJson); + + await style.once('style.load'); + style.setSky(undefined); + expect(style.serialize().sky).toBeUndefined(); + style.setSky(sky); + expect(style.serialize().sky).toBeDefined(); + style.setSky(undefined); + expect(style.serialize().sky).toBeUndefined(); + }); + test('do not include sky property after removing sky from the map', async () => { const sky: SkySpecification = { 'horizon-fog-blend': 0.5, diff --git a/src/style/style.ts b/src/style/style.ts index dd500ff05d..00ac0f3656 100644 --- a/src/style/style.ts +++ b/src/style/style.ts @@ -1496,17 +1496,20 @@ export class Style extends Evented { } setSky(skyOptions?: SkySpecification, options: StyleSetterOptions = {}) { - const sky = this.sky.getSky(); + const sky = this.getSky(); let update = false; - if (!skyOptions) { - if (sky) { - update = true; - } - } - for (const key in skyOptions) { - if (!deepEqual(skyOptions[key], sky[key])) { - update = true; - break; + if (!skyOptions && !sky) return; + + if (skyOptions && !sky) { + update = true; + } else if (!skyOptions && sky) { + update = true; + } else { + for (const key in skyOptions) { + if (!deepEqual(skyOptions[key], sky[key])) { + update = true; + break; + } } } if (!update) return; diff --git a/src/ui/map.ts b/src/ui/map.ts index b7cb3379e0..1bbd1c7fb2 100644 --- a/src/ui/map.ts +++ b/src/ui/map.ts @@ -2665,7 +2665,7 @@ export class Map extends Camera { /** * Loads sky and fog defined by {@link SkySpecification} onto the map. * Note: The fog only shows when using the terrain 3D feature. - * @param sky - Sky properties to set. Must conform to the [MapLibre Style Specification](https://maplibre.org/maplibre-gl-js-docs/style-spec/#sky). + * @param sky - Sky properties to set. Must conform to the [MapLibre Style Specification](https://maplibre.org/maplibre-style-spec/sky/). * @returns `this` * @example * ```ts @@ -2681,7 +2681,11 @@ export class Map extends Camera { /** * Returns the value of the sky object. * - * @returns sky Sky properties of the style. + * @returns the sky properties of the style. + * @example + * ```ts + * map.getSky(); + * ``` */ getSky() { return this.style.getSky(); diff --git a/src/util/test/util.ts b/src/util/test/util.ts index 6f08798b9e..78cf03df93 100644 --- a/src/util/test/util.ts +++ b/src/util/test/util.ts @@ -172,7 +172,7 @@ export function createStyleSource() { } as SourceSpecification; } -export function createStyle() { +export function createStyle(): StyleSpecification { return { version: 8, center: [-73.9749, 40.7736], @@ -181,5 +181,5 @@ export function createStyle() { pitch: 50, sources: {}, layers: [] - } as StyleSpecification; + }; } diff --git a/test/examples/sky-with-fog-and-terrain.html b/test/examples/sky-with-fog-and-terrain.html index 7d0f40bee3..10f67b3135 100644 --- a/test/examples/sky-with-fog-and-terrain.html +++ b/test/examples/sky-with-fog-and-terrain.html @@ -25,6 +25,7 @@
+ @@ -34,6 +35,10 @@