Basic sky implementation #1713

prozessor13 · 2022-10-07T08:42:37Z

Based on BAschl's work i created a simple Sky implementation.

Sky can be setted via stylesheet or via map.setSky method. Currently the following prameters are supported:

sky-color
fog-color map below horizon is colored also with this color
horizon-blend with a value from 0 to 1. 0 is no blend, 1 is blend to height/2 (e.g. max visible sky at max pitch).
fog-blend with a value from 0 to 1. 0 is the map-center, 1 is the far-clipping-plane. This setting works only in 3d-mode, also fog is faded out when lowering pitch and disappers below pitch 60

Confirm your changes do not include backports from Mapbox projects (unless with compliant license) - if you are not sure about this, please ask!
Write tests for all new functionality.
Document any changes to public APIs.
Manually test the debug page.
Add an entry inside this element for inclusion in the maplibre-gl-js changelog: <changelog></changelog>.

HarelM · 2022-10-07T11:19:24Z

Can you share some visuals of how this would look?

prozessor13 · 2022-10-07T11:59:21Z

also played with fog.

prozessor13 · 2022-10-09T12:03:00Z

Implemented fog, which was really frustrating to understand the not linear z value of the depthbuffer. So created a separate fogMatrix which creates more useable z-values. As already sayed, fog only works in terrain, and i will not port this to 2d. This is because, for me, the benefit is very very low, and it is a lot of work. Every single shader has to be extended with the fog logic.

prozessor13 · 2022-10-09T12:05:24Z

Fog with default-values:

wipfli · 2022-10-09T13:28:06Z

Looks nice...

ghost · 2022-12-01T15:14:28Z

src/shaders/terrain_depth.fragment.glsl

 highp vec4 pack(highp float value) {
-    highp vec4 comp = fract(value * bitSh);
+    highp vec4 comp = fract((value * 0.5 + 0.5) * bitSh);


nit: In my opinion the old range [0.0; -1.0] was a bit more versatile.
If we implement other similar lookup textures we might want different values ranges (other than -1.0, 1.0), so scaling from [0.0; 1.0] the outside feels easier and slightly better to me.

Especially with this scaling being added, you should definitely add a comment about this having to stay in sync with the one in prelude and vice versa.
They should also be renamed, because pack/unpack are way too generic.

I think the [0, 1.0] range is a left over from the implementation without the fog_matrix. So i think it can be reverted??

I don't think it has to do with the fog_matrix (it doesn't appear to be used in terrain_depth.vertex.glsl).

Elsewhere in the code (in terrain.vertex.glsl) a similar situation is handled using v_depth = pos.z / pos.w * 0.5 + 0.5;, which would probably bump it from [-1;+1] clip-space into the [0;1] range.

However, in this file, where pack/unpack is used, v_depth is probably still in [-1;+1] range (= the GL clip space), so scale and bias is actually necessary.

For consistency I'd probably do the * 0.5 + 0.5 on v_depth, and keep the texture in pack/unpack in [0;1] range.
This is also what I originally referred to as "scaling on the outside".

Thx, i will keep this in mind. Currently i have to re-dive into my own code to understand it in detail, completely lost all my thoughts when wrote it :D

reverted the * 0.5 + 0.5 code, because it was a left over from "before u_fog_matrix".
The problem was that the z-value of the depth-buffer, created by u_matrix is not linear, and so for me it was impossible to create a nice fog-transition from map-center to far-clipping-plane.
With u_fog_matrix i calculate a linear z-value.
So the v_depth from terrain_depth.vertext.glsl and terrain.vertex.glsl are calculated differently. One with the u_matrix and one with the u_fog_matrix. So renamed the fog's v_depth to v_fog_depth.

and the pack/unpack methods are used now, as before, only to pack/unpack z-values into/from the depth-buffer. And the depth-buffer itself currently only used to calculate symbol-visibility. Fog has nothing to do with the depth-buffer.

ghost · 2022-12-01T15:17:03Z

src/style/sky.ts

+
+    calculateFogBlendOpacity(pitch) {
+        if (pitch < 60) return 0; // disable
+        if (pitch < 70) return (pitch - 60) / 10; // fade in


These values feel a bit arbitrary

That's right. The numbers are evaluated by trial/error. And its working ok-ish.

I think there should be a comment then (that these are chosen for looks / personal preference); otherwise people might be scared to change them in the future, believing them to be mathematically important.

Yep, will do that as well.

appended documentation

ghost · 2022-12-01T15:19:19Z

src/style/sky.ts

+
+        this.vertexBuffer = context.createVertexBuffer(vertexArray, posAttributes.members);
+        this.indexBuffer = context.createIndexBuffer(indexArray);
+        this.segments = SegmentVector.simpleSegment(0, 0, vertexArray.length, indexArray.length);


I believe a simple quad vbuffer/ibuffer/segment already exists in painter somewhere?

After a very quick lookup i did not find such a vertex-buffer ([-1, 1] extent). Every draw call is based on tiles, even the background-layer.

I was thinking of

maplibre-gl-js/src/render/painter.ts

Lines 197 to 203 in 9756943

const viewportArray = new PosArray();

viewportArray.emplaceBack(0, 0);

viewportArray.emplaceBack(1, 0);

viewportArray.emplaceBack(0, 1);

viewportArray.emplaceBack(1, 1);

this.viewportBuffer = context.createVertexBuffer(viewportArray, posAttributes.members);

this.viewportSegments = SegmentVector.simpleSegment(0, 0, 4, 2);

and

maplibre-gl-js/src/render/painter.ts

Lines 213 to 216 in 9756943

const quadTriangleIndices = new TriangleIndexArray();

quadTriangleIndices.emplaceBack(0, 1, 2);

quadTriangleIndices.emplaceBack(2, 1, 3);

this.quadTriangleIndexBuffer = context.createIndexBuffer(quadTriangleIndices);

These appear to be used for a fullscreen pass (?) in the heatmap layer:

maplibre-gl-js/src/render/draw_heatmap.ts

Lines 130 to 131 in 9756943

layer.id, painter.viewportBuffer, painter.quadTriangleIndexBuffer,

painter.viewportSegments, layer.paint, painter.transform.zoom);

I don't care too much about this, but it might be able to avoid further state switches in the future (likely won't matter for performance, but still feels cleaner).

Ah, heatmap. right! I basically agree with you, but to reuse this code some refacturing has to be done. If someone is willed doing it, a separate PR would be better i think.

ghost · 2022-12-01T15:20:01Z

src/style-spec/reference/v8.json

@@ -57,6 +57,13 @@
        "intensity": 0.4
      }
    },
+    "sky": {
+      "type": "sky",
+      "doc": "Tho map sky.",


src/shaders/sky.vertex.glsl

ghost · 2022-12-01T15:29:07Z

src/shaders/terrain.fragment.glsl

+    vec4 color = texture2D(u_texture, v_texture_pos);
+    if (v_depth > u_fog_blend) {
+        float a = (v_depth - u_fog_blend) / (1.0 - u_fog_blend);
+        gl_FragColor = mix(color, u_fog_color, pow(a * u_fog_blend_opacity, 2.0));


I believe we know the light direction anyway, so we might want to add https://iquilezles.org/articles/fog/ ?

That's definitely a great example, but because of the lack of any experience in sky/fog and time i kept it simple.

ghost · 2022-12-01T15:32:04Z

src/shaders/sky.fragment.glsl

+    if (y > u_horizon) {
+        float blend = y - u_horizon;
+        if (blend < u_horizon_blend) {
+            gl_FragColor = mix(u_sky_color, u_fog_color, pow(1.0 - blend / u_horizon_blend, 2.0));


I'm not sure if this a good parameterization for fog as it doesn't allow much artistic control but isn't physically based either.

I think sky color should be using a gradient texture instead; it would allow this behaviour, but also many others.

BAchl had implemented a gradient, mapbox in the current setFog method dont. So to reduce complexibility i removed this feature.

prozessor13 · 2022-12-02T07:05:43Z

@JannikGM thanks for you review! But i have to say, that currently i do no longer have resources to work on maplibre. May in the next weeks i have time to:

remove the [0, 1] extent in depth-buffer
draw sky at the very beginning, and remove the 0.99999 hack in z-direction.
document calculateFogBlendOpacity

Regards. Max.

klokan · 2022-12-10T14:22:22Z

This looks amazing - would be a great addition to 3.0 release.

HarelM · 2022-12-10T17:38:50Z

This still needs to be well defined in terms of spec and properties, but 3.0 is not urgent and I believe we can wait with it until this is fully implemented.

prozessor13 · 2023-02-08T14:05:41Z

From code-side i think this PR is complete, and i already implemented Jannik's comments. Furthermore this code is well tested in our company and already productive.
Would be grateful, if someone else will create tests.

HarelM · 2023-02-08T14:30:35Z

Thanks for the update. I'll see what I can do, maybe we can talk about it in the TSC meeting today and see if anyone would like to push this forward.

birkskyum · 2023-05-12T11:57:06Z

@wipfli , is the result of the code sprint to be found anywhere? is that work still ongoing, and by who?

birkskyum · 2023-05-13T15:23:25Z

It's only render tests that can be done here with different sky configs, right? And maybe unit tests to verify that the setSky changes the properties as expected.

birkskyum

Had a look at this.

The SkySpecification has to become part of the style spec repo, before it can be implemented here.

birkskyum · 2023-05-13T15:57:28Z

This still needs to be well defined in terms of spec and properties, but 3.0 is not urgent and I believe we can wait with it until this is fully implemented.

If there is discussion around the definition, let's contain that information in this Style Spec Design Proposal.

ambientlight · 2023-05-15T05:36:02Z

is the result of the code sprint to be found anywhere? is that work still ongoing, and by who?

@birkskyum: should be @jonathanlurie

* Copy style-spec pieces from maplibre/maplibre-gl-js#1713 * add missing SkySpecification * fix lint error

* Style Spec for sky and fog (#298) * Copy style-spec pieces from maplibre/maplibre-gl-js#1713 * add missing SkySpecification * fix lint error * Add tests, improve documentation, add to the style-spec docs * Fix lint * Update changelog --------- Co-authored-by: Andrew Calcutt <acalcutt@techidiots.net>

HarelM · 2024-01-11T11:39:09Z

@prozessor13 as discussed in the meeting, once you create a new PR, please close this one.
THANKS!!!

basic sky implementation

a67fb26

implement basic fog, pitch -fadeout logic is missing

e861e59

prozessor13 added 4 commits October 9, 2022 17:30

fadeout fog when pitch < 70, fog-opacity grows exp instead of linear

9945df3

Merge branch 'main' into sky

f7850b4

load new terrain depth & fragment vertext shaders

7ed74a0

nicer default values, quadratic horizon-blend, fix map.setSky

0508279

ghost reviewed Dec 1, 2022

View reviewed changes

src/shaders/sky.vertex.glsl Outdated Show resolved Hide resolved

ghost reviewed Dec 1, 2022

View reviewed changes

prozessor13 added 2 commits December 5, 2022 11:35

minor fixes based on JannikGM's code-review

d51bd93

rename v_depth used for fog to v_fog_depth

2aca406

klokan added this to the 3.0.0 milestone Dec 10, 2022

acalcutt mentioned this pull request Feb 16, 2023

3D view in maplibre maptiler/tileserver-gl#758

Closed

birkskyum requested changes May 13, 2023

View reviewed changes

birkskyum mentioned this pull request May 13, 2023

Design proposal: Atmosphere / Sky parameters maplibre/maplibre-style-spec#163

Open

birkskyum removed this from the 3.0.0 milestone May 15, 2023

acalcutt added a commit to WifiDB/maplibre-style-spec that referenced this pull request Aug 13, 2023

Copy style-spec pieces from maplibre/maplibre-gl-js#1713

486276e

acalcutt added a commit to WifiDB/maplibre-gl-js that referenced this pull request Aug 13, 2023

copy over maplibre changes from maplibre#1713

b4900c8

This was referenced Aug 13, 2023

Sky fog test #2974

Closed

Style Spec for sky and fog maplibre/maplibre-style-spec#298

Merged

HarelM pushed a commit to maplibre/maplibre-style-spec that referenced this pull request Jan 7, 2024

Style Spec for sky and fog (#298)

c158082

* Copy style-spec pieces from maplibre/maplibre-gl-js#1713 * add missing SkySpecification * fix lint error

prozessor13 mentioned this pull request Jan 30, 2024

Sky Implementation according to spec #3645

Merged

prozessor13 closed this Jan 30, 2024

	const viewportArray = new PosArray();
	viewportArray.emplaceBack(0, 0);
	viewportArray.emplaceBack(1, 0);
	viewportArray.emplaceBack(0, 1);
	viewportArray.emplaceBack(1, 1);
	this.viewportBuffer = context.createVertexBuffer(viewportArray, posAttributes.members);
	this.viewportSegments = SegmentVector.simpleSegment(0, 0, 4, 2);

	const quadTriangleIndices = new TriangleIndexArray();
	quadTriangleIndices.emplaceBack(0, 1, 2);
	quadTriangleIndices.emplaceBack(2, 1, 3);
	this.quadTriangleIndexBuffer = context.createIndexBuffer(quadTriangleIndices);

	layer.id, painter.viewportBuffer, painter.quadTriangleIndexBuffer,
	painter.viewportSegments, layer.paint, painter.transform.zoom);

Basic sky implementation #1713

Basic sky implementation #1713

Conversation

prozessor13 commented Oct 7, 2022 • edited Loading

HarelM commented Oct 7, 2022

prozessor13 commented Oct 7, 2022

prozessor13 commented Oct 9, 2022

prozessor13 commented Oct 9, 2022

wipfli commented Oct 9, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

prozessor13 commented Dec 2, 2022 • edited Loading

klokan commented Dec 10, 2022

HarelM commented Dec 10, 2022

prozessor13 commented Feb 8, 2023

HarelM commented Feb 8, 2023

birkskyum commented May 12, 2023

birkskyum commented May 13, 2023 • edited Loading

birkskyum left a comment

Choose a reason for hiding this comment

birkskyum commented May 13, 2023 • edited Loading

ambientlight commented May 15, 2023

HarelM commented Jan 11, 2024

prozessor13 commented Oct 7, 2022 •

edited

Loading

prozessor13 commented Dec 2, 2022 •

edited

Loading

birkskyum commented May 13, 2023 •

edited

Loading

birkskyum commented May 13, 2023 •

edited

Loading