Particle System Sandcastle Examples and Updates #6375

hanbollar · 2018-03-27T17:30:13Z

Adding two new sandcastle examples for Particle Systems

modifying certain aspects of the Particle System class as well

cesium-concierge · 2018-03-27T17:30:15Z

@hanbollar, thanks for the pull request! Maintainers, we have a signed CLA from @hanbollar, so you can review this at any time.

⚠️ I noticed that CHANGES.md has not been updated. If this change updates the public API in any way, fixes a bug, or makes any non-trivial update, please add a bullet point to CHANGES.md and comment on this pull request so we know it was updated. For more info, see the Pull Request Guidelines.

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

🌍 🌎 🌏

ggetz

Thanks @hanbollar!

This is a great start, I have some comments on cleaning up the examples a little, as well as a few visual suggestions.

ggetz · 2018-03-28T21:01:03Z

Apps/Sandcastle/gallery/Particle System Tails.html

+    function startup(Cesium) {
+        'use strict';
+//Sandcastle_Begin
+// Particle Image Grabber


~~Particle Image Grabber~~ Draw a particle image to a canvas

Just to be super clear what approach you're taking.

ggetz · 2018-03-28T21:03:15Z

Apps/Sandcastle/gallery/Particle System Tails.html

+    shouldAnimate : true
+});
+
+viewer.scene.debugShowFramesPerSecond = true;


Since this is more of a visuals demo then a performance demo, I don't think this is needed.

ggetz · 2018-03-28T21:03:40Z

Apps/Sandcastle/gallery/Particle System Tails.html

+    return particleCanvas;
+}
+
+// Cesium Viewer Information


Remove this comment, it's included in every Sandcastle example.

ggetz · 2018-03-28T21:07:42Z

Apps/Sandcastle/gallery/Particle System Tails.html

+var cartographicCenterLocation = Cesium.Cartographic.fromCartesian(centerLocation);
+var cartographicIncrease = 800.0;
+cartographicCenterLocation.height += cartographicIncrease;
+centerLocation = Cesium.Cartographic.toCartesian(cartographicCenterLocation);


Why do we need these values? I think there's probably an easier way to accomplish what you are trying to get at here.

See this Sandcastle example if you're trying to orient something compared to the plane entity.

ggetz · 2018-03-28T21:11:07Z

CHANGES.md

@@ -43,6 +45,7 @@ Change Log
 * Fixed rendering vector tiles when using `invertClassification`. [#6349](https://github.com/AnalyticalGraphicsInc/cesium/pull/6349)
 * Fixed animation for glTF models with missing animation targets. [#6351](https://github.com/AnalyticalGraphicsInc/cesium/pull/6351)
 * Fixed double-sided flag for glTF materials with `BLEND` enabled. [#6371](https://github.com/AnalyticalGraphicsInc/cesium/pull/6371)
+* Fixed typo in ParticleSystem documentation. [#6375](https://github.com/AnalyticalGraphicsInc/cesium/pull/6375)


This is really small, I don't think it's worth noting here.

ggetz · 2018-03-28T21:37:37Z

Apps/Sandcastle/gallery/Particle System Weather.html

+
+        scene.skyAtmosphere.hueShift = -0.92;
+        scene.skyAtmosphere.saturationShift = 0.25;
+        scene.skyAtmosphere.brightnessShift = -0.4;


I like the sky color changes, nice touch.

ggetz · 2018-03-28T21:38:01Z

Apps/Sandcastle/gallery/Particle System Weather.html

+
+    #toolbar .header {
+        font-weight: bold;
+    }


Again, make sure you're using all these styles.

ggetz · 2018-03-28T21:38:32Z

Apps/Sandcastle/gallery/Particle System Weather.html

@@ -0,0 +1,186 @@
+<!DOCTYPE html>


Make sure you're thumbnail is sized correctly.

Sandcastle provides a button to scale the output to the correct thumbnail size. You just need a paint program that can auto-crop to that after a screen capture.

ggetz · 2018-03-28T21:41:37Z

Apps/Sandcastle/gallery/Particle System Tails.html

+    usingOptions.particleSystems.push(item);
+}
+
+function runParticles(usingOptions) {


Perhaps add a comment here like "Create several particle systems encircling the base."

ggetz · 2018-03-28T21:42:44Z

Apps/Sandcastle/gallery/Particle System Weather.html

+    shouldAnimate : true
+});
+viewer.terrainProvider = new Cesium.CesiumTerrainProvider({
+    url : 'https://assets.agi.com/stk-terrain/v1/tilesets/world/tiles'


We should be using the Cesium World Terrain now.

pjcozzi · 2018-04-04T15:54:25Z

CHANGES.md

@@ -33,6 +33,8 @@ Change Log
 * Added support for ordering in `DataSourceCollection` [#6316](https://github.com/AnalyticalGraphicsInc/cesium/pull/6316)
  * All ground geometry from one `DataSource` will render in front of all ground geometry from another `DataSource` in the same collection with a lower index.
  * Use `DataSourceCollection.raise`, `DataSourceCollection.lower`, `DataSourceCollection.raiseToTop` and `DataSourceCollection.lowerToBottom` functions to change the ordering of a `DataSource` in the collection.
+* Added more ParticleSystem Sandcastle examples for rocket and comet tails and weather. [#6375](https://github.com/AnalyticalGraphicsInc/cesium/pull/6375)


Please move this to 1.45

pjcozzi · 2018-04-04T15:55:20Z

Can this be accompanied by a short blog post or tutorial with tips on how to setup different effects like this?

pjcozzi · 2018-04-04T16:03:19Z

Is this hosted somewhere? Would like to check out the demos but I can't fight the SSL error I am getting right now with this fork.

ggetz · 2018-04-04T16:39:13Z

Can this be accompanied by a short blog post or tutorial with tips on how to setup different effects like this?

That's the plan! PR from @hanbollar to the website incoming.

Is this hosted somewhere? Would like to check out the demos but I can't fight the SSL error I am getting right now with this fork.

Not currently, but it can be, see response in #6423

hanbollar · 2018-04-12T19:43:36Z

@ggetz just as a question - i really dont like calling the example for the rocket and comet - Particle System Tails - if you have any better ideas for how to name this pls lmk... i just couldnt think of anything better

hanbollar · 2018-04-12T20:00:58Z

updated all particle system examples to include the proper deprecations.

as a note - testing on localhost the visual will seem wrong because dont actually have the deprecations in this branch. waiting on #6429 to actually have the deprecations in master.

ggetz · 2018-04-12T20:16:50Z

@hanbollar You can merge that branch into this one and change the target of this PR from master to that branch.

ggetz · 2018-04-12T20:17:28Z

For the naming, "trails" or "streams"?

ggetz · 2018-04-19T13:22:40Z

@hanbollar Please retarget the base to be the new particle-system-updates branch and resolve any conflicts.

…rial

hanbollar · 2018-04-20T16:49:41Z

@ggetz resolved conflicts

remove unused imports

…delete-data Allow Resource.delete to send data

ggetz · 2018-04-24T14:01:56Z

Apps/Sandcastle/gallery/Particle System Tails.html

+var scratchCartographic = new Cesium.Cartographic();
+var forceFunction = function(options, iteration) {
+    var iterationOffset = iteration;
+    var func = function(particle) {


No abbreviations in variable names, but you should just be able to return the function directly without saving it to a variable.

ggetz · 2018-04-24T14:05:46Z

Source/Core/EllipseGeometry.js

@@ -19,11 +19,9 @@ define([
        './IndexDatatype',


These file changes shouldn't be here. Make sure your branch is up to date with the particle-updates branch by merging that branch into your instead of master like normal.

ggetz · 2018-04-24T14:06:06Z

Apps/Sandcastle/gallery/Particle System Weather.html

+    particle.velocity = Cesium.Cartesian3.add(particle.velocity, snowGravityScratch, particle.velocity);
+
+    var distance = Cesium.Cartesian3.distance(scene.camera.position, particle.position);
+    if (distance > (snowRadius)) {


These parenthesis aren't needed.

ggetz · 2018-04-24T14:07:37Z

Apps/Sandcastle/gallery/Particle System Weather.html

+    particle.position = Cesium.Cartesian3.add(particle.position, rainGravityScratch, particle.position);
+
+    var distance = Cesium.Cartesian3.distance(scene.camera.position, particle.position);
+    if (distance > (rainRadius)) {


Again, unneeded parenthesis.

ggetz · 2018-04-24T14:09:43Z

Apps/Sandcastle/gallery/Particle System Tails.html

+});
+
+// creating particles model matrix
+var transl = Cesium.Matrix4.fromTranslation(particlesOffset, new Cesium.Matrix4());


Don't abbreviate variables.

ggetz · 2018-04-24T14:11:52Z

Apps/Sandcastle/gallery/Particle System Tails.html

+        scratchCartographic = Cesium.Cartographic.fromCartesian(particle.position, Cesium.Ellipsoid.WGS84, scratchCartographic);
+
+        var angle = Cesium.Math.PI * 2.0 * iterationOffset / options.numberOfSystems;
+        iterationOffset += options.iterationOffset;


why are we saving

var iterationOffset = iteration;

then increment this new variable, and then not doing anything with it? Unless I'm missing something, I think we can remove that and just use iteration direction in the above angle calculation.

ggetz · 2018-04-24T14:12:47Z

Apps/Sandcastle/gallery/Particle System Tails.html

+            imageSize : imageSize,
+            emissionRate : 30.0,
+            emitter : new Cesium.CircleEmitter(0.1),
+            bursts : [ ],


If this is an empty array, we can just omit it.

ggetz · 2018-04-24T14:13:25Z

I'm still getting deprecation errors when I adjust the sliders in the original particle system demo. Please make sure we are using the new member names, (and update the labels on the UI if needed).

ggetz · 2018-04-24T14:14:42Z

Thanks @hanbollar, let's finish getting this all cleaned up!

hanbollar · 2018-04-24T15:21:34Z

@ggetz thanks for the feedback! - updated - lmk if there's anything else

ggetz · 2018-04-24T15:33:45Z

@hanbollar There are still a lot of extraneous file changes here, please make sure the only changes are your own. Let me know if we need to troubleshoot with git.

hanbollar · 2018-04-24T15:38:07Z

@ggetz yea... i kinda accidentally merged in master first before seeing the note:

Make sure your branch is up to date with the particle-updates branch

i'll work on fixing it

…ystem-blog-tutorial

…rial

ggetz · 2018-04-24T17:40:04Z

Apps/Sandcastle/gallery/Particle System Weather.html

+    emitter : new Cesium.SphereEmitter(rainRadius),
+    startScale : 1.0,
+    endScale : 0.0,
+    image : "../../SampleData/circular_particle.png",


Cesium code style - Use single quotes ', not double quotes "

Sorry I missed that previously, update throughout.

ggetz · 2018-04-24T17:47:12Z

Apps/Sandcastle/gallery/Particle System.html

-
-    maximumWidth : viewModel.particleSize,
-    maximumHeight : viewModel.particleSize,
+    imageSize : new Cesium.Cartesian2(viewModel.particleSize, viewModel.particleSize),


Using the slider for "Size" in the example throws errors. You'll need to have this observable be one number value, then update imageSize accordingly when update in the following function:

Cesium.knockout.getObservable(viewModel, 'particleSize').subscribe(

hanbollar · 2018-04-24T18:25:12Z

@ggetz lmk if u think there's further updates

ggetz · 2018-04-24T18:38:59Z

Apps/Sandcastle/gallery/Particle System Weather.html

+        scene.fog.minimumBrightness = 0.8;
+    }
+}, {
+    text : "Rain",


You're two text strings should also use single quotes.

ggetz · 2018-04-24T18:39:33Z

CHANGES.md

 * Added color and scale attributes to the `ParticleSystem` class constructor. When defined the variables override startColor and endColor and startScale and endScale. [#6429](https://github.com/AnalyticalGraphicsInc/cesium/pull/6429)
-* Improved `MapboxImageryProvider` performance by 300% via `tiles.mapbox.com` subdomain switching. [#6426](https://github.com/AnalyticalGraphicsInc/cesium/issues/6426)
-* Added ability to invoke `sampleTerrain` from node.js to enable offline terrain sampling


those are above the line - "Added more Particle System Sandcastle examples" - so it's more of just a reordering

ggetz · 2018-04-24T18:50:35Z

Apps/Sandcastle/gallery/Particle System.html

+            particleSystem.imageSize.y = particleSize;
+        } else {
+            particleSystem.imageSize = new Cesium.Cartesian2(particleSize, particleSize);
+        }


This is still broken. imageSize will never be defined- it's not a property of the class. (Additionally you should avoid creating a new Cartesian2 each update).

Replacing the function body with this should work:

particleSystem.minimumImageSize.x = newValue; particleSystem.minimumImageSize.y = newValue; particleSystem.maximumImageSize.x = newValue; particleSystem.maximumImageSize.y = newValue;

ggetz · 2018-04-25T15:09:20Z

Thanks @hanbollar! Now open a PR from the particle-system-updates branch into master.

ggetz requested changes Mar 28, 2018

View reviewed changes

pjcozzi reviewed Apr 4, 2018

View reviewed changes

proper updates

5fc1db4

hanbollar force-pushed the particle-system-blog-tutorial branch from 318abae to 5fc1db4 Compare April 5, 2018 14:57

hanbollar added 5 commits April 5, 2018 10:59

updated changes.md

e2379d5

remove changes in particle system class

12d0f3c

fix eslint errors

46ac1d7

Merge branch 'master' into particle-system-blog-tutorial

3e5b3ce

Merge branch 'master' into particle-system-blog-tutorial

2ad4965

updates for deprecations

93e7c45

hanbollar added 2 commits April 17, 2018 13:20

Merge branch 'master' into particle-system-blog-tutorial

2100df0

updated changes md for weird formatting after merging

4a1ba16

hanbollar mentioned this pull request Apr 18, 2018

Particle system updates and name changes #6429

Merged

3 tasks

Merge branch 'master' into particle-system-blog-tutorial

24cff24

ggetz mentioned this pull request Apr 19, 2018

New Particle System Sandcastle examples hanbollar/cesium#1

Closed

hanbollar changed the base branch from master to particle-system-updates April 20, 2018 16:43

Merge branch 'particle-system-updates' into particle-system-blog-tuto…

6a99168

…rial

Hannah and others added 3 commits April 20, 2018 13:04

Merge pull request CesiumGS#6470 from puckey/remove-unused-imports

ecdfd18

remove unused imports

Merge pull request CesiumGS#6468 from AnalyticalGraphicsInc/resource-…

3b37fab

…delete-data Allow Resource.delete to send data

updated for merge

50bc8fe

ggetz reviewed Apr 24, 2018

View reviewed changes

updates based on ggetz comments

bc7ad08

hanbollar force-pushed the particle-system-blog-tutorial branch from e9e1248 to bc7ad08 Compare April 24, 2018 14:49

hanbollar added 5 commits April 24, 2018 12:14

fixed conflicts

5c36af6

updates

02b258e

Merge remote-tracking branch 'cesium-upstream/master' into particle-s…

fb8fbbb

…ystem-blog-tutorial

Merge branch 'particle-system-updates' into particle-system-blog-tuto…

e140be5

…rial

Merge branch 'particle-system-updates' into particle-system-blog-tuto…

884b3ac

…rial

ggetz reviewed Apr 24, 2018

View reviewed changes

updates

091558a

hanbollar force-pushed the particle-system-blog-tutorial branch from 11ff9db to 091558a Compare April 24, 2018 19:15

Resolve CHANGES.md

5af2e55

ggetz approved these changes Apr 25, 2018

View reviewed changes

ggetz merged commit c5016cc into CesiumGS:particle-system-updates Apr 25, 2018

hanbollar mentioned this pull request Apr 25, 2018

Particle system updates and sandcastle examples #6494

Merged

Particle System Sandcastle Examples and Updates #6375

Particle System Sandcastle Examples and Updates #6375

Conversation

hanbollar commented Mar 27, 2018

cesium-concierge commented Mar 27, 2018

ggetz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pjcozzi commented Apr 4, 2018

pjcozzi commented Apr 4, 2018

ggetz commented Apr 4, 2018

hanbollar commented Apr 12, 2018

hanbollar commented Apr 12, 2018 • edited

ggetz commented Apr 12, 2018

ggetz commented Apr 12, 2018

ggetz commented Apr 19, 2018

hanbollar commented Apr 20, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ggetz commented Apr 24, 2018

ggetz commented Apr 24, 2018

hanbollar commented Apr 24, 2018

ggetz commented Apr 24, 2018

hanbollar commented Apr 24, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hanbollar commented Apr 24, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ggetz commented Apr 25, 2018 • edited

hanbollar commented Apr 12, 2018 •

edited

hanbollar commented Apr 20, 2018 •

edited

ggetz commented Apr 25, 2018 •

edited