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

Apply color palette on render. #4953

Closed
wants to merge 18 commits into from

Conversation

NaderCHASER
Copy link
Contributor

@NaderCHASER NaderCHASER commented Feb 6, 2017

  • CustomTemplateImageryProvider is based on UrlTemplateImageryProvider. Two main differences are it accepts a colorPalette array along with min/max values to pass to the tiler. The tiler can use the min/max to "bake in" a raw value into the PNG tile through the red channel.
  • GLSL modifications are a nightmare. Duplicating code was the only way I could get this to reliably work due to an inability to supply a texture argument to sampleAndBlend. Need insight on how to best approach this.
  • Is creating the new Texture in ImageryLayer appropriate?

This PR needs help and I know it. I've been posting on the forum but have yet to receive any real response from Ceisum graphics programmers.

screen shot 2017-02-06 at 11 42 30 am
screen shot 2017-02-02 at 9 33 48 pm

Here's a task list:

  • WEBGL RENDER WARNING: there is no texture bound to the unit #
  • Palette has to be assigned after load. Can this be moved to the options somehow?
  • Palette weirdness on zooming (see attachments) Determined to be a data issue. Fixed URL at c00a7b4
  • When the Imagery Layer is removed, the edge of the Earth still has the last color palette applied. Does the texture need to be destroyed somehow?
  • Determine if colorPalette texture is being created in proper location (ImageryLayer.js) and assigned to the proper variable (should it be private/separate from the option?)
  • Using the Imagery Layer's min/max, add pickFeatures capabilities to display the value at a particular point. (Should it use ImageryLayer._imageryCache?)
  • Write tests

https://groups.google.com/forum/#!msg/cesium-dev/CE_91W9dPUA/g9DrqgPJBAAJ
https://groups.google.com/forum/#!topic/cesium-dev/J3ws_ikULrk
https://groups.google.com/forum/#!searchin/cesium-dev/Custom$20image$20filters$20in$20SingleTileImageryProvider%7Csort:relevance/cesium-dev/aysBQWn9c88/JoR0dpQ_DAAJ

NASA JPL's use-case for this:
at9_tld6

@hpinkos
Copy link
Contributor

hpinkos commented Feb 6, 2017

Thanks for the pull request @NaderCHASER! Someone should be able to respond to this during our Bug Bash later this week.

@pjcozzi
Copy link
Contributor

pjcozzi commented Feb 6, 2017

Looks good, thanks @NaderCHASER!

Can you please send in a CLA so we can review this?

@NaderCHASER
Copy link
Contributor Author

NaderCHASER commented Feb 6, 2017

@pjcozzi

"You or any other entity (including a cross-claim or counterclaim in a
awsuit) alleging that your Contribution"

"lawsuit" is missing the "l". Filling it out and submitting it now.

Edit: Completed and sent.

@NaderCHASER
Copy link
Contributor Author

NaderCHASER commented Feb 8, 2017

I have a server-side tiler up and running that can be used.

http://cache.allisonhouse.com/maps/raw.php?model=rtma&run=201702081600&hour=1486569600000&layers=tmp_2m_0&z=3&x=1&y=3&min=-60&max=120

This is the current 2-meter temperature for the US. Min/max must be supplied for the tiler to determine the range to use in the red channel.

Here's a sample color palette: http://cache.allisonhouse.com/maps/palettes/TMP_2M.pal

Edit: Removed code due to poor formatting. Everything is now in Apps/Sandcastle/gallery/Imagery Layers Palette.html.

And here is what the rendered image should look like (but doesn't, see post 1):
screen shot 2017-02-08 at 10 18 45 am

@bagnell
Copy link
Contributor

bagnell commented Feb 8, 2017

I haven't been able to run an example. Can you post a Sandcastle gist? @kring would know where this fits better.

I read through the forum posts that you linked. Was the only problem in the second post that sampleAndBlend would not compile without passing the last sampler2D? Have you tried using #ifdefs to remove the parameter? Try

vec4 sampleAndBlend(
    vec4 previousColor,
    sampler2D texture,
#ifdef APPLY_COLOR_PALETTE
    sampler2D textureColorPalette,
#endif
    vec2 tileTextureCoordinates,
    vec4 textureCoordinateRectangle,
    vec4 textureCoordinateTranslationAndScale,
    float textureAlpha,
    float textureBrightness,
    float textureContrast,
    float textureHue,
    float textureSaturation,
    float textureOneOverGamma,
    float split)
{
//...

and rearranging the call in GlobeSurfaceShaderSet.js.

@NaderCHASER
Copy link
Contributor Author

NaderCHASER commented Feb 8, 2017

@bagnell I haven't tried the #ifdef in the parameter. I actually didn't know that was possible. This whole compiled GLSL (and just GLSL in general) is rather new to me.

I'm working on a sandcastle and the suggested change now. Will update in an hour or so once I've completed them.

@NaderCHASER
Copy link
Contributor Author

APPLY_COLOR_PALETTE will be set if there are any Imagery Layers which support a color palette. This means the argument will be bound in the shader and it will fail to compile due to not being able to provide a texture for certain layers.

I've committed a fix for an accidental code removal (frustumSplits[] in FrameState.js). I've also comitted a sandcastle. The problem is that I can't test the sandcastle myself due to a CORS issue. I'll be doing some server-side stuff to get a public URL which has "Access-Control-Allow-Origin: *"

* Removes unique sampleBlendAndPalette() function which was mostly duplicated.
* Adds a second boolean argument to tell the function whether or not to palette the imagery.
@NaderCHASER
Copy link
Contributor Author

NaderCHASER commented Feb 8, 2017

I've fixed the URL in the sandcastle. It also now responds to CORS appropriately.

screen shot 2017-02-08 at 1 39 44 pm

screen shot 2017-02-08 at 1 42 21 pm

@@ -38,6 +38,10 @@ uniform float u_dayTextureOneOverGamma[TEXTURE_UNITS];
uniform vec4 u_dayTextureTexCoordsRectangle[TEXTURE_UNITS];
#endif

#ifdef APPLY_COLOR_PALETTE
uniform sampler2D u_dayTextureColorPalette[TEXTURE_UNITS];
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 causing the WebGL warnings. There aren't as many color palettes as there are TEXTURE_UNITS. Define another constant for the number of color palettes and use that as the size of the array.

@@ -67,6 +71,7 @@ varying vec3 v_mieColor;
vec4 sampleAndBlend(
vec4 previousColor,
sampler2D texture,
sampler2D textureColorPalette,
Copy link
Contributor

@bagnell bagnell Feb 8, 2017

Choose a reason for hiding this comment

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

Since you don't have the same number of color palettes, create a uniform of indices into u_dayTextureColorPalette for each texture.

Copy link
Contributor

Choose a reason for hiding this comment

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

Or you can pass the index directly here

@@ -110,6 +116,25 @@ vec4 sampleAndBlend(
}
#endif

if(applyTextureColorPalette > 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.

If the index is a valid value, say >=0 execute this code.

a1 = mix(tA, tB, f.y);
}

vec4 pixColor = texture2D(textureColorPalette, vec2(0.0, a1));
Copy link
Contributor

Choose a reason for hiding this comment

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

Index directly into u_dayTextureColorPalette 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.

Using u_dayTextureColorPalette[index] here results in

'[]' : Index expression must be constant

error.
* Creates a `PALETTE_UNITS` constant to fix `bindTexture` errors.
* Passes a boolean to sampleAndBlend to tell it whether or not to palette the dayTexture.
@NaderCHASER
Copy link
Contributor Author

NaderCHASER commented Feb 8, 2017

I've pushed up a change which is an adaptation of your recommendation. I don't believe I can pass the index as an argument in GLSL because the array is of an unknown length at compile time. Correct me if I'm wrong here, but I just couldn't ever get that to work.

I did, however, create a PALETTE_UNITS constant to correct the bindTexture errors. I still get very weird results. I can't tell if these are due to the GLSL, or something else.

Edit: Add setTimeout(function() { layers.remove(layer) }, 10000); to the end of the sandcastle to see the following screenshot.

screen shot 2017-02-08 at 3 42 14 pm

Zooming creates some weirdness as well.
screen shot 2017-02-08 at 3 25 09 pm

@NaderCHASER
Copy link
Contributor Author

Code has been updated to pass all checks. Issue list is in the top post.

Biggest current issues are a) removing the "paletted" layer causes the underlying imagery to be paletted at the top edge and b) zooming in/out can lead to a strip of tiles not being paletted.

@pjcozzi
Copy link
Contributor

pjcozzi commented Feb 18, 2017

Thanks for all the updates, @NaderCHASER!

@bagnell when you have the chance, perhaps after #4979 and the orthographic projection, can you do another review here?

@cesium-concierge
Copy link

Congratulations on closing the issue! I found these Cesium forum links in the comments above:

https://groups.google.com/forum/#!msg/cesium-dev/CE_91W9dPUA/g9DrqgPJBAAJ
https://groups.google.com/forum/#!topic/cesium-dev/J3ws_ikULrk
https://groups.google.com/forum/#!searchin/cesium-dev/Custom$20image$20filters$20in$20SingleTileImageryProvider%7Csort

If this issue affects any of these threads, please post a comment like the following:

The issue at #4953 has just been closed and may resolve your issue. Look for the change in the next stable release of Cesium or get it now in the master branch on GitHub https://github.com/AnalyticalGraphicsInc/cesium.


I am a bot who helps you make Cesium awesome! Contributions to my configuration are welcome.

🌍 🌎 🌏

@NaderCHASER NaderCHASER deleted the tile-palette branch November 16, 2017 16:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants