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 Entity.show #2600

Merged
merged 5 commits into from
Mar 25, 2015
Merged

Support for Entity.show #2600

merged 5 commits into from
Mar 25, 2015

Conversation

mramato
Copy link
Contributor

@mramato mramato commented Mar 25, 2015

This change is way less scary than it may look at first, it's just that adding a top-level entity visibility property ended up with small changes throughout the code.

  1. Added Entity.show which is a boolean for easily hiding or showing an entity and its children.
  2. Added Entity.isShowing which is a read-only property that indicates if an entity is currently being drawn (i.e. its show property and all ancestor show properties are true). This is mainly used by the visualizers but could be useful for other stuff as well.
  3. Added support for the KML visibility element, which is mapped directly to Entity.show.
  4. Added a new beginner Sandcastle example for toggling entity visibility.
  5. To support the Sandcastle example, I changed viewer.zoomTo and viewer.flyTo to be "best effort" functions, rather than failing if any one entity is not displayed. This was both requested on the mailing and really an oversight on my part when I initial implemented them. For example, before this change a KML Folder would create an entity with no visualization but with children, zooming to the data would fail as soon as it found the folder, now it ignore the folder and zooms to all data that is actually visualized. This was a one line code change, so don't be alarmed by the length of this bullet point.
  6. On an architectural note, Entities, which previously only knew about their parent, now also know about their children. This was needed to properly propagate setting a parent's show property without dealing with tons of notification subscriptions. I didn't expose this publicly yet because I want to make sure there aren't ramifications that I didn't think of, but I wanted to mention it here in case anyone had any thoughts on the matter.

mramato added 3 commits March 23, 2015 23:03
1. Tests for Entity show/isShowing and events
2. Get rid of `isAvailableAndShowing`, which won't actually be needed in the long run.
1. Update CHANGES and reference documentation.
2. Add support for KML Feature `visibility` element.
3. Make dynamic updates obey Entity.isShowing.
4. Make static material and outline geometry obey Entity.isShowing.
5. Update specs.
6. Add Sandcastle example
7. To support the Sandcastle example, chaned `viewer.zoomTo` and `viewer.flyTo` to be "best effort" functions, rather than failing if any one entity is not displayed.  This was both requested on the mailing list and is needed to zoom to a data source on load or with time-dynamic data.
@mramato
Copy link
Contributor Author

mramato commented Mar 25, 2015

Issue 2600? Where's my Cap'n Crunch whistle?

While it's possible, it would create inconsistency with all other viz.
@mramato mramato mentioned this pull request Mar 25, 2015
@pjcozzi
Copy link
Contributor

pjcozzi commented Mar 25, 2015

All looks good.

pjcozzi added a commit that referenced this pull request Mar 25, 2015
@pjcozzi pjcozzi merged commit a7ba397 into master Mar 25, 2015
@pjcozzi pjcozzi deleted the entity-show-faster branch March 25, 2015 11:53
@mramato mramato mentioned this pull request Apr 20, 2015
70 tasks
child._definitionChanged.raiseEvent(child, 'isShowing', newValue, oldValue);
}
}
entity._definitionChanged.raiseEvent(entity, 'isShowing', isShowing, !isShowing);

Choose a reason for hiding this comment

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

This may be super old, but whenever you're passing 'isShowing' as a param inside of this function.. aren't you expecting to set the 'isShowing' property on the 'entity' object? The issue this raises is that the entity object only has a 'get' method defined but no set method for 'isShowing'

Choose a reason for hiding this comment

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

The issue occurs when dealing with entities from the CompositeEntityCollection. Its own CompositeEntityCollection.prototype._onDefinitionChanged throws an error whenever " entity._definitionChanged.raiseEvent(entity, 'isShowing', isShowing, !isShowing)" event is raised

Copy link
Contributor

Choose a reason for hiding this comment

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

@sceptre12 if you can reproduce this issue/error in a Sandcastle example, please open a new GitHub issue for it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants