Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for entities as reference frames #5817

Closed
wants to merge 19 commits into from

Conversation

speigg
Copy link
Contributor

@speigg speigg commented Sep 7, 2017

Based on previous pull request #2998, with optimizations to eliminate allocations.


This PR is based on the referenceFrames branch, and adds support for constructing trees of entities by allowing entities to be used as reference frames themselves (see #1045). The root reference frame for any tree is either FIXED, INERTIAL, or an entity with an undefined position (an arbitrary frame).

For example:

var e1 = new Entity();
e1.position = new ConstantPositionProperty(new Cartesian3(100000, 200000, 300000)); // implicit FIXED frame
var orientation = new Quaternion(0, 0, 1, 1);
e1.orientation = new ConstantProperty(Quaternion.normalize(orientation, orientation));

var e2 = new Entity();
e2.position = new ConstantPositionProperty(new Cartesian3(200, 400, 600), e1); // note: e1 here
orientation = new Quaternion(1, 0, 0, 1);
e2.orientation = new ConstantProperty(Quaternion.normalize(orientation, orientation));

var value = new Cartesian3(1, 2, 3);
var result = PositionProperty.convertToReferenceFrame(time, value, e2, ReferenceFrame.FIXED);

expect(result).toEqual(new Cartesian3(99603, 200201, 300602));

// Can also pass an arbitrary Entity as the referenceFrame parameter in getValueInReferenceFrame().
// The reference frame can be any Entity... if both entities are not connected, the result is undefined. 
e1.position.getValueInReferenceFrame(time, e2) 

PositionProperty.convertToReferenceFrame has been rewritten to support converting positions between any two reference frames, including any two entities. Likewise, OrientationProperty.convertToReferenceFrame has been added to convert orientations between any two reference frames.

ReferenceEntity class is added to support the '#id' notation for entities as reference frames in CzmlDataSource when the specified Entity does not yet exist in the EntityCollection (https://github.com/AnalyticalGraphicsInc/cesium/wiki/CZML-Content#position).

Specs have been added for PositionProperty and OrientationProperty.
JSHint passes.

Summary of Changes:

  • Modified CzmlDataSource to process ‘#id’
  • Added OrientationProperty (convertToReferenceFrame)
  • Modified PositionProperty (convertToReferenceFrame)
  • Added ReferenceEntity
  • Added OrientationPropertySpec
  • Fixed PositionPropertySpec tests in the referenceFrames branch
  • Added OrientationPropertySpec

Features not included:

  • getValueInReferenceFrame does not yet exist for the orientation property on the Entity class (the more cumbersome OrientationProperty.convertToReferenceFrame has to be used to convert orientations between reference frames instead). It seems that having a getValueInReferenceFrame method for orientation properties would require adding many new *OrientationProperty classes, the equivalent of all the *PositionProperty classes.
  • The ability to specify a reference frame for an orientation other than the reference frame specified by the position property.

mramato and others added 17 commits September 2, 2015 23:59
We can now succesfully translate from any input frame to FIXED or INERTIAL.  Output frames are still a problem.
Cleaned up some tests and added outputFrame tests that currently fail.
…y/unknown) reference frames

Modified CzmlDataSource to process ‘#id’ and null referenceFrames.
Added OrientationProperty (convertToReferenceFrame)
Modified PositionProperty (convertToReferenceFrame)
Added ReferenceEntity
Added OrientationPropertySpec
Fixed PositionPropertySpec tests and added new tests.
…renceFrames

# Conflicts:
#	Source/DataSources/CzmlDataSource.js
Supporting a "null" reference frame is unnecessary, since `new Entity()` without
a position property suffices as an arbitrary reference frame.
Removed allocations within convertToReferenceFrame for PositionProperty and OrientationProperty
@cesium-concierge
Copy link

@speigg, thanks for the pull request! Maintainers, we have a signed CLA from @speigg, 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! Thanks again.

@ggetz
Copy link
Contributor

ggetz commented Sep 8, 2017

Thanks for the pull request @speigg !

@shunter Do you want to take a first look at this?

@cesium-concierge
Copy link

Thanks again for your contribution @speigg!

No one has commented on this pull request in 30 days. Maintainers, can you review, merge or close to keep things tidy?

I'm going to re-bump this in 30 days. If you'd like me to stop, just comment with @cesium-concierge stop. If you want me to start again, just delete the comment.


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

🌍 🌎 🌏

@hpinkos
Copy link
Contributor

hpinkos commented Dec 12, 2018

@shunter @mramato what's the plan for this PR? Is this a change we'd like to take or do you think this should be closed?

@shunter
Copy link
Contributor

shunter commented Dec 12, 2018

Adding this capability is definitely desired. This has been on my list to look at but I don't know when I'll get to it. We need to ensure that it's added in way that enables it to work seamlessly and correctly across our product lines (Components/STK Desktop -> CZML -> Cesium).

When I have availability I expect I'll pick up from where this PR currently stands.

@cesium-concierge
Copy link

Thanks again for your contribution @speigg!

No one has commented on this pull request in 30 days. Maintainers, can you review, merge or close to keep things tidy?

I'm going to re-bump this in 30 days. If you'd like me to stop, just comment with @cesium-concierge stop. If you want me to start again, just delete the comment.


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

🌍 🌎 🌏

@cesium-concierge
Copy link

Thanks again for your contribution @speigg!

No one has commented on this pull request in 30 days. Maintainers, can you review, merge or close to keep things tidy?

I'm going to re-bump this in 30 days. If you'd like me to stop, just comment with @cesium-concierge stop. If you want me to start again, just delete the comment.

4 similar comments
@cesium-concierge
Copy link

Thanks again for your contribution @speigg!

No one has commented on this pull request in 30 days. Maintainers, can you review, merge or close to keep things tidy?

I'm going to re-bump this in 30 days. If you'd like me to stop, just comment with @cesium-concierge stop. If you want me to start again, just delete the comment.

@cesium-concierge
Copy link

Thanks again for your contribution @speigg!

No one has commented on this pull request in 30 days. Maintainers, can you review, merge or close to keep things tidy?

I'm going to re-bump this in 30 days. If you'd like me to stop, just comment with @cesium-concierge stop. If you want me to start again, just delete the comment.

@cesium-concierge
Copy link

Thanks again for your contribution @speigg!

No one has commented on this pull request in 30 days. Maintainers, can you review, merge or close to keep things tidy?

I'm going to re-bump this in 30 days. If you'd like me to stop, just comment with @cesium-concierge stop. If you want me to start again, just delete the comment.

@cesium-concierge
Copy link

Thanks again for your contribution @speigg!

No one has commented on this pull request in 30 days. Maintainers, can you review, merge or close to keep things tidy?

I'm going to re-bump this in 30 days. If you'd like me to stop, just comment with @cesium-concierge stop. If you want me to start again, just delete the comment.

@cesium-concierge
Copy link

Thanks again for your contribution @speigg!

No one has commented on this pull request in 30 days. Maintainers, can you review, merge or close to keep things tidy?

I'm going to re-bump this in 30 days. If you'd like me to stop, just comment with @cesium-concierge stop. If you want me to start again, just delete the comment.

2 similar comments
@cesium-concierge
Copy link

Thanks again for your contribution @speigg!

No one has commented on this pull request in 30 days. Maintainers, can you review, merge or close to keep things tidy?

I'm going to re-bump this in 30 days. If you'd like me to stop, just comment with @cesium-concierge stop. If you want me to start again, just delete the comment.

@cesium-concierge
Copy link

Thanks again for your contribution @speigg!

No one has commented on this pull request in 30 days. Maintainers, can you review, merge or close to keep things tidy?

I'm going to re-bump this in 30 days. If you'd like me to stop, just comment with @cesium-concierge stop. If you want me to start again, just delete the comment.

@hpinkos
Copy link
Contributor

hpinkos commented Sep 18, 2019

@shunter any idea when you might be able to work on this? Should we park this on a branch?

@cesium-concierge
Copy link

Thanks again for your contribution @speigg!

No one has commented on this pull request in 30 days. Maintainers, can you review, merge or close to keep things tidy?

I'm going to re-bump this in 30 days. If you'd like me to stop, just comment with @cesium-concierge stop. If you want me to start again, just delete the comment.

8 similar comments
@cesium-concierge
Copy link

Thanks again for your contribution @speigg!

No one has commented on this pull request in 30 days. Maintainers, can you review, merge or close to keep things tidy?

I'm going to re-bump this in 30 days. If you'd like me to stop, just comment with @cesium-concierge stop. If you want me to start again, just delete the comment.

@cesium-concierge
Copy link

Thanks again for your contribution @speigg!

No one has commented on this pull request in 30 days. Maintainers, can you review, merge or close to keep things tidy?

I'm going to re-bump this in 30 days. If you'd like me to stop, just comment with @cesium-concierge stop. If you want me to start again, just delete the comment.

@cesium-concierge
Copy link

Thanks again for your contribution @speigg!

No one has commented on this pull request in 30 days. Maintainers, can you review, merge or close to keep things tidy?

I'm going to re-bump this in 30 days. If you'd like me to stop, just comment with @cesium-concierge stop. If you want me to start again, just delete the comment.

@cesium-concierge
Copy link

Thanks again for your contribution @speigg!

No one has commented on this pull request in 30 days. Maintainers, can you review, merge or close to keep things tidy?

I'm going to re-bump this in 30 days. If you'd like me to stop, just comment with @cesium-concierge stop. If you want me to start again, just delete the comment.

@cesium-concierge
Copy link

Thanks again for your contribution @speigg!

No one has commented on this pull request in 30 days. Maintainers, can you review, merge or close to keep things tidy?

I'm going to re-bump this in 30 days. If you'd like me to stop, just comment with @cesium-concierge stop. If you want me to start again, just delete the comment.

@cesium-concierge
Copy link

Thanks again for your contribution @speigg!

No one has commented on this pull request in 30 days. Maintainers, can you review, merge or close to keep things tidy?

I'm going to re-bump this in 30 days. If you'd like me to stop, just comment with @cesium-concierge stop. If you want me to start again, just delete the comment.

@cesium-concierge
Copy link

Thanks again for your contribution @speigg!

No one has commented on this pull request in 30 days. Maintainers, can you review, merge or close to keep things tidy?

I'm going to re-bump this in 30 days. If you'd like me to stop, just comment with @cesium-concierge stop. If you want me to start again, just delete the comment.

@cesium-concierge
Copy link

Thanks again for your contribution @speigg!

No one has commented on this pull request in 30 days. Maintainers, can you review, merge or close to keep things tidy?

I'm going to re-bump this in 30 days. If you'd like me to stop, just comment with @cesium-concierge stop. If you want me to start again, just delete the comment.

@hpinkos
Copy link
Contributor

hpinkos commented Jun 15, 2020

@cesium-concierge stop

@devond5
Copy link

devond5 commented Jul 13, 2020

So will there be a convertToReferenceFrame() method for orientation?? or only for position??

@ggetz
Copy link
Contributor

ggetz commented Feb 21, 2024

I'm closing this for now as it's quite out of date with main. If we wish to revisit this in the future, let's open a fresh PR. Thanks all.

@ggetz ggetz closed this Feb 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Status: PR waiting for Cesium
Development

Successfully merging this pull request may close these issues.

7 participants