Skip to content

Adding light position configurability for surface and mesh3d, and general lighting configurability for the latter #556

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

Merged
merged 10 commits into from
May 26, 2016
Merged
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@
"gl-line2d": "^1.3.0",
"gl-line3d": "^1.1.0",
"gl-mat4": "^1.1.2",
"gl-mesh3d": "^1.0.7",
"gl-mesh3d": "^1.2.0",
"gl-plot2d": "^1.1.6",
"gl-plot3d": "^1.5.0",
"gl-scatter2d": "^1.0.5",
Expand Down
24 changes: 23 additions & 1 deletion src/traces/mesh3d/attributes.js
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,29 @@ module.exports = {
reversescale: colorscaleAttrs.reversescale,
showscale: colorscaleAttrs.showscale,

lighting: extendFlat({}, surfaceAtts.lighting),
lightposition: {
'x': extendFlat({}, surfaceAtts.lightposition.x, {dflt: 1e5}),
'y': extendFlat({}, surfaceAtts.lightposition.y, {dflt: 1e5}),
'z': extendFlat({}, surfaceAtts.lightposition.z, {dflt: 0})
},
lighting: extendFlat({}, {
vertexnormalsepsilon: {
valType: 'number',
role: 'style',
min: 0.00,
max: 1,
dflt: 1e-12, // otherwise finely tessellated things eg. the brain will have no specular light reflection
description: 'Epsilon for vertex normals calculation avoids math issues arising from degenerate geometry.'
},
facenormalsepsilon: {
valType: 'number',
role: 'style',
min: 0.00,
max: 1,
dflt: 1e-6, // even the brain model doesn't appear to need finer than this
description: 'Epsilon for face normals calculation avoids math issues arising from degenerate geometry.'
}
}, surfaceAtts.lighting),
Copy link
Contributor

Choose a reason for hiding this comment

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

thanks!


_nestedModules: { // nested module coupling
'colorbar': 'Colorbar'
Expand Down
3 changes: 3 additions & 0 deletions src/traces/mesh3d/convert.js
Original file line number Diff line number Diff line change
Expand Up @@ -106,11 +106,14 @@ proto.update = function(data) {
var config = {
positions: positions,
cells: cells,
lightPosition: [data.lightposition.x, data.lightposition.y, data.lightposition.z],
ambient: data.lighting.ambient,
diffuse: data.lighting.diffuse,
specular: data.lighting.specular,
roughness: data.lighting.roughness,
fresnel: data.lighting.fresnel,
vertexNormalsEpsilon: data.lighting.vertexnormalsepsilon,
faceNormalsEpsilon: data.lighting.facenormalsepsilon,
opacity: data.opacity,
contourEnable: data.contour.show,
contourColor: str2RgbaArray(data.contour.color).slice(0, 3),
Expand Down
8 changes: 7 additions & 1 deletion src/traces/mesh3d/defaults.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,11 +49,17 @@ module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout
}

//Coerce remaining properties
['lighting.ambient',
[
'lighting.ambient',
'lighting.diffuse',
'lighting.specular',
'lighting.roughness',
'lighting.fresnel',
'lighting.vertexnormalsepsilon',
'lighting.facenormalsepsilon',
'lightposition.x',
'lightposition.y',
'lightposition.z',
'contour.show',
'contour.color',
'contour.width',
Expand Down
45 changes: 40 additions & 5 deletions src/traces/surface/attributes.js
Original file line number Diff line number Diff line change
Expand Up @@ -147,41 +147,76 @@ module.exports = {
].join(' ')
},

lightposition: {
x: {
valType: 'number',
role: 'style',
min: -1e5,
max: 1e5,
dflt: 10,
description: 'Numeric vector, representing the X coordinate for each vertex.'
},
y: {
valType: 'number',
role: 'style',
min: -1e5,
max: 1e5,
dflt: 1e5,
description: 'Numeric vector, representing the Y coordinate for each vertex.'
},
z: {
valType: 'number',
role: 'style',
min: -1e5,
max: 1e5,
dflt: 0,
description: 'Numeric vector, representing the Z coordinate for each vertex.'
}
},

lighting: {
ambient: {
valType: 'number',
role: 'style',
min: 0.00,
max: 1.0,
dflt: 0.8
dflt: 0.8,
description: 'Ambient light increases overall color visibility but can wash out the image.'
},
diffuse: {
valType: 'number',
role: 'style',
min: 0.00,
max: 1.00,
dflt: 0.8
dflt: 0.8,
description: 'Represents the extent that incident rays are reflected in a range of angles.'
},
specular: {
valType: 'number',
role: 'style',
min: 0.00,
max: 2.00,
dflt: 0.05
dflt: 0.05,
description: 'Represents the level that incident rays are reflected in a single direction, causing shine.'
},
roughness: {
valType: 'number',
role: 'style',
min: 0.00,
max: 1.00,
dflt: 0.5
dflt: 0.5,
description: 'Alters specular reflection; the rougher the surface, the wider and less contrasty the shine.'
Copy link
Contributor

Choose a reason for hiding this comment

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

Wow. These descriptions are great. Thanks!

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

🙇

},
fresnel: {
valType: 'number',
role: 'style',
min: 0.00,
max: 5.00,
dflt: 0.2
dflt: 0.2,
description: [
'Represents the reflectance as a dependency of the viewing angle; e.g. paper is reflective',
'when viewing it from the edge of the paper (almost 90 degrees), causing shine.'
].join(' ')
}
},

Expand Down
10 changes: 10 additions & 0 deletions src/traces/surface/convert.js
Original file line number Diff line number Diff line change
Expand Up @@ -334,6 +334,16 @@ proto.update = function(data) {
surface.fresnel = data.lighting.fresnel;
}

if('lightposition' in data) {
if(surface.lightPosition === void(0)) {
surface.lightPosition = [data.lightposition.x, data.lightposition.y, data.lightposition.z];
} else {
surface.lightPosition.x = data.lightposition.x;
surface.lightPosition.y = data.lightposition.y;
surface.lightPosition.z = data.lightposition.z;
}
}

if(alpha && alpha < 1) {
surface.supportsTransparency = true;
}
Expand Down
20 changes: 13 additions & 7 deletions src/traces/surface/defaults.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,13 +50,19 @@ module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout
}
}

coerce('lighting.ambient');
coerce('lighting.diffuse');
coerce('lighting.specular');
coerce('lighting.roughness');
coerce('lighting.fresnel');
coerce('hidesurface');
coerce('opacity');
//Coerce remaining properties
[
'lighting.ambient',
'lighting.diffuse',
'lighting.specular',
'lighting.roughness',
'lighting.fresnel',
'lightposition.x',
'lightposition.y',
'lightposition.z',
'hidesurface',
Copy link
Contributor

Choose a reason for hiding this comment

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

looks like you're not coercing vertexnormalsepsilon and facenormalsepsilon in surface traces, is that on purpose?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Correct, initially I wasn't going to shoot for covering the surface3d case, just mesh3d; the reason for adding the properties there is that mesh3d piggybacks on it and we may at some point need this feature for the surface anyway. I'd suggest let's keep this PR for the mesh3d as it's being expected today and I'd do a subsequent small PR for the surface3d case.

Copy link
Contributor

Choose a reason for hiding this comment

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

Adding the vertexnormalsepsilon and facenormalsepsilon functionality to mesh3d traces only is fine. 👍

But in this case, we should not add vertexnormalsepsilon and facenormalsepsilon to the surface attributes to not confuse users reading attribute reference schema.

To do so:

  • remove vertexnormalsepsilon and facenormalsepsilon from the surface attributes
  • rewrite this line as
lighting: extendFlat({}, surfaceAtts.lighting, {
  vertexnormalepsilon: {
    /* */
  }, 
  facenormalepsilon: {
    /* */
  }
})

'opacity'
].forEach(function(x) { coerce(x); });

var surfaceColor = coerce('surfacecolor');

Expand Down
Binary file modified test/image/baselines/gl3d_autorange-zero.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified test/image/baselines/gl3d_bunny-hull.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified test/image/baselines/gl3d_bunny.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified test/image/baselines/gl3d_convex-hull.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified test/image/baselines/gl3d_nan-holes.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified test/image/baselines/gl3d_ribbons.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified test/image/baselines/gl3d_snowden.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added test/image/baselines/gl3d_snowden_altered.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified test/image/baselines/gl3d_surface-lighting.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified test/image/baselines/gl3d_xy-defined-ticks.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
8 changes: 7 additions & 1 deletion test/image/mocks/gl3d_snowden.json

Large diffs are not rendered by default.

33 changes: 33 additions & 0 deletions test/image/mocks/gl3d_snowden_altered.json

Large diffs are not rendered by default.

21 changes: 13 additions & 8 deletions test/image/mocks/gl3d_surface-lighting.json
Original file line number Diff line number Diff line change
Expand Up @@ -510,12 +510,17 @@
"show": false
}
},
"lightposition": {
"x": 1e5,
"y": 1e5,
"z": 1e5
},
"lighting": {
"fresnel": 0.8,
"roughness": 0.5,
"diffuse": 0.5,
"ambient": 0.5,
"specular": 0.5
"fresnel": 0.2,
"roughness": 0.1,
"diffuse": 2,
"ambient": 0.15,
"specular": 2
}
}
],
Expand Down Expand Up @@ -550,18 +555,18 @@
"xaxis": {
"showbackground": true,
"type": "linear",
"backgroundcolor": "rgb(255, 127, 14)",
"backgroundcolor": "rgba(255, 127, 14, 0.2)",
"tickangle": 180
},
"yaxis": {
"showbackground": true,
"type": "linear",
"backgroundcolor": "rgb(255, 0, 0)",
"backgroundcolor": "rgba(255, 0, 0, 0.2)",
"tickangle": -45
},
"zaxis": {
"showbackground": true,
"backgroundcolor": "rgb(44, 160, 44)",
"backgroundcolor": "rgba(44, 160, 44, 0.35)",
"tickangle": 30
}
}
Expand Down