Skip to content
New issue

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

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

Already on GitHub? # to your account

Remove prototype functions #1564

Merged
merged 9 commits into from
Mar 25, 2014
Merged

Remove prototype functions #1564

merged 9 commits into from
Mar 25, 2014

Conversation

hpinkos
Copy link
Contributor

@hpinkos hpinkos commented Mar 21, 2014

#1082

Removed prototype functions from:
AxisAlignedBoundingBox, BoundingRectangle, BoundingSphere, Plane, Ray, Spherical

I left in AxisAlignedBoundingBox.prototype.intersect, BoundingRectangle.prototype.intersect and BoundingSphere.prototype.intersect because one of the CullingVolume tests made it look like it was necessary.

@@ -24,6 +24,19 @@ Beta Releases
### b26 - 2014-03-03

* Breaking changes:
* Removed the following prototype functions. (Use 'static' versions of these functions instead):
* `AxisAlignedBoundingBox.prototype`
* `clone`, `equals`
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we are keeping clone and equals prototype functions like, for example, on Cartesian2.

@pjcozzi
Copy link
Contributor

pjcozzi commented Mar 21, 2014

I left in AxisAlignedBoundingBox.prototype.intersect, BoundingRectangle.prototype.intersect and BoundingSphere.prototype.intersect because one of the CullingVolume tests made it look like it was necessary.

Which test? I don't expect we'll need these since we no longer do view frustum culling polymorphically because it was a pain to avoid allocating new objects every frame.

@pjcozzi
Copy link
Contributor

pjcozzi commented Mar 21, 2014

@hpinkos once we merge this, is there anything else you want to do for #1082?

hpinkos added 2 commits March 25, 2014 10:39
Conflicts:
	Specs/Scene/PolylineCollectionSpec.js
@hpinkos
Copy link
Contributor Author

hpinkos commented Mar 25, 2014

Which test? I don't expect we'll need these since we no longer do view frustum culling polymorphically because it was a pain to avoid allocating new objects every frame.

I initially changed line 50 in CullingVolume.js to be BoundingSphere.intersect(boundingVolume, planes[k]), but CullingVolumeSpec tests both a BoundingSphere and an AxisAlignedBoundingBox.

@hpinkos
Copy link
Contributor Author

hpinkos commented Mar 25, 2014

once we merge this, is there anything else you want to do for #1082?

@pjcozzi The only other one I noticed that seemed to have a lot of prototype functions is Extent. Do we want to change that?

And this branch is ready for review.

@pjcozzi
Copy link
Contributor

pjcozzi commented Mar 25, 2014

@pjcozzi The only other one I noticed that seemed to have a lot of prototype functions is Extent. Do we want to change that?

Yes, let's do that as a separate pull request and then declare #1082 done.

@pjcozzi
Copy link
Contributor

pjcozzi commented Mar 25, 2014

I initially changed line 50 in CullingVolume.js to be BoundingSphere.intersect(boundingVolume, planes[k]), but CullingVolumeSpec tests both a BoundingSphere and an AxisAlignedBoundingBox.

Ah, OK.

* `Ray`
* `getPoint`
* `Spherical`
* `normalize`, `equalsEpsilon`, `toString`
Copy link
Contributor

Choose a reason for hiding this comment

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

Going off of Cartesian2, shouldn't we leave the prototype versions of equalsEpsilon and toString?

@pjcozzi
Copy link
Contributor

pjcozzi commented Mar 25, 2014

Just those questions. Tests, Sandcastle, and JSHint are good.

hpinkos added 3 commits March 25, 2014 15:48
@hpinkos
Copy link
Contributor Author

hpinkos commented Mar 25, 2014

@pjcozzi anything else?

@pjcozzi
Copy link
Contributor

pjcozzi commented Mar 25, 2014

No, looks good.

pjcozzi added a commit that referenced this pull request Mar 25, 2014
@pjcozzi pjcozzi merged commit b5196de into master Mar 25, 2014
@pjcozzi pjcozzi deleted the removePrototype branch March 25, 2014 21:00
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants