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.

Sign up for GitHub

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

Jump to bottom

Add fill-extrusion-vertical-gradient property #6841

Merged
merged 7 commits into from
Sep 25, 2018
Merged

Add fill-extrusion-vertical-gradient property #6841

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
d13dada
Add fill-extrusion-vertical-gradient property
Jun 15, 2018
47e3dfa
Remove stray comma
Sep 24, 2018
2e93481
Update doc description
Sep 24, 2018
084fe11
Use Uniform1i and better naming
Sep 24, 2018
28082d4
Cast vertical gradient boolean to number when binding to uniform
Sep 25, 2018
1e66d47
Revert uniform to a float
Sep 25, 2018
587cf44
Remove constant from directional calculation
Sep 25, 2018
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
5 changes: 3 additions & 2 deletions src/render/draw_fill_extrusion.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,9 +70,10 @@ function drawExtrusionTiles(painter, source, layer, coords, depthMode, stencilMo
layer.paint.get('fill-extrusion-translate'),
layer.paint.get('fill-extrusion-translate-anchor'));

const shouldUseVerticalGradient = layer.paint.get('fill-extrusion-vertical-gradient');
const uniformValues = image ?
fillExtrusionPatternUniformValues(matrix, painter, coord, crossfade, tile) :
fillExtrusionUniformValues(matrix, painter);
fillExtrusionPatternUniformValues(matrix, painter, shouldUseVerticalGradient, coord, crossfade, tile) :
fillExtrusionUniformValues(matrix, painter, shouldUseVerticalGradient);


program.draw(context, context.gl.TRIANGLES, depthMode, stencilMode, colorMode, CullFaceMode.backCCW,
Expand Down
17 changes: 12 additions & 5 deletions src/render/program/fill_extrusion_program.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ export type FillExtrusionUniformsType = {|
'u_matrix': UniformMatrix4f,
'u_lightpos': Uniform3f,
'u_lightintensity': Uniform1f,
'u_lightcolor': Uniform3f
'u_lightcolor': Uniform3f,
'u_vertical_gradient': Uniform1f
Copy link
Contributor

Choose a reason for hiding this comment

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

this is just 1 or 0 right? in that case we can use an integer uniform Uniform1i to follow the convention of other uniforms used as booleans such as u_is_zoom_constant for symbol icons/sdfs

Copy link
Contributor

Choose a reason for hiding this comment

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

This actually needs to be a float because the calculation in the shader has float constants.

|};

export type FillExtrusionPatternUniformsType = {|
Expand All @@ -33,6 +34,7 @@ export type FillExtrusionPatternUniformsType = {|
'u_lightintensity': Uniform1f,
'u_lightcolor': Uniform3f,
'u_height_factor': Uniform1f,
'u_vertical_gradient': Uniform1f,
// pattern uniforms:
'u_texsize': Uniform2f,
'u_image': Uniform1i,
Expand All @@ -53,14 +55,16 @@ const fillExtrusionUniforms = (context: Context, locations: UniformLocations): F
'u_matrix': new UniformMatrix4f(context, locations.u_matrix),
'u_lightpos': new Uniform3f(context, locations.u_lightpos),
'u_lightintensity': new Uniform1f(context, locations.u_lightintensity),
'u_lightcolor': new Uniform3f(context, locations.u_lightcolor)
'u_lightcolor': new Uniform3f(context, locations.u_lightcolor),
'u_vertical_gradient': new Uniform1f(context, locations.u_vertical_gradient)
});

const fillExtrusionPatternUniforms = (context: Context, locations: UniformLocations): FillExtrusionPatternUniformsType => ({
'u_matrix': new UniformMatrix4f(context, locations.u_matrix),
'u_lightpos': new Uniform3f(context, locations.u_lightpos),
'u_lightintensity': new Uniform1f(context, locations.u_lightintensity),
'u_lightcolor': new Uniform3f(context, locations.u_lightcolor),
'u_vertical_gradient': new Uniform1f(context, locations.u_vertical_gradient),
'u_height_factor': new Uniform1f(context, locations.u_height_factor),
// pattern uniforms
'u_image': new Uniform1i(context, locations.u_image),
Expand All @@ -80,7 +84,8 @@ const extrusionTextureUniforms = (context: Context, locations: UniformLocations)

const fillExtrusionUniformValues = (
matrix: Float32Array,
painter: Painter
painter: Painter,
shouldUseVerticalGradient: boolean
): UniformValues<FillExtrusionUniformsType> => {
const light = painter.style.light;
const _lp = light.properties.get('position');
Expand All @@ -97,18 +102,20 @@ const fillExtrusionUniformValues = (
'u_matrix': matrix,
'u_lightpos': lightPos,
'u_lightintensity': light.properties.get('intensity'),
'u_lightcolor': [lightColor.r, lightColor.g, lightColor.b]
'u_lightcolor': [lightColor.r, lightColor.g, lightColor.b],
'u_vertical_gradient': +shouldUseVerticalGradient
};
};

const fillExtrusionPatternUniformValues = (
matrix: Float32Array,
painter: Painter,
shouldUseVerticalGradient: boolean,
coord: OverscaledTileID,
crossfade: CrossfadeParameters,
tile: Tile
): UniformValues<FillExtrusionPatternUniformsType> => {
return extend(fillExtrusionUniformValues(matrix, painter),
return extend(fillExtrusionUniformValues(matrix, painter, shouldUseVerticalGradient),
patternUniformValues(crossfade, painter, tile),
{
'u_height_factor': -Math.pow(2, coord.overscaledZ) / tile.tileSize / 8
Expand Down
7 changes: 6 additions & 1 deletion src/shaders/fill_extrusion.vertex.glsl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ uniform mat4 u_matrix;
uniform vec3 u_lightcolor;
uniform lowp vec3 u_lightpos;
uniform lowp float u_lightintensity;
uniform float u_vertical_gradient;

attribute vec2 a_pos;
attribute vec4 a_normal_ed;
Expand Down Expand Up @@ -47,7 +48,11 @@ void main() {

// Add gradient along z axis of side surfaces
if (normal.y != 0.0) {
directional *= clamp((t + base) * pow(height / 150.0, 0.5), mix(0.7, 0.98, 1.0 - u_lightintensity), 1.0);
// This avoids another branching statement, but multiplies by a constant of 0.84 if no vertical gradient,
Copy link
Contributor

Choose a reason for hiding this comment

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

do we have any idea where the magic -0.84 comes from? maybe nicki would know

Copy link
Contributor

Choose a reason for hiding this comment

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

why would Nicki know? i have no idea where it comes from

Copy link
Contributor

Choose a reason for hiding this comment

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

Nicki worked on the extrusion shaders w lbud

Copy link
Contributor

Choose a reason for hiding this comment

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

@nickidlugash do you remember where this magic number came from?

Copy link

Choose a reason for hiding this comment

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

@ryanhamley @mollymerp I didn't work with lbud on this PR at all, but having worked on the initial implementation of this shader, my guess is that 0.84 is the lower clamp in the gradient logic when u_lightintensity is the default value of 0.5 (mix(0.7, 0.98, 0.5) = 0.84). I assume it's negative so that when u_vertical_gradient is 0, that first line just evaluates to 0.84.

0.7 and 0.98 are magic numbers that I came up with to keep the vertical gradient consistently subtle, regardless of how intense the light is.

I actually wonder though if it would be better not to multiply by a constant here if there is no vertical gradient (i.e. something like (1.0 - u_vertical_gradient) instead of the current top line). Multiplying by a constant seems like it would unnecessarily limit the range of light intensity such that you could never get as intense a light when intensity=1.

// and otherwise calculates the gradient based on base + height
directional *= ((
(-1.0 + u_vertical_gradient) * -0.84) +
(u_vertical_gradient * clamp((t + base) * pow(height / 150.0, 0.5), mix(0.7, 0.98, 1.0 - u_lightintensity), 1.0)));
}

// Assign final color based on surface + ambient light color, diffuse light directional, and light color
Expand Down
7 changes: 6 additions & 1 deletion src/shaders/fill_extrusion_pattern.vertex.glsl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ uniform vec2 u_pixel_coord_upper;
uniform vec2 u_pixel_coord_lower;
uniform float u_height_factor;
uniform vec4 u_scale;
uniform float u_vertical_gradient;

uniform vec3 u_lightcolor;
uniform lowp vec3 u_lightpos;
Expand Down Expand Up @@ -62,7 +63,11 @@ void main() {
directional = mix((1.0 - u_lightintensity), max((0.5 + u_lightintensity), 1.0), directional);

if (normal.y != 0.0) {
directional *= clamp((t + base) * pow(height / 150.0, 0.5), mix(0.7, 0.98, 1.0 - u_lightintensity), 1.0);
// This avoids another branching statement, but multiplies by a constant of 0.84 if no vertical gradient,
// and otherwise calculates the gradient based on base + height
directional *= ((
(-1.0 + u_vertical_gradient) * -0.84) +
(u_vertical_gradient * clamp((t + base) * pow(height / 150.0, 0.5), mix(0.7, 0.98, 1.0 - u_lightintensity), 1.0)));
}

v_lighting.rgb += clamp(directional * u_lightcolor, mix(vec3(0.0), vec3(0.3), 1.0 - u_lightcolor), vec3(1.0));
Expand Down
16 changes: 16 additions & 0 deletions src/style-spec/reference/v8.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3755,6 +3755,22 @@
]
},
"property-type": "data-driven"
},
"fill-extrusion-vertical-gradient": {
"type": "boolean",
"default": true,
"doc": "Whether to apply a vertical gradient to the sides of a fill-extrusion layer. If true, sides will be shaded slightly darker farther down.",
"transition": false,
"sdk-support": {
"basic functionality": {
"js": "0.50.0"
}
},
"expression": {
"interpolated": false,
"parameters": ["zoom"]
},
"property-type": "data-constant"
}
},
"paint_line": {
Expand Down
3 changes: 2 additions & 1 deletion src/style-spec/types.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -333,7 +333,8 @@ export type FillExtrusionLayerSpecification = {|
"fill-extrusion-translate-anchor"?: PropertyValueSpecification<"map" | "viewport">,
"fill-extrusion-pattern"?: DataDrivenPropertyValueSpecification<string>,
"fill-extrusion-height"?: DataDrivenPropertyValueSpecification<number>,
"fill-extrusion-base"?: DataDrivenPropertyValueSpecification<number>
"fill-extrusion-base"?: DataDrivenPropertyValueSpecification<number>,
"fill-extrusion-vertical-gradient"?: PropertyValueSpecification<boolean>
|}
|}

Expand Down
2 changes: 2 additions & 0 deletions src/style/style_layer/fill_extrusion_style_layer_properties.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ export type PaintProps = {|
"fill-extrusion-pattern": CrossFadedDataDrivenProperty<string>,
"fill-extrusion-height": DataDrivenProperty<number>,
"fill-extrusion-base": DataDrivenProperty<number>,
"fill-extrusion-vertical-gradient": DataConstantProperty<boolean>,
|};

const paint: Properties<PaintProps> = new Properties({
Expand All @@ -36,6 +37,7 @@ const paint: Properties<PaintProps> = new Properties({
"fill-extrusion-pattern": new CrossFadedDataDrivenProperty(styleSpec["paint_fill-extrusion"]["fill-extrusion-pattern"]),
"fill-extrusion-height": new DataDrivenProperty(styleSpec["paint_fill-extrusion"]["fill-extrusion-height"]),
"fill-extrusion-base": new DataDrivenProperty(styleSpec["paint_fill-extrusion"]["fill-extrusion-base"]),
"fill-extrusion-vertical-gradient": new DataConstantProperty(styleSpec["paint_fill-extrusion"]["fill-extrusion-vertical-gradient"]),
});

// Note: without adding the explicit type annotation, Flow infers weaker types
Expand Down
Binary file added .../integration/render-tests/fill-extrusion-vertical-gradient/default/expected.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
101 changes: 101 additions & 0 deletions test/integration/render-tests/fill-extrusion-vertical-gradient/default/style.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
{
"version": 8,
"metadata": {
"test": {
"height": 256
}
},
"light": {
"intensity": 1
},
"sources": {
"geojson": {
"type": "geojson",
"data": {
"type": "FeatureCollection",
"features": [
{
"type": "Feature",
"properties": {
"property": 0
},
"geometry": {
"type": "Polygon",
"coordinates": [
[
[
-0.0003,
-0.0003
],
[
-0.0003,
0.0003
],
[
0.0003,
0.0003
],
[
0.0003,
-0.0003
],
[
-0.0003,
-0.0003
]
]
]
}
},
{
"type": "Feature",
"properties": {
"property": 20
},
"geometry": {
"type": "Polygon",
"coordinates": [
[
[
-0.0003,
-0.0003
],
[
-0.0003,
0.0003
],
[
0.0003,
0.0003
],
[
0.0003,
-0.0003
],
[
-0.0003,
-0.0003
]
]
]
}
}
]
}
}
},
"pitch": 60,
"zoom": 18,
"layers": [
{
"id": "extrusion",
"type": "fill-extrusion",
"source": "geojson",
"paint": {
"fill-extrusion-height": ["+", ["get", "property"], 18],
"fill-extrusion-base": ["get", "property"],
"fill-extrusion-color": "#999"
}
}
]
}
Binary file added test/integration/render-tests/fill-extrusion-vertical-gradient/false/expected.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
102 changes: 102 additions & 0 deletions test/integration/render-tests/fill-extrusion-vertical-gradient/false/style.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
{
"version": 8,
"metadata": {
"test": {
"height": 256
}
},
"light": {
"intensity": 1
},
"sources": {
"geojson": {
"type": "geojson",
"data": {
"type": "FeatureCollection",
"features": [
{
"type": "Feature",
"properties": {
"property": 0
},
"geometry": {
"type": "Polygon",
"coordinates": [
[
[
-0.0003,
-0.0003
],
[
-0.0003,
0.0003
],
[
0.0003,
0.0003
],
[
0.0003,
-0.0003
],
[
-0.0003,
-0.0003
]
]
]
}
},
{
"type": "Feature",
"properties": {
"property": 20
},
"geometry": {
"type": "Polygon",
"coordinates": [
[
[
-0.0003,
-0.0003
],
[
-0.0003,
0.0003
],
[
0.0003,
0.0003
],
[
0.0003,
-0.0003
],
[
-0.0003,
-0.0003
]
]
]
}
}
]
}
}
},
"pitch": 60,
"zoom": 18,
"layers": [
{
"id": "extrusion",
"type": "fill-extrusion",
"source": "geojson",
"paint": {
"fill-extrusion-height": ["+", ["get", "property"], 18],
"fill-extrusion-base": ["get", "property"],
"fill-extrusion-color": "#999",
"fill-extrusion-vertical-gradient": false
}
}
]
}