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.

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

Add Material Support to Globe #5919

Merged
merged 35 commits into from
Nov 25, 2017
Merged

Conversation

jasonbeverage
Copy link
Contributor

Hi all!

This PR adds Material support to the Globe, as well as adding new height and slope values to the czm_materialInput struct so that it can be used as input when generating the materials.

I've added a new sandcastle example that you can play with here

The main goal of this effort was to visualize changes in slope on the terrain, but I tried to make it more generic by utilizing the existing Material support so we could do some more interesting effects in the future. I've added some new materials that you can see in use in the sandcastle example

  • ElevationContour - Displays elevation contour lines on the terrain with the spacing controled via a uniform. You can drag the slider in the sandcastle example to change the spacing.
  • ElevationRamp - Colors the terrain by applying a color ramp to the elevation. The elevation is scaled between minHeight and maxHeight uniforms and then does a texture lookup into the ramp to get the color. This ended up being really cool because you can generate nice color ramps programatically using a canvas and the createLinearGradient function.
  • SlopeRamp - Colors the terrain by applying a color ramp to the slope.

Some notes on the implementation:

  • I tried to keep the changes as minimal as possible and just re-use the great work already done in the existing Material framework.
  • The Globe now has a new material property which you can set to any Material you like.
  • Globe has a new dirtyShaders function that is called when you change the material so the correct material shader is injected in the globe shader.
  • If there is no material there shouldn't be a performance penalty b/c it's wrapped in a APPLY_MATERIAL define which is only set if there is material present.
  • GlobeSurfaceShader has a new material property so it can detect when the material has changed and a new shader needs to be generated.
  • GlobeSurfaceTileProvider has a new uniformMap property that is set to the materials uniforms so they are incorporated in the tile draw calls.
  • Currently the material only effects the diffuse color of the terrain. The original imagery color is blended with the materials color based on the materials alpha. This allows you to return an alpha value less than 1 to get the original imagery color to blend though. For example, try changing one of the color stops in the color ramp to "transparent" or "rgba(255,0,0,0.5)" and see the imagery come through.

Thanks!

Jason

heightramp

contourmap

@pjcozzi
Copy link
Contributor

pjcozzi commented Oct 21, 2017

@jasonbeverage this is super exciting, thanks again!

Here's Seneca Rocks where I use to rock climb:

image

For the contours, consider adding an optional where they "maintain" constant pixel width using screen-space partial derivatives. Perhaps something like this: https://github.com/AnalyticalGraphicsInc/cesium/blob/a81f2e7de64c2348558046b5e8e685278fde0dcf/Source/Shaders/Materials/GridMaterial.glsl#L23

@lilleyse can you take a first pass at this? I would like to do a quick review before merging.

@jasonbeverage
Copy link
Contributor Author

jasonbeverage commented Oct 21, 2017 via email

@pasu
Copy link
Contributor

pasu commented Nov 2, 2017

Hi, I also did this job but I just modified the globefs directly, I never thought adding material to support globe, it is a decent design, I gained it.

Here is my little suggestion to set the contour line.
Source/Shaders/Materials/ElevationContourMaterial.glsl:
czm_material material = czm_getDefaultMaterial(materialInput);

float dContourInterval = 100.0;// the interval value of contour line
float distanceToContour = mod(materialInput.height, dContourInterval);
float dxc = abs(dFdx(materialInput.height));
float dyc = abs(dFdy(materialInput.height));
float dF = max(dxc, dyc);
material.alpha = (distanceToContour < dF) ? 1.0 : 0.0;

material.diffuse = color.rgb;

return material;

Copy link
Contributor

@lilleyse lilleyse left a comment

Choose a reason for hiding this comment

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

This looks great so far! I have a few structural comments, but overall the approach is solid.

});
viewer.terrainProvider = cesiumTerrainProviderMeshes;

viewer.scene.globe.enableLighting = true;
Copy link
Contributor

Choose a reason for hiding this comment

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

The demo works better without lighting. Related to my comment below, we should be able to get the normals properly without this being set.

Copy link
Contributor

Choose a reason for hiding this comment

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

The color ramps are better with lighting so you can see the terrain features. The terrain all blends together without the shadows.

Copy link
Contributor

@lilleyse lilleyse Nov 8, 2017

Choose a reason for hiding this comment

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

The lighting does look fine most of the time, it's mainly when it's completely dark that it's hard to see what's going on.

Copy link
Contributor

Choose a reason for hiding this comment

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

I disagree. For example, there are some random Appalachian mountains with and without lighting:

image

image

Having the lighting enabled makes a big difference.
To make sure the lighting always looks good with the preset views, we can add a line to set the clock time:
viewer.clockViewModel.currentTime = Cesium.JulianDate.fromDate(new Date(2017, 8, 22, 0, 0, 0));

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok yeah, setting the clock is good. I should have been clearer, my main issue was that whenever I would open the demo it would be night at Everest and just hard to see anything.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've pushed a change to set the clock time.

viewer.scene.globe.material.uniforms.color = Cesium.Color.RED;
}
}, {
text : 'Slope Color Ramp Material',
Copy link
Contributor

Choose a reason for hiding this comment

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

This material doesn't work with enableLighting off.

If the material requires normals you can pass in the define GENERATE_POSITION_AND_NORMAL to the globe vertex shader.

For terrain that doesn't contain normals the material should be ignored so it doesn't appear black.

@@ -525,6 +540,10 @@ define([
return;
}

if (this._material) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Use defined(this._material).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.


var fragmentSources = [];
var fragmentDefines = [];
if (this._material) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Same comment.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

fragmentDefines.push('APPLY_MATERIAL');

// Set the material uniform map to the materials
this._surface._tileProvider.uniformMap = this._material._uniforms;
Copy link
Contributor

Choose a reason for hiding this comment

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

uniformMap should be defined in the constructor of GlobeSurfaceTileProvider.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also a better name would be materialUniformMap.

@@ -1262,6 +1264,10 @@ define([
uniformMapProperties.minMaxHeight.y = encoding.maximumHeight;
Matrix4.clone(encoding.matrix, uniformMapProperties.scaleAndBias);

if (tileProvider.uniformMap) {
uniformMap = combine(uniformMap, tileProvider.uniformMap);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be best to combine uniforms elsewhere since this will allocate a uniform map object every frame.

I wonder if this could be moved to the area near if (tileProvider._drawCommands.length <= tileProvider._usedDrawCommands) {. Can it detect when the material is dirty and combine up there?

@@ -21,6 +21,8 @@ varying vec3 v_positionEC;
varying vec3 v_textureCoordinates;
varying vec3 v_normalMC;
varying vec3 v_normalEC;
varying float v_slope;
Copy link
Contributor

Choose a reason for hiding this comment

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

Everything in here should also be surrounded by the APPLY_MATERIAL define too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

@@ -61,6 +61,8 @@ varying vec3 v_positionEC;
varying vec3 v_textureCoordinates;
varying vec3 v_normalMC;
varying vec3 v_normalEC;
varying float v_height;
varying float v_slope;
Copy link
Contributor

Choose a reason for hiding this comment

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

Surround these with the APPLY_MATERIAL define.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

@@ -1262,6 +1264,10 @@ define([
uniformMapProperties.minMaxHeight.y = encoding.maximumHeight;
Matrix4.clone(encoding.matrix, uniformMapProperties.scaleAndBias);

if (tileProvider.uniformMap) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Use defined.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

@lilleyse
Copy link
Contributor

lilleyse commented Nov 3, 2017

Added some style tweaks in 60c0121

@lilleyse
Copy link
Contributor

lilleyse commented Nov 3, 2017

Remaining TODO:

uniforms : {
image: Material.DefaultImageId,
minHeight: 0.0,
maxHeight: 10000.0,
Copy link
Contributor

Choose a reason for hiding this comment

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

Change these to minimumHeight and maximumHeight. We try to avoid abbreviations so there's no confusion

Copy link
Contributor

Choose a reason for hiding this comment

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

Remove the dangling comma here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

@pjcozzi
Copy link
Contributor

pjcozzi commented Nov 12, 2017

@jasonbeverage are you able to make the suggested changes to get this over the finish line for the upcoming 1.40 release? 😄

@jasonbeverage
Copy link
Contributor Author

jasonbeverage commented Nov 13, 2017 via email

@pjcozzi
Copy link
Contributor

pjcozzi commented Nov 13, 2017

Very nice, rock on @jasonbeverage!

@jasonbeverage
Copy link
Contributor Author

I just pushed a change to the ElevationContourMaterial that uses the code from @pasu to implement the screen space partial derivative based contouring. I also added a new lineThickness uniform so you can control the thickness of the line. It looks pretty good to me.

I think my recent changes addresses everything mentioned here. Let me know if you see anything else!

Copy link
Contributor

@pjcozzi pjcozzi left a comment

Choose a reason for hiding this comment

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

@jasonbeverage this is good stuff as always! We would like to include this in the 1.40 release going out on Friday so we would like to get this into master ASAP.

Do you have time to
(1) make the mostly trivial changes I requested?
(2) add unit tests?

Thanks!!!

CHANGES.md Outdated
@@ -3,6 +3,7 @@ Change Log

### 1.40 - 2017-12-01

* Adds Material support to Globe. [#5919](https://github.com/AnalyticalGraphicsInc/cesium/pull/5919)
Copy link
Contributor

Choose a reason for hiding this comment

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

Can this link to the future Sandcastle example that will live here: https://cesiumjs.org/Cesium/Apps/Sandcastle/

ramp.height = 1;
var ctx = ramp.getContext('2d');

var values = selectedShading === 'elevation' ? elevationRamp : slopeRamp;
Copy link
Contributor

Choose a reason for hiding this comment

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

@hpinkos can the color ramp be tweaked to be a bit less subtle for the default view?

image

Copy link
Contributor

Choose a reason for hiding this comment

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

Or change the default view.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I changed the default view to be of the Himalayas

/**
* @private
*/
Globe.prototype.dirtyShaders = function() {
Copy link
Contributor

Choose a reason for hiding this comment

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

We would generally name this a verb like makeShadersDirty and not make it on the Globe object; instead, it would be a stand-alone function at file level scope and then we pass in a Globe object. This provides better encapsulation.

fragmentSources.push(this._material.shaderSource);
defines.push('APPLY_MATERIAL');

// Set the material uniform map to the materials
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably don't need this comment.


this._surfaceShaderSet.baseVertexShaderSource = new ShaderSource({
sources : [GroundAtmosphere, GlobeVS],
defines: defines
Copy link
Contributor

Choose a reason for hiding this comment

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

Throughout, we would generally write defines : with a space before the semicolon. The auto formatter in your IDE probably does this. WebStorm does.

* <ul>
* <li><code>color</code>: color and alpha for the contour line.</li>
* <li><code>spacing</code>: spacing for contour lines in meters.</li>
* <li><code>lineThickness</code>: Number specifying the thickness of the grid lines in pixels.</li>
Copy link
Contributor

Choose a reason for hiding this comment

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

To be consistent with the rest of the Cesium API, can this be "width" instead of "thickness?"

@@ -10,6 +10,8 @@
* @property {vec3} normalEC Unperturbed surface normal in eye coordinates.
* @property {mat3} tangentToEyeMatrix Matrix for converting a tangent space normal to eye space.
* @property {vec3} positionToEyeEC Vector from the fragment to the eye in eye coordinates. The magnitude is the distance in meters from the fragment to the eye.
* @property {float} height The height of the terrain. Only available for globe materials.
Copy link
Contributor

Choose a reason for hiding this comment

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

Can this be more explicit? I assume the height in meters above or below the WGS84 ellipsoid?

@@ -10,6 +10,8 @@
* @property {vec3} normalEC Unperturbed surface normal in eye coordinates.
* @property {mat3} tangentToEyeMatrix Matrix for converting a tangent space normal to eye space.
* @property {vec3} positionToEyeEC Vector from the fragment to the eye in eye coordinates. The magnitude is the distance in meters from the fragment to the eye.
* @property {float} height The height of the terrain. Only available for globe materials.
* @property {float} slope The slope of the terrain normalized from 0 to 1. Only available for globe materials.
Copy link
Contributor

Choose a reason for hiding this comment

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

Can this be more explicit, e.g., 0 is flat and 1 is vertical or vice versa?

@@ -167,4 +172,11 @@ void main()
v_rayleighColor = atmosColor.rayleigh;
v_distance = length((czm_modelView3D * vec4(position3DWC, 1.0)).xyz);
#endif

#ifdef APPLY_MATERIAL
vec3 finalNormal = normalize(v_normalMC);
Copy link
Contributor

Choose a reason for hiding this comment

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


#ifdef APPLY_MATERIAL
vec3 finalNormal = normalize(v_normalMC);
vec3 worldNormal = normalize(v_positionMC.xyz);
Copy link
Contributor

Choose a reason for hiding this comment

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

Here and the line above, we generally try to treat varyings in a vertex shader as "write once, don't read" - more for style - I don't think it matters for performance.

Here, the return from czm_octDecode could be assigned to a local, and position3DWC can be used instead of v_positionMC.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also worldNormal is a confusing name - since it is not the terrain normal in world coordinate - perhaps rename to ellipsoidNormal since this is normal to the ellipsoid?

@jasonbeverage
Copy link
Contributor Author

Hey @pjcozzi I'll try to make those changes tonight or tomorrow. Do you have an example of something I could use for inspiration for the unit tests? I'm still not the best at writing them :)

I'm heading out of town for the IITSEC conference on Sunday and won't be back until the end of the week, so I'll try to get what I can wrapped up before I head out.

@pjcozzi
Copy link
Contributor

pjcozzi commented Nov 25, 2017

@jasonbeverage awesome - it is deeply appreciated!

I'm not sure that it is the best example, but here is a simple example for rendering a globe in a unit test:

https://github.com/AnalyticalGraphicsInc/cesium/blob/master/Specs/Scene/SceneSpec.js#L488-L505

For your unit tests, just cover rendering with each type of material and all the edge cases, e.g., changing material, going from no material to material, from material to no material, perhaps changing uniforms, etc.

The test guiding is pretty is good:

https://github.com/AnalyticalGraphicsInc/cesium/tree/master/Documentation/Contributors/TestingGuide#testing-guide

Also:

  • Please keep your eye out for cool Cesium apps at IITSEC - would love to hear what you find!
  • What is your next Cesium feature? 😄

@jasonbeverage
Copy link
Contributor Author

I think I've addressed all of your comments except for the unit tests.

@jasonbeverage
Copy link
Contributor Author

I just added a few unit tests that appear to pass :)

I'm heading out for the rest of the day and will be gone until next Wednesday but I'll be answering email and checking this PR if you need anything ;)

@pjcozzi
Copy link
Contributor

pjcozzi commented Nov 25, 2017

Very nice, thanks again @jasonbeverage. Have a great conference!

@pjcozzi pjcozzi merged commit 7799186 into CesiumGS:master Nov 25, 2017
@jasonbeverage
Copy link
Contributor Author

Thanks for merging this! I'll hopefully have at least one more little PR before the end of the year :)

@MapGiser
Copy link

Hi, I also did this job but I just modified the globefs directly, I never thought adding material to support globe, it is a decent design, I gained it.
Here is my little suggestion to set the contour line.
Source/Shaders/Materials/ElevationContourMaterial.glsl:
czm_material material = czm_getDefaultMaterial(materialInput);
float dContourInterval = 100.0;// the interval value of contour line
float distanceToContour = mod(materialInput.height, dContourInterval);
float dxc = abs(dFdx(materialInput.height));
float dyc = abs(dFdy(materialInput.height));
float dF = max(dxc, dyc);
material.alpha = (distanceToContour < dF) ? 1.0 : 0.0;
material.diffuse = color.rgb;
return material;
I want to draw counterline in restrict area, can you give me some advise?thanks

@Abhishek950650
Copy link

@jasonbeverage. Hello i am trying to implement Cesium globe materials slope, elevation, aspect features in my local project but i am facing issue in my project if i apply any these options my globe is not showing perfect output its look like only single color on my whole globe 🌐 like red or blue or green
How i resolve this issue..

Thanks

@OmarShehata
Copy link
Contributor

@Abhishek950650 I would ask your question in the Cesium community forum (https://community.cesium.com/) so others can take a look and help out. Include more information like a Sandcastle and how you're using these materials (see https://community.cesium.com/t/how-to-share-custom-sandcastle-examples/9828)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants