-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
Refactor DyanmicScene Properties #1080
Conversation
1. Remove `getValueCartographic` 2. Rename `getValueCartesian` to getValue. 3. Remove `getValueRangeCartesian` since it is unused. 4. Also start to clean up DyanmicDirectionsProperty.
…or DynamicDirectionsProperty. In reality, I might abandon what I've done so far in favor of a new approach.
Conflicts: Source/DynamicScene/DynamicConeVisualizerUsingCustomSensor.js
DynamicPositionProperty is still using it, but that will change shortly.
Conflicts: Source/DynamicScene/ConstantProperty.js Source/DynamicScene/CzmlUnitSpherical.js Source/DynamicScene/DynamicBillboard.js Source/DynamicScene/DynamicBillboardVisualizer.js Source/DynamicScene/DynamicColorMaterial.js Source/DynamicScene/DynamicCone.js Source/DynamicScene/DynamicDirectionsProperty.js Source/DynamicScene/DynamicEllipse.js Source/DynamicScene/DynamicEllipsoid.js Source/DynamicScene/DynamicGridMaterial.js Source/DynamicScene/DynamicImageMaterial.js Source/DynamicScene/DynamicLabel.js Source/DynamicScene/DynamicLabelVisualizer.js Source/DynamicScene/DynamicObject.js Source/DynamicScene/DynamicObjectView.js Source/DynamicScene/DynamicPath.js Source/DynamicScene/DynamicPoint.js Source/DynamicScene/DynamicPointVisualizer.js Source/DynamicScene/DynamicPolygon.js Source/DynamicScene/DynamicPolyline.js Source/DynamicScene/DynamicPolylineVisualizer.js Source/DynamicScene/DynamicPositionProperty.js Source/DynamicScene/DynamicPyramid.js Source/DynamicScene/DynamicPyramidVisualizer.js Source/DynamicScene/DynamicVector.js Source/DynamicScene/DynamicVertexPositionsProperty.js Source/DynamicScene/ReferenceProperty.js Specs/MockProperty.js
1. Move some of the specs to SampledPropertySpec. 2. Temporarily comment out path visualizer. 3. Use a regular property for now instead of position properties.
1. Introduce InterpolatableValue interface and make most basic types we support implement it. 2. Add InterpolatableNumber. 3. Update SampledProperty to use the InterpolatableValue interface rather than specialized helper classes.
1. Delete a lot of now dead code in preparation of bigger CZML parsing changes. 2. Move `processCzml` to `CzmlDataSource.processCzml` for now, but we may get rid of it entirely.
Instead, use a mix of native JavaScript and Core types.
return vectorUpdated; | ||
} | ||
|
||
var updaters = [processClock,// |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the current state, there's no easy way to add additional updaters, since the public API doesn't provide any way to pass updaterFunctions
to _processCzml
. I guess for now we can just attach this array of defaults to CzmlDataSource
to take the place of the former CzmlDefaults
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was an oversight on my part. I agree we can expose CzmlDataSource.updaters for now, and I suppose we can also expose all of the updater functions as well so that people can add/remove them. I also forgot to update changes to mention this.
First round of changes are in. All current review comments have been addressed. |
this._xTable = []; | ||
this._yTable = []; | ||
this._packedInterpolationLength = packedInterpolationLength; | ||
this._updateTables = true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updateTableLength
or updateTableSize
would be clearer, I think, since we always update the tables' contents regardless.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed.
This looks fine to me. Anyone else want to review? Otherwise, I see no need to delay this past next week's release. |
We'll need to update CHANGES, but otherwise we can merge this if you don't see any reason not to. |
@mramato Thanks for doing this! Makes the czml processing code a lot easier to understand. |
Thanks @cmorse It's nice to know we're headed in the right direction. We have more big changes planned to hopefully make the whole thing easier to use. I want it to be trivial for users to easily get there data into Cesium, whether they are using CZML or not. The next big pass is going to deal with optimizing visualizers so we can use alternate rendering paths automatically based on the type of data the user supplies. |
@shunter I updated changes to everything is in b20. This is ready whenever you want to merge it. |
Also fix bad doc do to re-use of throwInstantiationError
Conflicts: CHANGES.md
@mramato That's great to hear. The new CesiumViewer class and this commit are a big step towards it being much easier to get working with Cesium. |
Conflicts: CHANGES.md
…erties Refactor DyanmicScene Properties
This is a huge pull request that technically has many breaking changes, but in reality; almost all of our users' own code will be unaffected (for example, no sandcastle examples needed to be modified). This change is stage 1 of the DynamicScene & DataSource roadmap (#1045) and vastly improves the underlying property system used by DynamicScene. The monolithic DynamicProperty has been split into many smaller purpose-built objects that make writing DataSources much easier.
This pull request is all about infrastructure, there is no change to current functionality other than exposing new API and removing some obsolete ones. Stage 2 will consist of rewriting the Visualizers to take advantage of the new property system and greatly optimize rendering, by both reducing the amount of work done for static objects as well as taking alternate rendering paths when Geometry and Appearances can be used. This will make static data like GeoJSON and KML fly, and should also provide improvements to dynamic demos like Lots of Satellites.
I don't care if this goes into
b20
. I even started ab21
for CHANGES because I figured it will take a while for this to merge. I know @shunter has already been looking at this, @emackey this should barely affect the code in your app (if at all), but if you want to try it out and confirm that before we merge as a beta test, that would be great.Directly from CHANGES
processCzml
, useCzmlDataSource
instead.DynamicScene
property system. See #1080 for complete details.CzmlBoolean
,CzmlCartesian2
,CzmlCartesian3
,CzmlColor
,CzmlDefaults
,CzmlDirection
,CzmlHorizontalOrigin
,CzmlImage
,CzmlLabelStyle
,CzmlNumber
,CzmlPosition
,CzmlString
,CzmlUnitCartesian3
,CzmlUnitQuaternion
,CzmlUnitSpherical
, andCzmlVerticalOrigin
since they are no longer needed.DynamicProperty
,DynamicMaterialProperty
,DynamicDirectionsProperty
, andDynamicVertexPositionsProperty
; replacing them with an all new system of properties.Property
- base interface for all properties.CompositeProperty
- a property composed of other properties.ConstantProperty
- a property whose value never changes.SampledProperty
- a property whose value is interpolated from a set of samples.TimeIntervalCollectionProperty
- a property whose value changes based on time interval.MaterialProperty
- base interface for all material properties.CompositeMaterialProperty
- aCompositeProperty
for materials.ColorMaterialProperty
- a property that maps to a color material. (replacesDynamicColorMaterial
)GridMaterialProperty
- a property that maps to a grid material. (replacesDynamicGridMaterial
)ImageMaterialProperty
- a property that maps to an image material. (replacesDynamicImageMaterial
)PositionProperty
- base interface for all position properties.CompositePositionProperty
- aCompositeProperty
for positions.ConstantPositionProperty
- a whose value does not change in respect to theReferenceFrame
in which is it defined.SampledPositionProperty
- aSampledProperty
for positions.TimeIntervalCollectionPositionProperty
- ATimeIntervalCollectionProperty
for positions.Packable
andPackableForInterpolation
interfaces to aid interpolation and in-memory data storage. Also made most core Cesium types implement them.InterpolationAlgorithm
interface to codify the base interface already being used byLagrangePolynomialApproximation
,LinearApproximation
, andHermitePolynomialApproximation
.