Skip to content

Commit

Permalink
fix: several issues with surface plots: surfaceColors, shadow (#123)
Browse files Browse the repository at this point in the history
* Bugfix: make surfaceColors optional for SURFACE plots

Without this there was actually no proper way to let the user specify
that surfaceColors should not be used (other than setting
`graph.surfaceColors = undefined` manually after construction), which
means the `else { ... }` branch of the corresponding check in
_redrawSurfaceGraphPoint was effectively dead code:

- if called with `options.surfaceColors = undefined`, the default
  DEFAULTS.surfaceColors value was used

- the `if (colors && colors.length !== 0)` check in
  _redrawSurfaceGraphPoint makes it look as it was avoidable by
  specifying an empty array, however throws an error in
  Settings.setSurfaceColor.

* Improve lighting calculation

The previous behaviour looked incomprehensible to me. Now it scales by
how much the surface is oriented toward the viewer, and makes the bottom
of the surface darker. This corresponds to diffuse lighting with light
shining in the view direction.

Alternatively, we could:

- make no distinction between bottom and top:
  v = Math.abs(crossproduct.z / crossproduct.length())

- use a more standard lighting model (e.g. diffuse/specular/phong)
  as described here: https://learnopengl.com/Lighting/Basic-Lighting

- let the user provide a function (normal vector -> lighting value) in
  the showShadow option

Note that shadow for surface plots was probably not used by many users,
because before this PR this did require manually unsetting
graph.surfaceColors after graph initialization.

* Fix a bug with top/bottom side calculation + showPerspective

The criterion that distinguishes whether we are viewing the top or
bottom of a surface is the angle between surface normal and **view
direction**, which in case of the perspective projection is not the
Z-axis but the location of the fragment relative to the camera.

* Make showShadow work in combination with surfaceColors

* Add option 'showGrayBottom'

Previously, this depended on the value of showShadow, which IMO is a
bug, because it couples two independent decisions.

The option was mentioned as not working, because of (I'm guessing) two
problems: First, the top/bottom-side calculation bug addressed in the
previous commit. Second, because there are edge-cases (literally) where
where a surface is curved such that part of it is viewed from below and
another part from above. To properly address this scenario we would need
different drawing styles for the individual triangles of the surface
polygon.

However, this scenario occurs so rarely, and it even then looks good
enough to me. Furthermore, since this option is new and disabled by
default, I doubt we will make anyone unhappy by giving them this choice.

I made this new option visible in "Styling Surface", because I think it
fits there the most, and it should be visible at least one of the
examples.

* Add option 'showSurfaceGrid'

This fixes the following unexpected/inconsistent behaviours:

- if surfaceColors was active (which was basically *always*, before this
  PR), the style for grid lines was dependent on what was drawn before
  (which was usually an axis or legend, and therefore basically always
  defaulted to axisColor), because strokeStyle was never explicitly set
  in this `if` branch.

- if surfaceColors was inactive (for which a user would have had to
  manually set `graph.surfaceColors = undefined`) it was dependent on
  the value of the showShadow option , which is IMO also not a good
  choice

Again, I make new possibility to disable surface grid visible in the
"styling surface" example, because I believe it fits there the most, and
it should be visible at least one of the examples.
  • Loading branch information
coldfix authored Mar 22, 2020
1 parent 06030be commit 9cefeaa
Show file tree
Hide file tree
Showing 8 changed files with 103 additions and 15 deletions.
15 changes: 14 additions & 1 deletion docs/graph3d/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -390,7 +390,7 @@ <h2 id="Configuration_Options">Configuration Options</h2>

<tr class='toggle collapsible' onclick="toggleTable('optionTable','surfaceColors', this);">
<td><span parent="surfaceColors" class="right-caret"></span> surfaceColors</td>
<td>array or object</td>
<td>boolean, array or object</td>
<td><code>['#FF0000', '#FFF000', '#00FF00', '#68E8FB', '#000FFF']</code></td>
<td>When <code>surfaceColors</code> is set, the surface will be colored according to the colors provided. If it is an object it must have the <code>hue</code> key, it will then be converted into an array according to the following hue values provided:</td>
</tr>
Expand Down Expand Up @@ -500,6 +500,12 @@ <h2 id="Configuration_Options">Configuration Options</h2>
and a slider showing the current frame.
Only applicable when the provided data contains an animation.</td>
</tr>
<tr>
<td>showGrayBottom</td>
<td>boolean</td>
<td>false</td>
<td>If true, draw the bottom side of the surface in gray.</td>
</tr>
<tr>
<td>showGrid</td>
<td>boolean</td>
Expand Down Expand Up @@ -549,6 +555,13 @@ <h2 id="Configuration_Options">Configuration Options</h2>
<td>false</td>
<td>Show shadow on the graph.</td>
</tr>
<tr>
<td>showSurfaceGrid</td>
<td>boolean</td>
<td>true</td>
<td>If true, grid lines are drawn on the surface of the graph itself
(only effective if <code>style: 'surface'</code>.</td>
</tr>
<tr>
<td>style</td>
<td>string</td>
Expand Down
4 changes: 3 additions & 1 deletion examples/graph3d/16_styling_surface.html
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,10 @@
height: "600px",
style: "surface",
showPerspective: true,
showGrayBottom: true,
showGrid: true,
showShadow: false,
showShadow: true,
showSurfaceGrid: false,
keepAspectRatio: true,
verticalRatio: 0.5
};
Expand Down
8 changes: 8 additions & 0 deletions examples/graph3d/playground/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,10 @@ <h2>Options</h2>
<td>showAnimationControls</td>
<td><input type="checkbox" id="showAnimationControls" checked /></td>
</tr>
<tr>
<td>showGrayBottom</td>
<td><input type="checkbox" id="showGrayBottom" /></td>
</tr>
<tr>
<td>showGrid</td>
<td><input type="checkbox" id="showGrid" checked /></td>
Expand Down Expand Up @@ -137,6 +141,10 @@ <h2>Options</h2>
<td>showShadow</td>
<td><input type="checkbox" id="showShadow" /></td>
</tr>
<tr>
<td>showSurfaceGrid</td>
<td><input type="checkbox" id="showSurfaceGrid" checked /></td>
</tr>

<tr>
<td>keepAspectRatio</td>
Expand Down
2 changes: 2 additions & 0 deletions examples/graph3d/playground/playground.js
Original file line number Diff line number Diff line change
Expand Up @@ -405,13 +405,15 @@ function getOptions() {
height: document.getElementById("height").value,
style: document.getElementById("style").value,
showAnimationControls: (document.getElementById("showAnimationControls").checked != false),
showGrayBottom: (document.getElementById("showGrayBottom").checked != false),
showGrid: (document.getElementById("showGrid").checked != false),
showXAxis: (document.getElementById("showXAxis").checked != false),
showYAxis: (document.getElementById("showYAxis").checked != false),
showZAxis: (document.getElementById("showZAxis").checked != false),
showPerspective: (document.getElementById("showPerspective").checked != false),
showLegend: (document.getElementById("showLegend").checked != false),
showShadow: (document.getElementById("showShadow").checked != false),
showSurfaceGrid: (document.getElementById("showSurfaceGrid").checked != false),
keepAspectRatio: (document.getElementById("keepAspectRatio").checked != false),
verticalRatio: Number(document.getElementById("verticalRatio").value) || undefined,
animationInterval: Number(document.getElementById("animationInterval").value) || undefined,
Expand Down
43 changes: 34 additions & 9 deletions lib/graph3d/Graph3d.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,11 @@ Graph3d.DEFAULTS = {
showXAxis : true,
showYAxis : true,
showZAxis : true,
showGrayBottom : false,
showGrid : true,
showPerspective : true,
showShadow : false,
showSurfaceGrid : true,
keepAspectRatio : true,
rotateAxisLabels : true,
verticalRatio : 0.5, // 0.1 to 1.0, where 1.0 results in a 'cube'
Expand Down Expand Up @@ -1849,6 +1851,7 @@ Graph3d.prototype._redrawSurfaceGraphPoint = function(ctx, point) {
var topSideVisible = true;
var fillStyle;
var strokeStyle;
var cosViewAngle;

if (this.showGrayBottom || this.showShadow) {
// calculate the cross product of the two vectors from center
Expand All @@ -1857,14 +1860,25 @@ Graph3d.prototype._redrawSurfaceGraphPoint = function(ctx, point) {
// for calculating light intensity
var aDiff = Point3d.subtract(cross.trans, point.trans);
var bDiff = Point3d.subtract(top.trans, right.trans);
var crossproduct = Point3d.crossProduct(aDiff, bDiff);
var len = crossproduct.length();
// FIXME: there is a bug with determining the surface side (shadow or colored)
var surfaceNormal = Point3d.crossProduct(aDiff, bDiff);

topSideVisible = (crossproduct.z > 0);
if (this.showPerspective) {
let surfacePosition = Point3d.avg(
Point3d.avg(point.trans, cross.trans),
Point3d.avg(right.trans, top.trans));
// This corresponds to diffuse lighting with light source at (0, 0, 0).
// More generally, we would need `surfacePosition - lightPosition`:
cosViewAngle = -Point3d.dotProduct(
surfaceNormal.normalize(),
surfacePosition.normalize())
}
else {
cosViewAngle = surfaceNormal.z / surfaceNormal.length();
}
topSideVisible = cosViewAngle > 0;
}

if (topSideVisible) {
if (topSideVisible || !this.showGrayBottom) {

// calculate Hue from the current value. At vMin the hue is 240, at vMax the hue is 0
var vAvg = (point.point.value + right.point.value + top.point.value + cross.point.value) / 4;
Expand Down Expand Up @@ -1892,26 +1906,37 @@ Graph3d.prototype._redrawSurfaceGraphPoint = function(ctx, point) {
var maxB = colors[endIndex].b;
var b = minB + ratioWithin*(maxB - minB);

if (this.showShadow) {
var v = (1 + cosViewAngle) / 2; // value. TODO: scale
r *= v;
g *= v;
b *= v;
}

fillStyle = `RGB(${r}, ${g}, ${b})`;
} else {
var h = ratio * 240;
var s = 1; // saturation
var v = 1; // value
if (this.showShadow) {
v = Math.min(1 + (crossproduct.x / len) / 2, 1); // value. TODO: scale
v = (1 + cosViewAngle) / 2; // value. TODO: scale
fillStyle = this._hsv2rgb(h, s, v);
strokeStyle = fillStyle;
}
else {
v = 1;
fillStyle = this._hsv2rgb(h, s, v);
strokeStyle = this.axisColor; // TODO: should be customizable
}
}
}
else {
fillStyle = 'gray';
strokeStyle = this.axisColor;
}

if (this.showSurfaceGrid) {
strokeStyle = this.axisColor; // TODO: should be customizable
}
else {
strokeStyle = fillStyle;
}

ctx.lineWidth = this._getStrokeWidth(point);
Expand Down
33 changes: 32 additions & 1 deletion lib/graph3d/Point3d.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,27 @@ Point3d.avg = function(a, b) {
);
};

/**
* Scale the provided point by a scalar, returns p*c
* @param {Point3d} p
* @param {number} c
* @return {Point3d} p*c
*/
Point3d.scalarProduct = function(p, c) {
return new Point3d(p.x * c, p.y * c, p.z * c);
};

/**
* Calculate the dot product of the two provided points, returns a.b
* Documentation: http://en.wikipedia.org/wiki/Dot_product
* @param {Point3d} a
* @param {Point3d} b
* @return {Point3d} dot product a.b
*/
Point3d.dotProduct = function(a, b) {
return a.x * b.x + a.y * b.y + a.z * b.z;
};

/**
* Calculate the cross product of the two provided points, returns axb
* Documentation: http://en.wikipedia.org/wiki/Cross_product
Expand All @@ -71,7 +92,7 @@ Point3d.crossProduct = function(a, b) {


/**
* Rtrieve the length of the vector (or the distance from this point to the origin
* Retrieve the length of the vector (or the distance from this point to the origin
* @return {number} length
*/
Point3d.prototype.length = function() {
Expand All @@ -82,4 +103,14 @@ Point3d.prototype.length = function() {
);
};


/**
* Return a normalized vector pointing in the same direction.
* @return {Point3d} normalized
*/
Point3d.prototype.normalize = function() {
return Point3d.scalarProduct(this, 1/this.length());
};


module.exports = Point3d;
9 changes: 7 additions & 2 deletions lib/graph3d/Settings.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,11 @@ var OPTIONKEYS = [
'showXAxis',
'showYAxis',
'showZAxis',
'showGrayBottom',
'showGrid',
'showPerspective',
'showShadow',
'showSurfaceGrid',
'keepAspectRatio',
'rotateAxisLabels',
'verticalRatio',
Expand Down Expand Up @@ -247,7 +249,6 @@ function setDefaults(src, dst) {

// Following are internal fields, not part of the user settings
dst.margin = 10; // px
dst.showGrayBottom = false; // TODO: this does not work correctly
dst.showTooltip = false;
dst.onclick_callback = null;
dst.eye = new Point3d(0, 0, -1); // TODO: set eye.z about 3/4 of the width of the window?
Expand Down Expand Up @@ -480,9 +481,13 @@ function setDataColor(dataColor, dst) {
* @param {Object} dst
*/
function setSurfaceColor(surfaceColors, dst) {
if(surfaceColors === undefined) {
if(surfaceColors === undefined || surfaceColors === true) {
return; // Nothing to do
}
if (surfaceColors === false) {
dst.surfaceColors = undefined;
return;
}

if (dst.surfaceColors === undefined) {
dst.surfaceColors = {};
Expand Down
4 changes: 3 additions & 1 deletion lib/graph3d/options.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ let surfaceColorsOptions = {
colorStops : { number },
__type__ : { object },
},
__type__ : { array, object },
__type__ : { boolean: bool, array, object },
};

/**
Expand Down Expand Up @@ -79,10 +79,12 @@ let allOptions = {
yMax : { number, 'undefined': 'undefined' },
zMax : { number, 'undefined': 'undefined' },
showAnimationControls: { boolean: bool, 'undefined': 'undefined' },
showGrayBottom : { boolean: bool },
showGrid : { boolean: bool },
showLegend : { boolean: bool, 'undefined': 'undefined' },
showPerspective : { boolean: bool },
showShadow : { boolean: bool },
showSurfaceGrid : { boolean: bool },
showXAxis : { boolean: bool },
showYAxis : { boolean: bool },
showZAxis : { boolean: bool },
Expand Down

0 comments on commit 9cefeaa

Please sign in to comment.