Lens Flare #5498
Closed
Lens Flare #5498
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The algorithm of this Lens flare is, actually, for HDR texutre. But, Cesium does not use HDR texture. So, I have to modify the implementing way to fit with Cesium. I would like to notify some issues for later maintainer - Zero, there is some kind of initialization problem (Sean already knew this problem). It will disappear when the user updates variables ( ex) Lens Flare check box, Intensity slide .......) - First, about algorithm, instead of using HDR texture, I used Scene Image for creating Lensflare effect. So, I masked the portion of sun with shader codes, if then it can also address the situation what sun is blocked by the earth. - Second, at the inner Earth mode, if the sun exists with terrains in the screen space, the terrains are also used to create Lens flare effect. - Third, two texture made by me are being used for Lens Flare (DirtMaskTextureExample.png, StarBurst.png) It seems that those size are little bit bigger than the images already used for Cesium. Feel free to modify these, if you want
|
Very cool, thanks @byumjin! When I run your branch locally, I get this:
Perhaps I did something wrong merging in the latest version of the |
|
Could you also make this a Sandcastle example inside Cesium, including a screenshot? For examples, see https://github.com/AnalyticalGraphicsInc/cesium/tree/master/Apps/Sandcastle/gallery We can figure out the public API later since this is going into the in-progress |
|
Also, please add yourself to CONTRIBUTORS.md. |
|
Did you create both .png files yourself? If not, do you know the source(s)? I need to determine if we are allowed to use them and what credit they would require. Also, these .png files are pretty large. Do they need to be .png or could they be .jpg? Could they also be a lower resolution? Or would this impact visual quality too much? |
|
That initial image is due to all the post-processing stages being on by default in that branch. Also the initialization of the Sandcastle UI with the actual code is not quite right. A bunch of those changes will be fixed in the |
|
Umm. It seems there are no files updated by me.
But, Sean has confirmed that it works, correctly.
I'll forward the mails.
And, sure, ill send the example and a screenshot.
Lastly, about the externship, i will send another mail to you.
Thank you,
Kim
…On Mon, Jun 19, 2017 at 3:29 PM, Patrick Cozzi ***@***.***> wrote:
http://localhost:8080/Apps/Sandcastle/?src=Hello%20World.
html&label=Showcases&gist=f32e90b16dbefd3176d30bfcaea122e9
Could you also make this a Sandcastle example inside Cesium, including a
screenshot? For examples, see https://github.com/
AnalyticalGraphicsInc/cesium/tree/master/Apps/Sandcastle/gallery
We can figure out the public API later since this is going into the
in-progress post-processing branch.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#5498 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AVPCk1D9wcGaidrm9tp_nlMYc77WdBM1ks5sFswMgaJpZM4N9y9Y>
.
|
Closed
|
@lilleyse gotcha. Please bump when fixed. |
add Byumjin Kim
Add SendCastle for LensFlare
|
I made StartBurst.png by my self And, yes, it could be jpg and also find to use lower resolution version. |
Fixed Mix tap and spacevvvvvvvvvvvv
There is not an obvious license associated with that image so we can't include it. Do you want to try to create a new image? Or I can ask if someone on the Cesium team could do it. |
|
Yes, ill make it asap.
…On Mon, Jun 19, 2017 at 5:51 PM, Patrick Cozzi ***@***.***> wrote:
But, I modified the source image from below url to create
DirtMaskTextureExample,png
There is not an obvious license associated with that image so we can't
include it. Do you want to try to create a new image? Or I can ask if
someone on the Cesium team could do it.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#5498 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AVPCk7y9JmbZUgqQ5FGlKuKB-no00Vojks5sFu1ygaJpZM4N9y9Y>
.
|
Sry for concern about the source image. I just made new one. And disable other postprocess. Now, you can check the lens flare on SandCastle. But, it is still have an initialization issue.
|
Sry for concern about the source image. I just made new one. And disable other postprocess. Now, you can check the lens flare on SandCastle. But, it is still have an initialization issue. |
Delete previous images
|
Thanks for creating the new image! We'll do a code review soon. |
Optimization
|
@lilleyse Could you please do a review? |
pjcozzi
requested changes
Jun 23, 2017
Source/Scene/PostProcessorScene.js
Outdated
| ]; | ||
|
|
||
| this._postProcessor = new PostProcessor({ | ||
| stages : stages | ||
| }); | ||
| } | ||
|
|
||
| function createLensFlareStage() { | ||
| var url_Dirt = buildModuleUrl('Assets/Textures/LensFlare/DirtMask.jpg'); |
There was a problem hiding this comment.
For local variable naming conventions, we use camelCase.
If you haven't already browse through our Coding Guide.
Source/Scene/PostProcessorScene.js
Outdated
|
|
||
| var uniformValues = { | ||
|
|
||
| DirtTexture: url_Dirt, |
There was a problem hiding this comment.
Same camelCase comment here and throughout.
Source/Scene/PostProcessorScene.js
Outdated
| var url_Star = buildModuleUrl('Assets/Textures/LensFlare/StarBurst.jpg'); | ||
|
|
||
| var uniformValues = { | ||
|
|
Source/Scene/PostProcessorScene.js
Outdated
| 'uniform sampler2D u_DirtTexture; \n' + | ||
| 'uniform sampler2D u_StarTexture; \n' + | ||
|
|
||
| 'uniform float u_Distortion; \n' + |
There was a problem hiding this comment.
If it is currently possible with the post-processing framework, I would combine all these floats into a vec4...and generally pack uniforms together as much as possible. We'll still want to expose these in JavaScript as separate variables then map to the uniforms x, y, z, w components like we do here.
Since this shader isn't called often, this isn't a huge performance issue, but this is the common practice throughout Cesium.
If the post-process framework doesn't support this, @lilleyse can add it and we can wait on this change or @byumjin you are welcome to go for it.
There was a problem hiding this comment.
It does support vec4, should work by sending a Cartesian4 to the uniform values area.
Source/Scene/PostProcessorScene.js
Outdated
| 'varying vec2 v_textureCoordinates; \n' + | ||
|
|
||
| //return ndc from world coordinate biased EarthRadius | ||
| 'vec4 GetNDCFromWC(in vec3 WC, in float EarthRadius) \n' + |
There was a problem hiding this comment.
Could you make this a built-in Cesium GLSL function including a unit test?
For example, see
https://github.com/AnalyticalGraphicsInc/cesium/tree/master/Source/Shaders/Builtin/Functions
and
https://github.com/AnalyticalGraphicsInc/cesium/blob/master/Specs/Renderer/BuiltinFunctionsSpec.js
To learn more about unit tests in general, see our Testing Guide, especially the GLSL section.
Also rename this to czm_WorldCoordinatestoNDC like our color transform functions.
Source/Scene/PostProcessorScene.js
Outdated
| 'bool bSpace = true; \n' + | ||
|
|
||
| //whether it is in space or not | ||
| 'if(length(czm_viewerPositionWC.xyz) < 6500000.0) \n' + |
There was a problem hiding this comment.
How did you determine 6500000.0?
Source/Scene/PostProcessorScene.js
Outdated
| ' vec2 offset = fract(texcoord + ghostVec * float(i)); \n' + | ||
|
|
||
| //Only bright spots from the centre of the source image | ||
| //' float weight = length(vec2(0.5) - offset) / length(vec2(0.5)); \n' + |
There was a problem hiding this comment.
Delete commented out code.
If you really need it, it could stay for now, but once this is merged into master, we do not leave in commented out code.
Source/Scene/PostProcessorScene.js
Outdated
| //Rotating starburst texture's coordinate | ||
| ' vec3 camx = vec3(czm_view[0][0], czm_view[0][1], czm_view[0][2] ); \n' + | ||
| ' vec3 camz = vec3(czm_view[1][0], czm_view[1][1], czm_view[1][2] ); \n' + | ||
| ' float camrot = dot(camx, vec3(0, 0, 1)) + dot(camz, vec3(0, 1, 0)); \n' + |
There was a problem hiding this comment.
Throughout GLSL, when your intention is floating point, always include .0, e.g., 0.0, etc.
On some implementations, not doing so at least use to result in a compiler error.
Source/Scene/PostProcessorScene.js
Outdated
| ' vec3 camz = vec3(czm_view[1][0], czm_view[1][1], czm_view[1][2] ); \n' + | ||
| ' float camrot = dot(camx, vec3(0, 0, 1)) + dot(camz, vec3(0, 1, 0)); \n' + | ||
|
|
||
| ' mat3 rotation = mat3( \n' + |
There was a problem hiding this comment.
I don't trust that all GLSL compilers for factor out the common cos and sin calls. Call each function once above here and assign to local variables.
package-lock.json
Outdated
| @@ -0,0 +1,5024 @@ | |||
| { | |||
There was a problem hiding this comment.
Are you sure this file should be included? I think you should remove it from git.
|
Thanks for reviewing!
I'll refine it asap.
Hmm.. I don't remember what is package-lock.json
<#5498 (comment)>
and
its updated part.
I'll check it and remove the part if it is useless.
Thank you,
Kim.
…On Fri, Jun 23, 2017 at 7:52 AM, Patrick Cozzi ***@***.***> wrote:
***@***.**** requested changes on this pull request.
I did a quick review, mostly style changes. @lilleyse
<https://github.com/lilleyse> or @bagnell <https://github.com/bagnell>
should still look at this.
------------------------------
In Source/Scene/PostProcessorScene.js
<#5498 (comment)>
:
> ];
this._postProcessor = new PostProcessor({
stages : stages
});
}
+ function createLensFlareStage() {
+ var url_Dirt = buildModuleUrl('Assets/Textures/LensFlare/DirtMask.jpg');
For local variable naming conventions, we use camelCase.
If you haven't already browse through our Coding Guide
<https://github.com/AnalyticalGraphicsInc/cesium/tree/master/Documentation/Contributors/CodingGuide>
.
------------------------------
In Source/Scene/PostProcessorScene.js
<#5498 (comment)>
:
> ];
this._postProcessor = new PostProcessor({
stages : stages
});
}
+ function createLensFlareStage() {
+ var url_Dirt = buildModuleUrl('Assets/Textures/LensFlare/DirtMask.jpg');
+ var url_Star = buildModuleUrl('Assets/Textures/LensFlare/StarBurst.jpg');
+
+ var uniformValues = {
+
+ DirtTexture: url_Dirt,
Same camelCase comment here and throughout.
------------------------------
In Source/Scene/PostProcessorScene.js
<#5498 (comment)>
:
> ];
this._postProcessor = new PostProcessor({
stages : stages
});
}
+ function createLensFlareStage() {
+ var url_Dirt = buildModuleUrl('Assets/Textures/LensFlare/DirtMask.jpg');
+ var url_Star = buildModuleUrl('Assets/Textures/LensFlare/StarBurst.jpg');
+
+ var uniformValues = {
+
Remove extra whitespace.
------------------------------
In Source/Scene/PostProcessorScene.js
<#5498 (comment)>
:
> +
+ Distortion: 5.0,
+ NumofGhosts: 4,
+ GhostDispersal: 0.4,
+
+ HaloWidth: 0.4,
+
+ EarthRadius: 0.0
+ };
+
+ var LensFlareShader =
+ 'uniform sampler2D u_colorTexture; \n' +
+ 'uniform sampler2D u_DirtTexture; \n' +
+ 'uniform sampler2D u_StarTexture; \n' +
+
+ 'uniform float u_Distortion; \n' +
If it is currently possible with the post-processing framework, I would
combine all these floats into a vec4...and generally pack uniforms
together as much as possible. We'll still want to expose these in
JavaScript as separate variables then map to the uniforms x, y, z, w
components like we do here
<https://github.com/AnalyticalGraphicsInc/cesium/blob/master/Source/Shaders/BillboardCollectionVS.glsl>
.
Since this shader isn't called often, this isn't a huge performance issue,
but this is the common practice throughout Cesium.
If the post-process framework doesn't support this, @lilleyse
<https://github.com/lilleyse> can add it and we can wait on this change
or @byumjin <https://github.com/byumjin> you are welcome to go for it.
------------------------------
In Source/Scene/PostProcessorScene.js
<#5498 (comment)>
:
> + var LensFlareShader =
+ 'uniform sampler2D u_colorTexture; \n' +
+ 'uniform sampler2D u_DirtTexture; \n' +
+ 'uniform sampler2D u_StarTexture; \n' +
+
+ 'uniform float u_Distortion; \n' +
+ 'uniform float u_GhostDispersal; \n' +
+ 'uniform float u_HaloWidth; \n' +
+ 'uniform float u_EarthRadius; \n' +
+
+ 'uniform float u_Intensity; \n' +
+
+ 'varying vec2 v_textureCoordinates; \n' +
+
+ //return ndc from world coordinate biased EarthRadius
+ 'vec4 GetNDCFromWC(in vec3 WC, in float EarthRadius) \n' +
Could you make this a built-in Cesium GLSL function including a unit test?
For example, see
https://github.com/AnalyticalGraphicsInc/cesium/
tree/master/Source/Shaders/Builtin/Functions
and
https://github.com/AnalyticalGraphicsInc/cesium/
blob/master/Specs/Renderer/BuiltinFunctionsSpec.js
To learn more about unit tests in general, see our Testing Guide
<https://github.com/AnalyticalGraphicsInc/cesium/tree/master/Documentation/Contributors/TestingGuide#testing-guide>,
especially the GLSL section
<https://github.com/AnalyticalGraphicsInc/cesium/tree/master/Documentation/Contributors/TestingGuide#glsl>
.
Also rename this to czm_WorldCoordinatestoNDC like our color transform
functions.
------------------------------
In Source/Scene/PostProcessorScene.js
<#5498 (comment)>
:
> +
+ 'varying vec2 v_textureCoordinates; \n' +
+
+ //return ndc from world coordinate biased EarthRadius
+ 'vec4 GetNDCFromWC(in vec3 WC, in float EarthRadius) \n' +
+ '{ \n' +
+ ' vec4 position = vec4(WC, 1.0); \n' +
+ ' vec4 positionEC = czm_view * position; \n' +
+ ' positionEC = vec4(positionEC.x + EarthRadius , positionEC.y, positionEC.z, 1.0); \n' +
+ ' vec4 positionWC = czm_eyeToWindowCoordinates(positionEC); \n' +
+ ' return czm_viewportOrthographic * vec4(positionWC.xy, -positionWC.z, 1.0); \n' +
+ '} \n' +
+
+ //Check if current pixel is included Earth
+ //if then mask it gradually
+ 'float IsinEarth(in vec2 texcoord, in vec2 SceneSize) \n' +
Throughout GLSL, we don't use the in, etc. parameter prefixes.
------------------------------
In Source/Scene/PostProcessorScene.js
<#5498 (comment)>
:
> + ' positionEC = vec4(positionEC.x + EarthRadius , positionEC.y, positionEC.z, 1.0); \n' +
+ ' vec4 positionWC = czm_eyeToWindowCoordinates(positionEC); \n' +
+ ' return czm_viewportOrthographic * vec4(positionWC.xy, -positionWC.z, 1.0); \n' +
+ '} \n' +
+
+ //Check if current pixel is included Earth
+ //if then mask it gradually
+ 'float IsinEarth(in vec2 texcoord, in vec2 SceneSize) \n' +
+ '{ \n' +
+
+ 'vec2 NDC = texcoord * 2.0 - 1.0; \n' +
+ 'vec4 EarthPosSC = GetNDCFromWC(vec3(0.0,0.0,0.0), 0.0); \n' +
+ 'vec4 EarthPosSCEdge = GetNDCFromWC(vec3( 0.0,0.0,0.0), u_EarthRadius *1.5); \n' +
+
+ 'NDC.xy -= EarthPosSC.xy; \n' +
+ 'float X = abs(NDC.x)*SceneSize.x; \n' +
Fix whitespace. Just
float x = abs(ndc.x) * sceneSize.x;
(and also fixed naming convention).
------------------------------
In Source/Scene/PostProcessorScene.js
<#5498 (comment)>
:
> + ' positionEC = vec4(positionEC.x + EarthRadius , positionEC.y, positionEC.z, 1.0); \n' +
+ ' vec4 positionWC = czm_eyeToWindowCoordinates(positionEC); \n' +
+ ' return czm_viewportOrthographic * vec4(positionWC.xy, -positionWC.z, 1.0); \n' +
+ '} \n' +
+
+ //Check if current pixel is included Earth
+ //if then mask it gradually
+ 'float IsinEarth(in vec2 texcoord, in vec2 SceneSize) \n' +
+ '{ \n' +
+
+ 'vec2 NDC = texcoord * 2.0 - 1.0; \n' +
+ 'vec4 EarthPosSC = GetNDCFromWC(vec3(0.0,0.0,0.0), 0.0); \n' +
+ 'vec4 EarthPosSCEdge = GetNDCFromWC(vec3( 0.0,0.0,0.0), u_EarthRadius *1.5); \n' +
+
+ 'NDC.xy -= EarthPosSC.xy; \n' +
+ 'float X = abs(NDC.x)*SceneSize.x; \n' +
The GLSL could use a lot of tweaks like this to look like our other GLSL
code. Feel free to make them now or wait until this is in a separate .glsl
file where it would be easier and the WebStorm auto formatter might work
(we use it for JavaScript).
------------------------------
In Source/Scene/PostProcessorScene.js
<#5498 (comment)>
:
> + 'float Y = abs(NDC.y)*SceneSize.y; \n' +
+ 'Y *= Y; \n' +
+
+ 'return clamp(0.0, 1.0, max( sqrt(X + Y) / max(abs(EarthPosSCEdge.x*SceneSize.x), 1.0) - 0.8 , 0.0)); \n' +
+
+ '} \n' +
+
+
+ //For Chromatic effect
+ 'vec4 textureDistorted( \n' +
+ ' in sampler2D tex, \n' +
+ ' in vec2 texcoord, \n' +
+ ' in vec2 direction, \n' +
+ ' in vec3 distortion, \n' +
+ ' in bool bSpace \n' +
+ ' ) { \n' +
In GLSL, we put { on a new line.
------------------------------
In Source/Scene/PostProcessorScene.js
<#5498 (comment)>
:
> + 'X *= X; \n' +
+ 'float Y = abs(NDC.y)*SceneSize.y; \n' +
+ 'Y *= Y; \n' +
+
+ 'return clamp(0.0, 1.0, max( sqrt(X + Y) / max(abs(EarthPosSCEdge.x*SceneSize.x), 1.0) - 0.8 , 0.0)); \n' +
+
+ '} \n' +
+
+
+ //For Chromatic effect
+ 'vec4 textureDistorted( \n' +
+ ' in sampler2D tex, \n' +
+ ' in vec2 texcoord, \n' +
+ ' in vec2 direction, \n' +
+ ' in vec3 distortion, \n' +
+ ' in bool bSpace \n' +
Do not use hungarian notation.
------------------------------
In Source/Scene/PostProcessorScene.js
<#5498 (comment)>
:
> + 'float red = 0.0; \n' +
+ 'float green = 0.0; \n' +
+ 'float blue = 0.0; \n' +
+ 'if(bSpace) \n' +
+ '{ \n' +
+ 'red = IsinEarth(texcoord + direction * distortion.r, SceneSize) * texture2D(tex, texcoord + direction * distortion.r).r; \n' +
+ 'green = IsinEarth(texcoord + direction * distortion.g, SceneSize) * texture2D(tex, texcoord + direction * distortion.g).g; \n' +
+ 'blue = IsinEarth(texcoord + direction * distortion.b, SceneSize) * texture2D(tex, texcoord + direction * distortion.b).b; \n' +
+ '} \n' +
+ 'else \n' +
+ '{ \n' +
+ 'red = texture2D(tex, texcoord + direction * distortion.r).r; \n' +
+ 'green = texture2D(tex, texcoord + direction * distortion.g).g; \n' +
+ 'blue = texture2D(tex, texcoord + direction * distortion.b).b; \n' +
+ '} \n' +
+
Here are throughout, no need to have all this extra whitespace. We never
use more than one blank line.
------------------------------
In Source/Scene/PostProcessorScene.js
<#5498 (comment)>
:
> +
+ 'return vec4(clamp(red, 0.0, 1.0), clamp(green, 0.0, 1.0), clamp(blue, 0.0, 1.0), 0.0); \n' +
+
+ '} \n' +
+
+
+
+ 'void main(void) \n' +
+ '{ \n' +
+ ' vec3 rgb = texture2D(u_colorTexture, v_textureCoordinates).rgb; \n' +
+
+
+ 'bool bSpace = true; \n' +
+
+ //whether it is in space or not
+ 'if(length(czm_viewerPositionWC.xyz) < 6500000.0) \n' +
How did you determine 6500000.0?
------------------------------
In Source/Scene/PostProcessorScene.js
<#5498 (comment)>
:
> +
+ 'vec2 texelSize = 1.0 / czm_viewport.zw; \n' +
+ 'vec3 distortion = vec3(-texelSize.x * u_Distortion, 0.0, texelSize.x * u_Distortion); \n' +
+
+ // ghost vector to image centre:
+ 'vec2 ghostVec = (vec2(0.5) - texcoord) * u_GhostDispersal; \n' +
+ 'vec3 direction = normalize(vec3(ghostVec, 0.0)); \n' +
+
+ // sample ghosts:
+ ' vec4 result = vec4(0.0); \n' +
+ ' vec4 ghost = vec4(0.0); \n' +
+ ' for (int i = 0; i < 4; ++i) { \n' +
+ ' vec2 offset = fract(texcoord + ghostVec * float(i)); \n' +
+
+ //Only bright spots from the centre of the source image
+ //' float weight = length(vec2(0.5) - offset) / length(vec2(0.5)); \n' +
Delete commented out code.
If you really need it, it could stay for now, but once this is merged into
master, we do not leave in commented out code.
------------------------------
In Source/Scene/PostProcessorScene.js
<#5498 (comment)>
:
> + 'vec4 halo; \n' +
+ ' vec2 haloVec = normalize(ghostVec) * u_HaloWidth; \n' +
+ ' float WeightForHalo = length(vec2(0.5) - fract(texcoord + haloVec)) / length(vec2(0.5)); \n' +
+ ' WeightForHalo = pow(1.0 - WeightForHalo, 5.0); \n' +
+ ' halo = textureDistorted(u_colorTexture, texcoord + haloVec, direction.xy, distortion, bSpace) * WeightForHalo * 1.5; \n' +
+
+ ' result += halo; \n' +
+ ' result += texture2D(u_DirtTexture, v_textureCoordinates); \n' +
+
+
+
+
+ //Rotating starburst texture's coordinate
+ ' vec3 camx = vec3(czm_view[0][0], czm_view[0][1], czm_view[0][2] ); \n' +
+ ' vec3 camz = vec3(czm_view[1][0], czm_view[1][1], czm_view[1][2] ); \n' +
+ ' float camrot = dot(camx, vec3(0, 0, 1)) + dot(camz, vec3(0, 1, 0)); \n' +
Throughout GLSL, when your intention is floating point, always include .0,
e.g., 0.0, etc.
On some implementations, not doing so at least use to result in a compiler
error.
------------------------------
In Source/Scene/PostProcessorScene.js
<#5498 (comment)>
:
> + ' float WeightForHalo = length(vec2(0.5) - fract(texcoord + haloVec)) / length(vec2(0.5)); \n' +
+ ' WeightForHalo = pow(1.0 - WeightForHalo, 5.0); \n' +
+ ' halo = textureDistorted(u_colorTexture, texcoord + haloVec, direction.xy, distortion, bSpace) * WeightForHalo * 1.5; \n' +
+
+ ' result += halo; \n' +
+ ' result += texture2D(u_DirtTexture, v_textureCoordinates); \n' +
+
+
+
+
+ //Rotating starburst texture's coordinate
+ ' vec3 camx = vec3(czm_view[0][0], czm_view[0][1], czm_view[0][2] ); \n' +
+ ' vec3 camz = vec3(czm_view[1][0], czm_view[1][1], czm_view[1][2] ); \n' +
+ ' float camrot = dot(camx, vec3(0, 0, 1)) + dot(camz, vec3(0, 1, 0)); \n' +
+
+ ' mat3 rotation = mat3( \n' +
I don't trust that all GLSL compilers for factor out the common cos and
sin calls. Call each function once above here and assign to local
variables.
------------------------------
In package-lock.json
<#5498 (comment)>
:
> @@ -0,0 +1,5024 @@
+{
Are you sure this file should be included? I think you should remove it
from git.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#5498 (review)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AVPCk8zTiVCPkTAVvmomrJJvbwgbPn0jks5sG6b2gaJpZM4N9y9Y>
.
|
LensFlare codes refine Add WorldCoordinatestoNDC for a built-in Cesium GLSL function including a unit test Delete pakage-lock.json
remove package-lock
|
I just refined my codes and created the new built-in function.
But, when I removed package-lock.json
<#5498 (comment)>,
it made build error.
On Fri, Jun 23, 2017 at 10:36 AM, Byumjin Kim <byumjin@seas.upenn.edu>
wrote:
… Thanks for reviewing!
I'll refine it asap.
Hmm.. I don't remember what is package-lock.json
<#5498 (comment)> and
its updated part.
I'll check it and remove the part if it is useless.
Thank you,
Kim.
On Fri, Jun 23, 2017 at 7:52 AM, Patrick Cozzi ***@***.***>
wrote:
> ***@***.**** requested changes on this pull request.
>
> I did a quick review, mostly style changes. @lilleyse
> <https://github.com/lilleyse> or @bagnell <https://github.com/bagnell>
> should still look at this.
> ------------------------------
>
> In Source/Scene/PostProcessorScene.js
> <#5498 (comment)>
> :
>
> > ];
>
> this._postProcessor = new PostProcessor({
> stages : stages
> });
> }
>
> + function createLensFlareStage() {
> + var url_Dirt = buildModuleUrl('Assets/Textures/LensFlare/DirtMask.jpg');
>
> For local variable naming conventions, we use camelCase.
>
> If you haven't already browse through our Coding Guide
> <https://github.com/AnalyticalGraphicsInc/cesium/tree/master/Documentation/Contributors/CodingGuide>
> .
> ------------------------------
>
> In Source/Scene/PostProcessorScene.js
> <#5498 (comment)>
> :
>
> > ];
>
> this._postProcessor = new PostProcessor({
> stages : stages
> });
> }
>
> + function createLensFlareStage() {
> + var url_Dirt = buildModuleUrl('Assets/Textures/LensFlare/DirtMask.jpg');
> + var url_Star = buildModuleUrl('Assets/Textures/LensFlare/StarBurst.jpg');
> +
> + var uniformValues = {
> +
> + DirtTexture: url_Dirt,
>
> Same camelCase comment here and throughout.
> ------------------------------
>
> In Source/Scene/PostProcessorScene.js
> <#5498 (comment)>
> :
>
> > ];
>
> this._postProcessor = new PostProcessor({
> stages : stages
> });
> }
>
> + function createLensFlareStage() {
> + var url_Dirt = buildModuleUrl('Assets/Textures/LensFlare/DirtMask.jpg');
> + var url_Star = buildModuleUrl('Assets/Textures/LensFlare/StarBurst.jpg');
> +
> + var uniformValues = {
> +
>
> Remove extra whitespace.
> ------------------------------
>
> In Source/Scene/PostProcessorScene.js
> <#5498 (comment)>
> :
>
> > +
> + Distortion: 5.0,
> + NumofGhosts: 4,
> + GhostDispersal: 0.4,
> +
> + HaloWidth: 0.4,
> +
> + EarthRadius: 0.0
> + };
> +
> + var LensFlareShader =
> + 'uniform sampler2D u_colorTexture; \n' +
> + 'uniform sampler2D u_DirtTexture; \n' +
> + 'uniform sampler2D u_StarTexture; \n' +
> +
> + 'uniform float u_Distortion; \n' +
>
> If it is currently possible with the post-processing framework, I would
> combine all these floats into a vec4...and generally pack uniforms
> together as much as possible. We'll still want to expose these in
> JavaScript as separate variables then map to the uniforms x, y, z, w
> components like we do here
> <https://github.com/AnalyticalGraphicsInc/cesium/blob/master/Source/Shaders/BillboardCollectionVS.glsl>
> .
>
> Since this shader isn't called often, this isn't a huge performance
> issue, but this is the common practice throughout Cesium.
>
> If the post-process framework doesn't support this, @lilleyse
> <https://github.com/lilleyse> can add it and we can wait on this change
> or @byumjin <https://github.com/byumjin> you are welcome to go for it.
> ------------------------------
>
> In Source/Scene/PostProcessorScene.js
> <#5498 (comment)>
> :
>
> > + var LensFlareShader =
> + 'uniform sampler2D u_colorTexture; \n' +
> + 'uniform sampler2D u_DirtTexture; \n' +
> + 'uniform sampler2D u_StarTexture; \n' +
> +
> + 'uniform float u_Distortion; \n' +
> + 'uniform float u_GhostDispersal; \n' +
> + 'uniform float u_HaloWidth; \n' +
> + 'uniform float u_EarthRadius; \n' +
> +
> + 'uniform float u_Intensity; \n' +
> +
> + 'varying vec2 v_textureCoordinates; \n' +
> +
> + //return ndc from world coordinate biased EarthRadius
> + 'vec4 GetNDCFromWC(in vec3 WC, in float EarthRadius) \n' +
>
> Could you make this a built-in Cesium GLSL function including a unit test?
>
> For example, see
>
> https://github.com/AnalyticalGraphicsInc/cesium/tree/master/
> Source/Shaders/Builtin/Functions
>
> and
>
> https://github.com/AnalyticalGraphicsInc/cesium/blob/master/
> Specs/Renderer/BuiltinFunctionsSpec.js
>
> To learn more about unit tests in general, see our Testing Guide
> <https://github.com/AnalyticalGraphicsInc/cesium/tree/master/Documentation/Contributors/TestingGuide#testing-guide>,
> especially the GLSL section
> <https://github.com/AnalyticalGraphicsInc/cesium/tree/master/Documentation/Contributors/TestingGuide#glsl>
> .
>
> Also rename this to czm_WorldCoordinatestoNDC like our color transform
> functions.
> ------------------------------
>
> In Source/Scene/PostProcessorScene.js
> <#5498 (comment)>
> :
>
> > +
> + 'varying vec2 v_textureCoordinates; \n' +
> +
> + //return ndc from world coordinate biased EarthRadius
> + 'vec4 GetNDCFromWC(in vec3 WC, in float EarthRadius) \n' +
> + '{ \n' +
> + ' vec4 position = vec4(WC, 1.0); \n' +
> + ' vec4 positionEC = czm_view * position; \n' +
> + ' positionEC = vec4(positionEC.x + EarthRadius , positionEC.y, positionEC.z, 1.0); \n' +
> + ' vec4 positionWC = czm_eyeToWindowCoordinates(positionEC); \n' +
> + ' return czm_viewportOrthographic * vec4(positionWC.xy, -positionWC.z, 1.0); \n' +
> + '} \n' +
> +
> + //Check if current pixel is included Earth
> + //if then mask it gradually
> + 'float IsinEarth(in vec2 texcoord, in vec2 SceneSize) \n' +
>
> Throughout GLSL, we don't use the in, etc. parameter prefixes.
> ------------------------------
>
> In Source/Scene/PostProcessorScene.js
> <#5498 (comment)>
> :
>
> > + ' positionEC = vec4(positionEC.x + EarthRadius , positionEC.y, positionEC.z, 1.0); \n' +
> + ' vec4 positionWC = czm_eyeToWindowCoordinates(positionEC); \n' +
> + ' return czm_viewportOrthographic * vec4(positionWC.xy, -positionWC.z, 1.0); \n' +
> + '} \n' +
> +
> + //Check if current pixel is included Earth
> + //if then mask it gradually
> + 'float IsinEarth(in vec2 texcoord, in vec2 SceneSize) \n' +
> + '{ \n' +
> +
> + 'vec2 NDC = texcoord * 2.0 - 1.0; \n' +
> + 'vec4 EarthPosSC = GetNDCFromWC(vec3(0.0,0.0,0.0), 0.0); \n' +
> + 'vec4 EarthPosSCEdge = GetNDCFromWC(vec3( 0.0,0.0,0.0), u_EarthRadius *1.5); \n' +
> +
> + 'NDC.xy -= EarthPosSC.xy; \n' +
> + 'float X = abs(NDC.x)*SceneSize.x; \n' +
>
> Fix whitespace. Just
>
> float x = abs(ndc.x) * sceneSize.x;
>
> (and also fixed naming convention).
> ------------------------------
>
> In Source/Scene/PostProcessorScene.js
> <#5498 (comment)>
> :
>
> > + ' positionEC = vec4(positionEC.x + EarthRadius , positionEC.y, positionEC.z, 1.0); \n' +
> + ' vec4 positionWC = czm_eyeToWindowCoordinates(positionEC); \n' +
> + ' return czm_viewportOrthographic * vec4(positionWC.xy, -positionWC.z, 1.0); \n' +
> + '} \n' +
> +
> + //Check if current pixel is included Earth
> + //if then mask it gradually
> + 'float IsinEarth(in vec2 texcoord, in vec2 SceneSize) \n' +
> + '{ \n' +
> +
> + 'vec2 NDC = texcoord * 2.0 - 1.0; \n' +
> + 'vec4 EarthPosSC = GetNDCFromWC(vec3(0.0,0.0,0.0), 0.0); \n' +
> + 'vec4 EarthPosSCEdge = GetNDCFromWC(vec3( 0.0,0.0,0.0), u_EarthRadius *1.5); \n' +
> +
> + 'NDC.xy -= EarthPosSC.xy; \n' +
> + 'float X = abs(NDC.x)*SceneSize.x; \n' +
>
> The GLSL could use a lot of tweaks like this to look like our other GLSL
> code. Feel free to make them now or wait until this is in a separate .glsl
> file where it would be easier and the WebStorm auto formatter might work
> (we use it for JavaScript).
> ------------------------------
>
> In Source/Scene/PostProcessorScene.js
> <#5498 (comment)>
> :
>
> > + 'float Y = abs(NDC.y)*SceneSize.y; \n' +
> + 'Y *= Y; \n' +
> +
> + 'return clamp(0.0, 1.0, max( sqrt(X + Y) / max(abs(EarthPosSCEdge.x*SceneSize.x), 1.0) - 0.8 , 0.0)); \n' +
> +
> + '} \n' +
> +
> +
> + //For Chromatic effect
> + 'vec4 textureDistorted( \n' +
> + ' in sampler2D tex, \n' +
> + ' in vec2 texcoord, \n' +
> + ' in vec2 direction, \n' +
> + ' in vec3 distortion, \n' +
> + ' in bool bSpace \n' +
> + ' ) { \n' +
>
> In GLSL, we put { on a new line.
> ------------------------------
>
> In Source/Scene/PostProcessorScene.js
> <#5498 (comment)>
> :
>
> > + 'X *= X; \n' +
> + 'float Y = abs(NDC.y)*SceneSize.y; \n' +
> + 'Y *= Y; \n' +
> +
> + 'return clamp(0.0, 1.0, max( sqrt(X + Y) / max(abs(EarthPosSCEdge.x*SceneSize.x), 1.0) - 0.8 , 0.0)); \n' +
> +
> + '} \n' +
> +
> +
> + //For Chromatic effect
> + 'vec4 textureDistorted( \n' +
> + ' in sampler2D tex, \n' +
> + ' in vec2 texcoord, \n' +
> + ' in vec2 direction, \n' +
> + ' in vec3 distortion, \n' +
> + ' in bool bSpace \n' +
>
> Do not use hungarian notation.
> ------------------------------
>
> In Source/Scene/PostProcessorScene.js
> <#5498 (comment)>
> :
>
> > + 'float red = 0.0; \n' +
> + 'float green = 0.0; \n' +
> + 'float blue = 0.0; \n' +
> + 'if(bSpace) \n' +
> + '{ \n' +
> + 'red = IsinEarth(texcoord + direction * distortion.r, SceneSize) * texture2D(tex, texcoord + direction * distortion.r).r; \n' +
> + 'green = IsinEarth(texcoord + direction * distortion.g, SceneSize) * texture2D(tex, texcoord + direction * distortion.g).g; \n' +
> + 'blue = IsinEarth(texcoord + direction * distortion.b, SceneSize) * texture2D(tex, texcoord + direction * distortion.b).b; \n' +
> + '} \n' +
> + 'else \n' +
> + '{ \n' +
> + 'red = texture2D(tex, texcoord + direction * distortion.r).r; \n' +
> + 'green = texture2D(tex, texcoord + direction * distortion.g).g; \n' +
> + 'blue = texture2D(tex, texcoord + direction * distortion.b).b; \n' +
> + '} \n' +
> +
>
> Here are throughout, no need to have all this extra whitespace. We never
> use more than one blank line.
> ------------------------------
>
> In Source/Scene/PostProcessorScene.js
> <#5498 (comment)>
> :
>
> > +
> + 'return vec4(clamp(red, 0.0, 1.0), clamp(green, 0.0, 1.0), clamp(blue, 0.0, 1.0), 0.0); \n' +
> +
> + '} \n' +
> +
> +
> +
> + 'void main(void) \n' +
> + '{ \n' +
> + ' vec3 rgb = texture2D(u_colorTexture, v_textureCoordinates).rgb; \n' +
> +
> +
> + 'bool bSpace = true; \n' +
> +
> + //whether it is in space or not
> + 'if(length(czm_viewerPositionWC.xyz) < 6500000.0) \n' +
>
> How did you determine 6500000.0?
> ------------------------------
>
> In Source/Scene/PostProcessorScene.js
> <#5498 (comment)>
> :
>
> > +
> + 'vec2 texelSize = 1.0 / czm_viewport.zw; \n' +
> + 'vec3 distortion = vec3(-texelSize.x * u_Distortion, 0.0, texelSize.x * u_Distortion); \n' +
> +
> + // ghost vector to image centre:
> + 'vec2 ghostVec = (vec2(0.5) - texcoord) * u_GhostDispersal; \n' +
> + 'vec3 direction = normalize(vec3(ghostVec, 0.0)); \n' +
> +
> + // sample ghosts:
> + ' vec4 result = vec4(0.0); \n' +
> + ' vec4 ghost = vec4(0.0); \n' +
> + ' for (int i = 0; i < 4; ++i) { \n' +
> + ' vec2 offset = fract(texcoord + ghostVec * float(i)); \n' +
> +
> + //Only bright spots from the centre of the source image
> + //' float weight = length(vec2(0.5) - offset) / length(vec2(0.5)); \n' +
>
> Delete commented out code.
>
> If you really need it, it could stay for now, but once this is merged
> into master, we do not leave in commented out code.
> ------------------------------
>
> In Source/Scene/PostProcessorScene.js
> <#5498 (comment)>
> :
>
> > + 'vec4 halo; \n' +
> + ' vec2 haloVec = normalize(ghostVec) * u_HaloWidth; \n' +
> + ' float WeightForHalo = length(vec2(0.5) - fract(texcoord + haloVec)) / length(vec2(0.5)); \n' +
> + ' WeightForHalo = pow(1.0 - WeightForHalo, 5.0); \n' +
> + ' halo = textureDistorted(u_colorTexture, texcoord + haloVec, direction.xy, distortion, bSpace) * WeightForHalo * 1.5; \n' +
> +
> + ' result += halo; \n' +
> + ' result += texture2D(u_DirtTexture, v_textureCoordinates); \n' +
> +
> +
> +
> +
> + //Rotating starburst texture's coordinate
> + ' vec3 camx = vec3(czm_view[0][0], czm_view[0][1], czm_view[0][2] ); \n' +
> + ' vec3 camz = vec3(czm_view[1][0], czm_view[1][1], czm_view[1][2] ); \n' +
> + ' float camrot = dot(camx, vec3(0, 0, 1)) + dot(camz, vec3(0, 1, 0)); \n' +
>
> Throughout GLSL, when your intention is floating point, always include .0,
> e.g., 0.0, etc.
>
> On some implementations, not doing so at least use to result in a
> compiler error.
> ------------------------------
>
> In Source/Scene/PostProcessorScene.js
> <#5498 (comment)>
> :
>
> > + ' float WeightForHalo = length(vec2(0.5) - fract(texcoord + haloVec)) / length(vec2(0.5)); \n' +
> + ' WeightForHalo = pow(1.0 - WeightForHalo, 5.0); \n' +
> + ' halo = textureDistorted(u_colorTexture, texcoord + haloVec, direction.xy, distortion, bSpace) * WeightForHalo * 1.5; \n' +
> +
> + ' result += halo; \n' +
> + ' result += texture2D(u_DirtTexture, v_textureCoordinates); \n' +
> +
> +
> +
> +
> + //Rotating starburst texture's coordinate
> + ' vec3 camx = vec3(czm_view[0][0], czm_view[0][1], czm_view[0][2] ); \n' +
> + ' vec3 camz = vec3(czm_view[1][0], czm_view[1][1], czm_view[1][2] ); \n' +
> + ' float camrot = dot(camx, vec3(0, 0, 1)) + dot(camz, vec3(0, 1, 0)); \n' +
> +
> + ' mat3 rotation = mat3( \n' +
>
> I don't trust that all GLSL compilers for factor out the common cos and
> sin calls. Call each function once above here and assign to local
> variables.
> ------------------------------
>
> In package-lock.json
> <#5498 (comment)>
> :
>
> > @@ -0,0 +1,5024 @@
> +{
>
> Are you sure this file should be included? I think you should remove it
> from git.
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <#5498 (review)>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/AVPCk8zTiVCPkTAVvmomrJJvbwgbPn0jks5sG6b2gaJpZM4N9y9Y>
> .
>
|
|
Could someone give advice to solve this issue?
…On Fri, Jun 23, 2017 at 1:44 PM, Byumjin Kim ***@***.***> wrote:
I just refined my codes and created the new built-in function.
But, when I removed package-lock.json
<#5498 (comment)>,
it made build error.
On Fri, Jun 23, 2017 at 10:36 AM, Byumjin Kim ***@***.***>
wrote:
> Thanks for reviewing!
>
> I'll refine it asap.
>
> Hmm.. I don't remember what is package-lock.json
> <#5498 (comment)> and
> its updated part.
> I'll check it and remove the part if it is useless.
>
> Thank you,
>
> Kim.
>
> On Fri, Jun 23, 2017 at 7:52 AM, Patrick Cozzi ***@***.***>
> wrote:
>
>> ***@***.**** requested changes on this pull request.
>>
>> I did a quick review, mostly style changes. @lilleyse
>> <https://github.com/lilleyse> or @bagnell <https://github.com/bagnell>
>> should still look at this.
>> ------------------------------
>>
>> In Source/Scene/PostProcessorScene.js
>> <#5498 (comment)>
>> :
>>
>> > ];
>>
>> this._postProcessor = new PostProcessor({
>> stages : stages
>> });
>> }
>>
>> + function createLensFlareStage() {
>> + var url_Dirt = buildModuleUrl('Assets/Textures/LensFlare/DirtMask.jpg');
>>
>> For local variable naming conventions, we use camelCase.
>>
>> If you haven't already browse through our Coding Guide
>> <https://github.com/AnalyticalGraphicsInc/cesium/tree/master/Documentation/Contributors/CodingGuide>
>> .
>> ------------------------------
>>
>> In Source/Scene/PostProcessorScene.js
>> <#5498 (comment)>
>> :
>>
>> > ];
>>
>> this._postProcessor = new PostProcessor({
>> stages : stages
>> });
>> }
>>
>> + function createLensFlareStage() {
>> + var url_Dirt = buildModuleUrl('Assets/Textures/LensFlare/DirtMask.jpg');
>> + var url_Star = buildModuleUrl('Assets/Textures/LensFlare/StarBurst.jpg');
>> +
>> + var uniformValues = {
>> +
>> + DirtTexture: url_Dirt,
>>
>> Same camelCase comment here and throughout.
>> ------------------------------
>>
>> In Source/Scene/PostProcessorScene.js
>> <#5498 (comment)>
>> :
>>
>> > ];
>>
>> this._postProcessor = new PostProcessor({
>> stages : stages
>> });
>> }
>>
>> + function createLensFlareStage() {
>> + var url_Dirt = buildModuleUrl('Assets/Textures/LensFlare/DirtMask.jpg');
>> + var url_Star = buildModuleUrl('Assets/Textures/LensFlare/StarBurst.jpg');
>> +
>> + var uniformValues = {
>> +
>>
>> Remove extra whitespace.
>> ------------------------------
>>
>> In Source/Scene/PostProcessorScene.js
>> <#5498 (comment)>
>> :
>>
>> > +
>> + Distortion: 5.0,
>> + NumofGhosts: 4,
>> + GhostDispersal: 0.4,
>> +
>> + HaloWidth: 0.4,
>> +
>> + EarthRadius: 0.0
>> + };
>> +
>> + var LensFlareShader =
>> + 'uniform sampler2D u_colorTexture; \n' +
>> + 'uniform sampler2D u_DirtTexture; \n' +
>> + 'uniform sampler2D u_StarTexture; \n' +
>> +
>> + 'uniform float u_Distortion; \n' +
>>
>> If it is currently possible with the post-processing framework, I would
>> combine all these floats into a vec4...and generally pack uniforms
>> together as much as possible. We'll still want to expose these in
>> JavaScript as separate variables then map to the uniforms x, y, z, w
>> components like we do here
>> <https://github.com/AnalyticalGraphicsInc/cesium/blob/master/Source/Shaders/BillboardCollectionVS.glsl>
>> .
>>
>> Since this shader isn't called often, this isn't a huge performance
>> issue, but this is the common practice throughout Cesium.
>>
>> If the post-process framework doesn't support this, @lilleyse
>> <https://github.com/lilleyse> can add it and we can wait on this change
>> or @byumjin <https://github.com/byumjin> you are welcome to go for it.
>> ------------------------------
>>
>> In Source/Scene/PostProcessorScene.js
>> <#5498 (comment)>
>> :
>>
>> > + var LensFlareShader =
>> + 'uniform sampler2D u_colorTexture; \n' +
>> + 'uniform sampler2D u_DirtTexture; \n' +
>> + 'uniform sampler2D u_StarTexture; \n' +
>> +
>> + 'uniform float u_Distortion; \n' +
>> + 'uniform float u_GhostDispersal; \n' +
>> + 'uniform float u_HaloWidth; \n' +
>> + 'uniform float u_EarthRadius; \n' +
>> +
>> + 'uniform float u_Intensity; \n' +
>> +
>> + 'varying vec2 v_textureCoordinates; \n' +
>> +
>> + //return ndc from world coordinate biased EarthRadius
>> + 'vec4 GetNDCFromWC(in vec3 WC, in float EarthRadius) \n' +
>>
>> Could you make this a built-in Cesium GLSL function including a unit
>> test?
>>
>> For example, see
>>
>> https://github.com/AnalyticalGraphicsInc/cesium/tree/master/
>> Source/Shaders/Builtin/Functions
>>
>> and
>>
>> https://github.com/AnalyticalGraphicsInc/cesium/blob/master/
>> Specs/Renderer/BuiltinFunctionsSpec.js
>>
>> To learn more about unit tests in general, see our Testing Guide
>> <https://github.com/AnalyticalGraphicsInc/cesium/tree/master/Documentation/Contributors/TestingGuide#testing-guide>,
>> especially the GLSL section
>> <https://github.com/AnalyticalGraphicsInc/cesium/tree/master/Documentation/Contributors/TestingGuide#glsl>
>> .
>>
>> Also rename this to czm_WorldCoordinatestoNDC like our color transform
>> functions.
>> ------------------------------
>>
>> In Source/Scene/PostProcessorScene.js
>> <#5498 (comment)>
>> :
>>
>> > +
>> + 'varying vec2 v_textureCoordinates; \n' +
>> +
>> + //return ndc from world coordinate biased EarthRadius
>> + 'vec4 GetNDCFromWC(in vec3 WC, in float EarthRadius) \n' +
>> + '{ \n' +
>> + ' vec4 position = vec4(WC, 1.0); \n' +
>> + ' vec4 positionEC = czm_view * position; \n' +
>> + ' positionEC = vec4(positionEC.x + EarthRadius , positionEC.y, positionEC.z, 1.0); \n' +
>> + ' vec4 positionWC = czm_eyeToWindowCoordinates(positionEC); \n' +
>> + ' return czm_viewportOrthographic * vec4(positionWC.xy, -positionWC.z, 1.0); \n' +
>> + '} \n' +
>> +
>> + //Check if current pixel is included Earth
>> + //if then mask it gradually
>> + 'float IsinEarth(in vec2 texcoord, in vec2 SceneSize) \n' +
>>
>> Throughout GLSL, we don't use the in, etc. parameter prefixes.
>> ------------------------------
>>
>> In Source/Scene/PostProcessorScene.js
>> <#5498 (comment)>
>> :
>>
>> > + ' positionEC = vec4(positionEC.x + EarthRadius , positionEC.y, positionEC.z, 1.0); \n' +
>> + ' vec4 positionWC = czm_eyeToWindowCoordinates(positionEC); \n' +
>> + ' return czm_viewportOrthographic * vec4(positionWC.xy, -positionWC.z, 1.0); \n' +
>> + '} \n' +
>> +
>> + //Check if current pixel is included Earth
>> + //if then mask it gradually
>> + 'float IsinEarth(in vec2 texcoord, in vec2 SceneSize) \n' +
>> + '{ \n' +
>> +
>> + 'vec2 NDC = texcoord * 2.0 - 1.0; \n' +
>> + 'vec4 EarthPosSC = GetNDCFromWC(vec3(0.0,0.0,0.0), 0.0); \n' +
>> + 'vec4 EarthPosSCEdge = GetNDCFromWC(vec3( 0.0,0.0,0.0), u_EarthRadius *1.5); \n' +
>> +
>> + 'NDC.xy -= EarthPosSC.xy; \n' +
>> + 'float X = abs(NDC.x)*SceneSize.x; \n' +
>>
>> Fix whitespace. Just
>>
>> float x = abs(ndc.x) * sceneSize.x;
>>
>> (and also fixed naming convention).
>> ------------------------------
>>
>> In Source/Scene/PostProcessorScene.js
>> <#5498 (comment)>
>> :
>>
>> > + ' positionEC = vec4(positionEC.x + EarthRadius , positionEC.y, positionEC.z, 1.0); \n' +
>> + ' vec4 positionWC = czm_eyeToWindowCoordinates(positionEC); \n' +
>> + ' return czm_viewportOrthographic * vec4(positionWC.xy, -positionWC.z, 1.0); \n' +
>> + '} \n' +
>> +
>> + //Check if current pixel is included Earth
>> + //if then mask it gradually
>> + 'float IsinEarth(in vec2 texcoord, in vec2 SceneSize) \n' +
>> + '{ \n' +
>> +
>> + 'vec2 NDC = texcoord * 2.0 - 1.0; \n' +
>> + 'vec4 EarthPosSC = GetNDCFromWC(vec3(0.0,0.0,0.0), 0.0); \n' +
>> + 'vec4 EarthPosSCEdge = GetNDCFromWC(vec3( 0.0,0.0,0.0), u_EarthRadius *1.5); \n' +
>> +
>> + 'NDC.xy -= EarthPosSC.xy; \n' +
>> + 'float X = abs(NDC.x)*SceneSize.x; \n' +
>>
>> The GLSL could use a lot of tweaks like this to look like our other GLSL
>> code. Feel free to make them now or wait until this is in a separate .glsl
>> file where it would be easier and the WebStorm auto formatter might work
>> (we use it for JavaScript).
>> ------------------------------
>>
>> In Source/Scene/PostProcessorScene.js
>> <#5498 (comment)>
>> :
>>
>> > + 'float Y = abs(NDC.y)*SceneSize.y; \n' +
>> + 'Y *= Y; \n' +
>> +
>> + 'return clamp(0.0, 1.0, max( sqrt(X + Y) / max(abs(EarthPosSCEdge.x*SceneSize.x), 1.0) - 0.8 , 0.0)); \n' +
>> +
>> + '} \n' +
>> +
>> +
>> + //For Chromatic effect
>> + 'vec4 textureDistorted( \n' +
>> + ' in sampler2D tex, \n' +
>> + ' in vec2 texcoord, \n' +
>> + ' in vec2 direction, \n' +
>> + ' in vec3 distortion, \n' +
>> + ' in bool bSpace \n' +
>> + ' ) { \n' +
>>
>> In GLSL, we put { on a new line.
>> ------------------------------
>>
>> In Source/Scene/PostProcessorScene.js
>> <#5498 (comment)>
>> :
>>
>> > + 'X *= X; \n' +
>> + 'float Y = abs(NDC.y)*SceneSize.y; \n' +
>> + 'Y *= Y; \n' +
>> +
>> + 'return clamp(0.0, 1.0, max( sqrt(X + Y) / max(abs(EarthPosSCEdge.x*SceneSize.x), 1.0) - 0.8 , 0.0)); \n' +
>> +
>> + '} \n' +
>> +
>> +
>> + //For Chromatic effect
>> + 'vec4 textureDistorted( \n' +
>> + ' in sampler2D tex, \n' +
>> + ' in vec2 texcoord, \n' +
>> + ' in vec2 direction, \n' +
>> + ' in vec3 distortion, \n' +
>> + ' in bool bSpace \n' +
>>
>> Do not use hungarian notation.
>> ------------------------------
>>
>> In Source/Scene/PostProcessorScene.js
>> <#5498 (comment)>
>> :
>>
>> > + 'float red = 0.0; \n' +
>> + 'float green = 0.0; \n' +
>> + 'float blue = 0.0; \n' +
>> + 'if(bSpace) \n' +
>> + '{ \n' +
>> + 'red = IsinEarth(texcoord + direction * distortion.r, SceneSize) * texture2D(tex, texcoord + direction * distortion.r).r; \n' +
>> + 'green = IsinEarth(texcoord + direction * distortion.g, SceneSize) * texture2D(tex, texcoord + direction * distortion.g).g; \n' +
>> + 'blue = IsinEarth(texcoord + direction * distortion.b, SceneSize) * texture2D(tex, texcoord + direction * distortion.b).b; \n' +
>> + '} \n' +
>> + 'else \n' +
>> + '{ \n' +
>> + 'red = texture2D(tex, texcoord + direction * distortion.r).r; \n' +
>> + 'green = texture2D(tex, texcoord + direction * distortion.g).g; \n' +
>> + 'blue = texture2D(tex, texcoord + direction * distortion.b).b; \n' +
>> + '} \n' +
>> +
>>
>> Here are throughout, no need to have all this extra whitespace. We never
>> use more than one blank line.
>> ------------------------------
>>
>> In Source/Scene/PostProcessorScene.js
>> <#5498 (comment)>
>> :
>>
>> > +
>> + 'return vec4(clamp(red, 0.0, 1.0), clamp(green, 0.0, 1.0), clamp(blue, 0.0, 1.0), 0.0); \n' +
>> +
>> + '} \n' +
>> +
>> +
>> +
>> + 'void main(void) \n' +
>> + '{ \n' +
>> + ' vec3 rgb = texture2D(u_colorTexture, v_textureCoordinates).rgb; \n' +
>> +
>> +
>> + 'bool bSpace = true; \n' +
>> +
>> + //whether it is in space or not
>> + 'if(length(czm_viewerPositionWC.xyz) < 6500000.0) \n' +
>>
>> How did you determine 6500000.0?
>> ------------------------------
>>
>> In Source/Scene/PostProcessorScene.js
>> <#5498 (comment)>
>> :
>>
>> > +
>> + 'vec2 texelSize = 1.0 / czm_viewport.zw; \n' +
>> + 'vec3 distortion = vec3(-texelSize.x * u_Distortion, 0.0, texelSize.x * u_Distortion); \n' +
>> +
>> + // ghost vector to image centre:
>> + 'vec2 ghostVec = (vec2(0.5) - texcoord) * u_GhostDispersal; \n' +
>> + 'vec3 direction = normalize(vec3(ghostVec, 0.0)); \n' +
>> +
>> + // sample ghosts:
>> + ' vec4 result = vec4(0.0); \n' +
>> + ' vec4 ghost = vec4(0.0); \n' +
>> + ' for (int i = 0; i < 4; ++i) { \n' +
>> + ' vec2 offset = fract(texcoord + ghostVec * float(i)); \n' +
>> +
>> + //Only bright spots from the centre of the source image
>> + //' float weight = length(vec2(0.5) - offset) / length(vec2(0.5)); \n' +
>>
>> Delete commented out code.
>>
>> If you really need it, it could stay for now, but once this is merged
>> into master, we do not leave in commented out code.
>> ------------------------------
>>
>> In Source/Scene/PostProcessorScene.js
>> <#5498 (comment)>
>> :
>>
>> > + 'vec4 halo; \n' +
>> + ' vec2 haloVec = normalize(ghostVec) * u_HaloWidth; \n' +
>> + ' float WeightForHalo = length(vec2(0.5) - fract(texcoord + haloVec)) / length(vec2(0.5)); \n' +
>> + ' WeightForHalo = pow(1.0 - WeightForHalo, 5.0); \n' +
>> + ' halo = textureDistorted(u_colorTexture, texcoord + haloVec, direction.xy, distortion, bSpace) * WeightForHalo * 1.5; \n' +
>> +
>> + ' result += halo; \n' +
>> + ' result += texture2D(u_DirtTexture, v_textureCoordinates); \n' +
>> +
>> +
>> +
>> +
>> + //Rotating starburst texture's coordinate
>> + ' vec3 camx = vec3(czm_view[0][0], czm_view[0][1], czm_view[0][2] ); \n' +
>> + ' vec3 camz = vec3(czm_view[1][0], czm_view[1][1], czm_view[1][2] ); \n' +
>> + ' float camrot = dot(camx, vec3(0, 0, 1)) + dot(camz, vec3(0, 1, 0)); \n' +
>>
>> Throughout GLSL, when your intention is floating point, always include
>> .0, e.g., 0.0, etc.
>>
>> On some implementations, not doing so at least use to result in a
>> compiler error.
>> ------------------------------
>>
>> In Source/Scene/PostProcessorScene.js
>> <#5498 (comment)>
>> :
>>
>> > + ' float WeightForHalo = length(vec2(0.5) - fract(texcoord + haloVec)) / length(vec2(0.5)); \n' +
>> + ' WeightForHalo = pow(1.0 - WeightForHalo, 5.0); \n' +
>> + ' halo = textureDistorted(u_colorTexture, texcoord + haloVec, direction.xy, distortion, bSpace) * WeightForHalo * 1.5; \n' +
>> +
>> + ' result += halo; \n' +
>> + ' result += texture2D(u_DirtTexture, v_textureCoordinates); \n' +
>> +
>> +
>> +
>> +
>> + //Rotating starburst texture's coordinate
>> + ' vec3 camx = vec3(czm_view[0][0], czm_view[0][1], czm_view[0][2] ); \n' +
>> + ' vec3 camz = vec3(czm_view[1][0], czm_view[1][1], czm_view[1][2] ); \n' +
>> + ' float camrot = dot(camx, vec3(0, 0, 1)) + dot(camz, vec3(0, 1, 0)); \n' +
>> +
>> + ' mat3 rotation = mat3( \n' +
>>
>> I don't trust that all GLSL compilers for factor out the common cos and
>> sin calls. Call each function once above here and assign to local
>> variables.
>> ------------------------------
>>
>> In package-lock.json
>> <#5498 (comment)>
>> :
>>
>> > @@ -0,0 +1,5024 @@
>> +{
>>
>> Are you sure this file should be included? I think you should remove it
>> from git.
>>
>> —
>> You are receiving this because you were mentioned.
>> Reply to this email directly, view it on GitHub
>> <#5498 (review)>,
>> or mute the thread
>> <https://github.com/notifications/unsubscribe-auth/AVPCk8zTiVCPkTAVvmomrJJvbwgbPn0jks5sG6b2gaJpZM4N9y9Y>
>> .
>>
>
>
|
|
[17:33:11] 'combine' errored after 3.02 s
npm ERR! Linux 4.8.12-040812-generic
npm ERR! argv "/home/travis/.nvm/versions/node/v4.3.2/bin/node"
"/home/travis/.nvm/versions/node/v4.3.2/bin/npm" "run" "makeZipFile"
npm ERR! node v4.3.2
npm ERR! npm v2.14.12
npm ERR! code ELIFECYCLE
npm ERR! cesium@1.34.0-post-processing22348 makeZipFile: `gulp makeZipFile`
npm ERR! Exit status 1
npm ERR!
npm ERR! Failed at the cesium@1.34.0-post-processing22348 makeZipFile
script 'gulp makeZipFile'.
npm ERR! This is most likely a problem with the cesium package,
npm ERR! not with npm itself.
npm ERR! Tell the author that this fails on your system:
npm ERR! gulp makeZipFile
npm ERR! You can get their info via:
npm ERR! npm owner ls cesium
npm ERR! There is likely additional logging output above.
npm ERR! Please include the following file with any support request:
npm ERR! /home/travis/build/AnalyticalGraphicsInc/cesium/npm-debug.log
…On Fri, Jun 23, 2017 at 1:49 PM, Byumjin Kim ***@***.***> wrote:
Could someone give advice to solve this issue?
On Fri, Jun 23, 2017 at 1:44 PM, Byumjin Kim ***@***.***>
wrote:
> I just refined my codes and created the new built-in function.
>
> But, when I removed package-lock.json
> <#5498 (comment)>,
> it made build error.
>
> On Fri, Jun 23, 2017 at 10:36 AM, Byumjin Kim ***@***.***>
> wrote:
>
>> Thanks for reviewing!
>>
>> I'll refine it asap.
>>
>> Hmm.. I don't remember what is package-lock.json
>> <#5498 (comment)> and
>> its updated part.
>> I'll check it and remove the part if it is useless.
>>
>> Thank you,
>>
>> Kim.
>>
>> On Fri, Jun 23, 2017 at 7:52 AM, Patrick Cozzi ***@***.***
>> > wrote:
>>
>>> ***@***.**** requested changes on this pull request.
>>>
>>> I did a quick review, mostly style changes. @lilleyse
>>> <https://github.com/lilleyse> or @bagnell <https://github.com/bagnell>
>>> should still look at this.
>>> ------------------------------
>>>
>>> In Source/Scene/PostProcessorScene.js
>>> <#5498 (comment)>
>>> :
>>>
>>> > ];
>>>
>>> this._postProcessor = new PostProcessor({
>>> stages : stages
>>> });
>>> }
>>>
>>> + function createLensFlareStage() {
>>> + var url_Dirt = buildModuleUrl('Assets/Textures/LensFlare/DirtMask.jpg');
>>>
>>> For local variable naming conventions, we use camelCase.
>>>
>>> If you haven't already browse through our Coding Guide
>>> <https://github.com/AnalyticalGraphicsInc/cesium/tree/master/Documentation/Contributors/CodingGuide>
>>> .
>>> ------------------------------
>>>
>>> In Source/Scene/PostProcessorScene.js
>>> <#5498 (comment)>
>>> :
>>>
>>> > ];
>>>
>>> this._postProcessor = new PostProcessor({
>>> stages : stages
>>> });
>>> }
>>>
>>> + function createLensFlareStage() {
>>> + var url_Dirt = buildModuleUrl('Assets/Textures/LensFlare/DirtMask.jpg');
>>> + var url_Star = buildModuleUrl('Assets/Textures/LensFlare/StarBurst.jpg');
>>> +
>>> + var uniformValues = {
>>> +
>>> + DirtTexture: url_Dirt,
>>>
>>> Same camelCase comment here and throughout.
>>> ------------------------------
>>>
>>> In Source/Scene/PostProcessorScene.js
>>> <#5498 (comment)>
>>> :
>>>
>>> > ];
>>>
>>> this._postProcessor = new PostProcessor({
>>> stages : stages
>>> });
>>> }
>>>
>>> + function createLensFlareStage() {
>>> + var url_Dirt = buildModuleUrl('Assets/Textures/LensFlare/DirtMask.jpg');
>>> + var url_Star = buildModuleUrl('Assets/Textures/LensFlare/StarBurst.jpg');
>>> +
>>> + var uniformValues = {
>>> +
>>>
>>> Remove extra whitespace.
>>> ------------------------------
>>>
>>> In Source/Scene/PostProcessorScene.js
>>> <#5498 (comment)>
>>> :
>>>
>>> > +
>>> + Distortion: 5.0,
>>> + NumofGhosts: 4,
>>> + GhostDispersal: 0.4,
>>> +
>>> + HaloWidth: 0.4,
>>> +
>>> + EarthRadius: 0.0
>>> + };
>>> +
>>> + var LensFlareShader =
>>> + 'uniform sampler2D u_colorTexture; \n' +
>>> + 'uniform sampler2D u_DirtTexture; \n' +
>>> + 'uniform sampler2D u_StarTexture; \n' +
>>> +
>>> + 'uniform float u_Distortion; \n' +
>>>
>>> If it is currently possible with the post-processing framework, I would
>>> combine all these floats into a vec4...and generally pack uniforms
>>> together as much as possible. We'll still want to expose these in
>>> JavaScript as separate variables then map to the uniforms x, y, z, w
>>> components like we do here
>>> <https://github.com/AnalyticalGraphicsInc/cesium/blob/master/Source/Shaders/BillboardCollectionVS.glsl>
>>> .
>>>
>>> Since this shader isn't called often, this isn't a huge performance
>>> issue, but this is the common practice throughout Cesium.
>>>
>>> If the post-process framework doesn't support this, @lilleyse
>>> <https://github.com/lilleyse> can add it and we can wait on this
>>> change or @byumjin <https://github.com/byumjin> you are welcome to go
>>> for it.
>>> ------------------------------
>>>
>>> In Source/Scene/PostProcessorScene.js
>>> <#5498 (comment)>
>>> :
>>>
>>> > + var LensFlareShader =
>>> + 'uniform sampler2D u_colorTexture; \n' +
>>> + 'uniform sampler2D u_DirtTexture; \n' +
>>> + 'uniform sampler2D u_StarTexture; \n' +
>>> +
>>> + 'uniform float u_Distortion; \n' +
>>> + 'uniform float u_GhostDispersal; \n' +
>>> + 'uniform float u_HaloWidth; \n' +
>>> + 'uniform float u_EarthRadius; \n' +
>>> +
>>> + 'uniform float u_Intensity; \n' +
>>> +
>>> + 'varying vec2 v_textureCoordinates; \n' +
>>> +
>>> + //return ndc from world coordinate biased EarthRadius
>>> + 'vec4 GetNDCFromWC(in vec3 WC, in float EarthRadius) \n' +
>>>
>>> Could you make this a built-in Cesium GLSL function including a unit
>>> test?
>>>
>>> For example, see
>>>
>>> https://github.com/AnalyticalGraphicsInc/cesium/tree/master/
>>> Source/Shaders/Builtin/Functions
>>>
>>> and
>>>
>>> https://github.com/AnalyticalGraphicsInc/cesium/blob/master/
>>> Specs/Renderer/BuiltinFunctionsSpec.js
>>>
>>> To learn more about unit tests in general, see our Testing Guide
>>> <https://github.com/AnalyticalGraphicsInc/cesium/tree/master/Documentation/Contributors/TestingGuide#testing-guide>,
>>> especially the GLSL section
>>> <https://github.com/AnalyticalGraphicsInc/cesium/tree/master/Documentation/Contributors/TestingGuide#glsl>
>>> .
>>>
>>> Also rename this to czm_WorldCoordinatestoNDC like our color transform
>>> functions.
>>> ------------------------------
>>>
>>> In Source/Scene/PostProcessorScene.js
>>> <#5498 (comment)>
>>> :
>>>
>>> > +
>>> + 'varying vec2 v_textureCoordinates; \n' +
>>> +
>>> + //return ndc from world coordinate biased EarthRadius
>>> + 'vec4 GetNDCFromWC(in vec3 WC, in float EarthRadius) \n' +
>>> + '{ \n' +
>>> + ' vec4 position = vec4(WC, 1.0); \n' +
>>> + ' vec4 positionEC = czm_view * position; \n' +
>>> + ' positionEC = vec4(positionEC.x + EarthRadius , positionEC.y, positionEC.z, 1.0); \n' +
>>> + ' vec4 positionWC = czm_eyeToWindowCoordinates(positionEC); \n' +
>>> + ' return czm_viewportOrthographic * vec4(positionWC.xy, -positionWC.z, 1.0); \n' +
>>> + '} \n' +
>>> +
>>> + //Check if current pixel is included Earth
>>> + //if then mask it gradually
>>> + 'float IsinEarth(in vec2 texcoord, in vec2 SceneSize) \n' +
>>>
>>> Throughout GLSL, we don't use the in, etc. parameter prefixes.
>>> ------------------------------
>>>
>>> In Source/Scene/PostProcessorScene.js
>>> <#5498 (comment)>
>>> :
>>>
>>> > + ' positionEC = vec4(positionEC.x + EarthRadius , positionEC.y, positionEC.z, 1.0); \n' +
>>> + ' vec4 positionWC = czm_eyeToWindowCoordinates(positionEC); \n' +
>>> + ' return czm_viewportOrthographic * vec4(positionWC.xy, -positionWC.z, 1.0); \n' +
>>> + '} \n' +
>>> +
>>> + //Check if current pixel is included Earth
>>> + //if then mask it gradually
>>> + 'float IsinEarth(in vec2 texcoord, in vec2 SceneSize) \n' +
>>> + '{ \n' +
>>> +
>>> + 'vec2 NDC = texcoord * 2.0 - 1.0; \n' +
>>> + 'vec4 EarthPosSC = GetNDCFromWC(vec3(0.0,0.0,0.0), 0.0); \n' +
>>> + 'vec4 EarthPosSCEdge = GetNDCFromWC(vec3( 0.0,0.0,0.0), u_EarthRadius *1.5); \n' +
>>> +
>>> + 'NDC.xy -= EarthPosSC.xy; \n' +
>>> + 'float X = abs(NDC.x)*SceneSize.x; \n' +
>>>
>>> Fix whitespace. Just
>>>
>>> float x = abs(ndc.x) * sceneSize.x;
>>>
>>> (and also fixed naming convention).
>>> ------------------------------
>>>
>>> In Source/Scene/PostProcessorScene.js
>>> <#5498 (comment)>
>>> :
>>>
>>> > + ' positionEC = vec4(positionEC.x + EarthRadius , positionEC.y, positionEC.z, 1.0); \n' +
>>> + ' vec4 positionWC = czm_eyeToWindowCoordinates(positionEC); \n' +
>>> + ' return czm_viewportOrthographic * vec4(positionWC.xy, -positionWC.z, 1.0); \n' +
>>> + '} \n' +
>>> +
>>> + //Check if current pixel is included Earth
>>> + //if then mask it gradually
>>> + 'float IsinEarth(in vec2 texcoord, in vec2 SceneSize) \n' +
>>> + '{ \n' +
>>> +
>>> + 'vec2 NDC = texcoord * 2.0 - 1.0; \n' +
>>> + 'vec4 EarthPosSC = GetNDCFromWC(vec3(0.0,0.0,0.0), 0.0); \n' +
>>> + 'vec4 EarthPosSCEdge = GetNDCFromWC(vec3( 0.0,0.0,0.0), u_EarthRadius *1.5); \n' +
>>> +
>>> + 'NDC.xy -= EarthPosSC.xy; \n' +
>>> + 'float X = abs(NDC.x)*SceneSize.x; \n' +
>>>
>>> The GLSL could use a lot of tweaks like this to look like our other
>>> GLSL code. Feel free to make them now or wait until this is in a separate
>>> .glsl file where it would be easier and the WebStorm auto formatter might
>>> work (we use it for JavaScript).
>>> ------------------------------
>>>
>>> In Source/Scene/PostProcessorScene.js
>>> <#5498 (comment)>
>>> :
>>>
>>> > + 'float Y = abs(NDC.y)*SceneSize.y; \n' +
>>> + 'Y *= Y; \n' +
>>> +
>>> + 'return clamp(0.0, 1.0, max( sqrt(X + Y) / max(abs(EarthPosSCEdge.x*SceneSize.x), 1.0) - 0.8 , 0.0)); \n' +
>>> +
>>> + '} \n' +
>>> +
>>> +
>>> + //For Chromatic effect
>>> + 'vec4 textureDistorted( \n' +
>>> + ' in sampler2D tex, \n' +
>>> + ' in vec2 texcoord, \n' +
>>> + ' in vec2 direction, \n' +
>>> + ' in vec3 distortion, \n' +
>>> + ' in bool bSpace \n' +
>>> + ' ) { \n' +
>>>
>>> In GLSL, we put { on a new line.
>>> ------------------------------
>>>
>>> In Source/Scene/PostProcessorScene.js
>>> <#5498 (comment)>
>>> :
>>>
>>> > + 'X *= X; \n' +
>>> + 'float Y = abs(NDC.y)*SceneSize.y; \n' +
>>> + 'Y *= Y; \n' +
>>> +
>>> + 'return clamp(0.0, 1.0, max( sqrt(X + Y) / max(abs(EarthPosSCEdge.x*SceneSize.x), 1.0) - 0.8 , 0.0)); \n' +
>>> +
>>> + '} \n' +
>>> +
>>> +
>>> + //For Chromatic effect
>>> + 'vec4 textureDistorted( \n' +
>>> + ' in sampler2D tex, \n' +
>>> + ' in vec2 texcoord, \n' +
>>> + ' in vec2 direction, \n' +
>>> + ' in vec3 distortion, \n' +
>>> + ' in bool bSpace \n' +
>>>
>>> Do not use hungarian notation.
>>> ------------------------------
>>>
>>> In Source/Scene/PostProcessorScene.js
>>> <#5498 (comment)>
>>> :
>>>
>>> > + 'float red = 0.0; \n' +
>>> + 'float green = 0.0; \n' +
>>> + 'float blue = 0.0; \n' +
>>> + 'if(bSpace) \n' +
>>> + '{ \n' +
>>> + 'red = IsinEarth(texcoord + direction * distortion.r, SceneSize) * texture2D(tex, texcoord + direction * distortion.r).r; \n' +
>>> + 'green = IsinEarth(texcoord + direction * distortion.g, SceneSize) * texture2D(tex, texcoord + direction * distortion.g).g; \n' +
>>> + 'blue = IsinEarth(texcoord + direction * distortion.b, SceneSize) * texture2D(tex, texcoord + direction * distortion.b).b; \n' +
>>> + '} \n' +
>>> + 'else \n' +
>>> + '{ \n' +
>>> + 'red = texture2D(tex, texcoord + direction * distortion.r).r; \n' +
>>> + 'green = texture2D(tex, texcoord + direction * distortion.g).g; \n' +
>>> + 'blue = texture2D(tex, texcoord + direction * distortion.b).b; \n' +
>>> + '} \n' +
>>> +
>>>
>>> Here are throughout, no need to have all this extra whitespace. We
>>> never use more than one blank line.
>>> ------------------------------
>>>
>>> In Source/Scene/PostProcessorScene.js
>>> <#5498 (comment)>
>>> :
>>>
>>> > +
>>> + 'return vec4(clamp(red, 0.0, 1.0), clamp(green, 0.0, 1.0), clamp(blue, 0.0, 1.0), 0.0); \n' +
>>> +
>>> + '} \n' +
>>> +
>>> +
>>> +
>>> + 'void main(void) \n' +
>>> + '{ \n' +
>>> + ' vec3 rgb = texture2D(u_colorTexture, v_textureCoordinates).rgb; \n' +
>>> +
>>> +
>>> + 'bool bSpace = true; \n' +
>>> +
>>> + //whether it is in space or not
>>> + 'if(length(czm_viewerPositionWC.xyz) < 6500000.0) \n' +
>>>
>>> How did you determine 6500000.0?
>>> ------------------------------
>>>
>>> In Source/Scene/PostProcessorScene.js
>>> <#5498 (comment)>
>>> :
>>>
>>> > +
>>> + 'vec2 texelSize = 1.0 / czm_viewport.zw; \n' +
>>> + 'vec3 distortion = vec3(-texelSize.x * u_Distortion, 0.0, texelSize.x * u_Distortion); \n' +
>>> +
>>> + // ghost vector to image centre:
>>> + 'vec2 ghostVec = (vec2(0.5) - texcoord) * u_GhostDispersal; \n' +
>>> + 'vec3 direction = normalize(vec3(ghostVec, 0.0)); \n' +
>>> +
>>> + // sample ghosts:
>>> + ' vec4 result = vec4(0.0); \n' +
>>> + ' vec4 ghost = vec4(0.0); \n' +
>>> + ' for (int i = 0; i < 4; ++i) { \n' +
>>> + ' vec2 offset = fract(texcoord + ghostVec * float(i)); \n' +
>>> +
>>> + //Only bright spots from the centre of the source image
>>> + //' float weight = length(vec2(0.5) - offset) / length(vec2(0.5)); \n' +
>>>
>>> Delete commented out code.
>>>
>>> If you really need it, it could stay for now, but once this is merged
>>> into master, we do not leave in commented out code.
>>> ------------------------------
>>>
>>> In Source/Scene/PostProcessorScene.js
>>> <#5498 (comment)>
>>> :
>>>
>>> > + 'vec4 halo; \n' +
>>> + ' vec2 haloVec = normalize(ghostVec) * u_HaloWidth; \n' +
>>> + ' float WeightForHalo = length(vec2(0.5) - fract(texcoord + haloVec)) / length(vec2(0.5)); \n' +
>>> + ' WeightForHalo = pow(1.0 - WeightForHalo, 5.0); \n' +
>>> + ' halo = textureDistorted(u_colorTexture, texcoord + haloVec, direction.xy, distortion, bSpace) * WeightForHalo * 1.5; \n' +
>>> +
>>> + ' result += halo; \n' +
>>> + ' result += texture2D(u_DirtTexture, v_textureCoordinates); \n' +
>>> +
>>> +
>>> +
>>> +
>>> + //Rotating starburst texture's coordinate
>>> + ' vec3 camx = vec3(czm_view[0][0], czm_view[0][1], czm_view[0][2] ); \n' +
>>> + ' vec3 camz = vec3(czm_view[1][0], czm_view[1][1], czm_view[1][2] ); \n' +
>>> + ' float camrot = dot(camx, vec3(0, 0, 1)) + dot(camz, vec3(0, 1, 0)); \n' +
>>>
>>> Throughout GLSL, when your intention is floating point, always include
>>> .0, e.g., 0.0, etc.
>>>
>>> On some implementations, not doing so at least use to result in a
>>> compiler error.
>>> ------------------------------
>>>
>>> In Source/Scene/PostProcessorScene.js
>>> <#5498 (comment)>
>>> :
>>>
>>> > + ' float WeightForHalo = length(vec2(0.5) - fract(texcoord + haloVec)) / length(vec2(0.5)); \n' +
>>> + ' WeightForHalo = pow(1.0 - WeightForHalo, 5.0); \n' +
>>> + ' halo = textureDistorted(u_colorTexture, texcoord + haloVec, direction.xy, distortion, bSpace) * WeightForHalo * 1.5; \n' +
>>> +
>>> + ' result += halo; \n' +
>>> + ' result += texture2D(u_DirtTexture, v_textureCoordinates); \n' +
>>> +
>>> +
>>> +
>>> +
>>> + //Rotating starburst texture's coordinate
>>> + ' vec3 camx = vec3(czm_view[0][0], czm_view[0][1], czm_view[0][2] ); \n' +
>>> + ' vec3 camz = vec3(czm_view[1][0], czm_view[1][1], czm_view[1][2] ); \n' +
>>> + ' float camrot = dot(camx, vec3(0, 0, 1)) + dot(camz, vec3(0, 1, 0)); \n' +
>>> +
>>> + ' mat3 rotation = mat3( \n' +
>>>
>>> I don't trust that all GLSL compilers for factor out the common cos
>>> and sin calls. Call each function once above here and assign to local
>>> variables.
>>> ------------------------------
>>>
>>> In package-lock.json
>>> <#5498 (comment)>
>>> :
>>>
>>> > @@ -0,0 +1,5024 @@
>>> +{
>>>
>>> Are you sure this file should be included? I think you should remove it
>>> from git.
>>>
>>> —
>>> You are receiving this because you were mentioned.
>>> Reply to this email directly, view it on GitHub
>>> <#5498 (review)>,
>>> or mute the thread
>>> <https://github.com/notifications/unsubscribe-auth/AVPCk8zTiVCPkTAVvmomrJJvbwgbPn0jks5sG6b2gaJpZM4N9y9Y>
>>> .
>>>
>>
>>
>
|
|
@byumjin not 100% sure about the build error. At some point you have ran correct? Then you get this error when running the following command (or another build step)? You could try deleting your Also, we usually use the GitHub UI to add comments to issues instead of replying to the GitHub notification email since replying to the email includes all the quoted text in the GitHub comment. |
|
Yes, I just deleted my node_modules directory and re-running npm install then npm run build. |
@mramato do you know if we should add |
|
Then, how can I make my fork to be built correctly? |
(Test) commits new files when I run new server
+ add my name on Analytical Graphics, Inc. in CONTRIBUTORS.md
|
Can you send me a link to the failure in Travis CI? Looks like it is passing now: https://travis-ci.org/AnalyticalGraphicsInc/cesium/builds/247264174 |
|
@byumjin please remove all the |
|
@lilleyse please help see this one over the finish line when you are available. |
|
… On Mon, Jun 26, 2017 at 6:12 PM, Patrick Cozzi ***@***.***> wrote:
Can you send me a link to the failure in Travis CI? Looks like it is
passing now: https://travis-ci.org/AnalyticalGraphicsInc/cesium/
builds/247264174
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#5498 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AVPCkzqszkgl5FABLSZf--F2J10ZF8JNks5sICzlgaJpZM4N9y9Y>
.
|
|
That is an older commit; Travis is passing now. |
removed .vs files
|
Yes, and I just removed the .vs files
<e5306ed>
.
And, its commit is also passing now..
…On Mon, Jun 26, 2017 at 6:28 PM, Patrick Cozzi ***@***.***> wrote:
That is an older commit; Travis is passing now.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#5498 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AVPCk7KAdB12BbuIH8Be-6bj5o_twMw7ks5sIDClgaJpZM4N9y9Y>
.
|
|
Perfect, thanks! |
Has added HBAO codes and SandCastle and fixed broken parts of LensFlare when it was merged
|
Lens flare is also included in #5645, I'll do the final review there. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The algorithm of this Lens flare is, actually, for HDR texutre.
But, Cesium does not use HDR texture. So, I have to modify the implementing way to fit with Cesium.
I would like to notify some issues for later maintainer
Zero, there is some kind of initialization problem (Sean already knew this problem).
It will disappear when the user updates variables ( ex) Lens Flare check box, Intensity slide .......)
First, about algorithm, instead of using HDR texture, I used Scene Image for creating Lensflare effect.
So, I masked the portion of sun with shader codes, if then it can also address the situation what sun is blocked by the earth.
Second, at the inner Earth mode, if the sun exists with terrains in the screen space, the terrains are also used to create Lens flare effect.
Third, two texture made by me are being used for Lens Flare (DirtMaskTextureExample.png, StarBurst.png)
It seems that those size are little bit bigger than the images already used for Cesium.
Feel free to modify these, if you want