-
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
Added Ellipsoid surface area #8986
Conversation
Thank you so much for the pull request @ErixenCruz! I noticed this is your first pull request and I wanted to say welcome to the Cesium community! The Pull Request Guidelines is a handy reference for making sure your PR gets accepted quickly, so make sure to skim that.
Reviewers, don't forget to make sure that:
|
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.
As I was reading this again just noticed a few areas where the wording could be clearer. Otherwise looks good!
CHANGES.md
Outdated
- Added support for PolylineVolume in CZML [#8841](https://github.com/CesiumGS/cesium/pull/8841) | ||
|
||
##### Fixes :wrench: | ||
|
||
- Fixed `Ellipsoid.geodeticSurfaceNormal` dividing by zero when given origin as input. |
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.
- Fixed `Ellipsoid.geodeticSurfaceNormal` dividing by zero when given origin as input. | |
- Fixed a divide-by-zero bug in `Ellipsoid.geodeticSurfaceNormal` when given the origin as input. `undefined` is returned instead. [#8986](https://github.com/CesiumGS/cesium/pull/8986) |
CHANGES.md
Outdated
@@ -8,10 +8,12 @@ | |||
|
|||
##### Additions :tada: | |||
|
|||
- Added `Ellipsoid.surfaceArea` for computing the surface area under given rectangle. |
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.
- Added `Ellipsoid.surfaceArea` for computing the surface area under given rectangle. | |
- Added `Ellipsoid.surfaceArea` for computing the approximate surface area of a rectangle on the surface of an ellipsoid. |
@@ -3,6 +3,7 @@ import { Cartographic } from "../../Source/Cesium.js"; | |||
import { Ellipsoid } from "../../Source/Cesium.js"; | |||
import { Math as CesiumMath } from "../../Source/Cesium.js"; | |||
import createPackableSpecs from "../createPackableSpecs.js"; | |||
import { Rectangle } from "../../Source/Cesium.js"; |
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.
Just to keep things tidy, move this import
above one line so it's next to the other Cesium.js
imports.
Source/Core/Ellipsoid.js
Outdated
*/ | ||
|
||
/** | ||
* Computes an approximation of the surface area of a given section of the surface of an ellipsoid using |
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.
* Computes an approximation of the surface area of a given section of the surface of an ellipsoid using | |
* Computes an approximation of the surface area of a rectangle on the surface of an ellipsoid using |
Source/Core/Ellipsoid.js
Outdated
* Gauss-Legendre 10th order quadrature. | ||
* | ||
* @param {Rectangle} rectangle The rectangle to find the surface area of. | ||
* @returns {Number} The approximate area of the section on the surface of this ellipsoid. |
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.
* @returns {Number} The approximate area of the section on the surface of this ellipsoid. | |
* @returns {Number} The approximate area of the rectangle on the surface of this ellipsoid. |
Source/Core/Ellipsoid.js
Outdated
* Computes an approximation of the surface area of a given section of the surface of an ellipsoid using | ||
* Gauss-Legendre 10th order quadrature. | ||
* | ||
* @param {Rectangle} rectangle The rectangle to find the surface area of. |
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.
* @param {Rectangle} rectangle The rectangle to find the surface area of. | |
* @param {Rectangle} rectangle The rectangle used for computing the surface area. |
Ok. That last commit should do it @lilleyse Thanks for the feedback. |
Just kidding I missed the CHANGES.md changes but that last commit should do it. |
1f6e9cd
to
5255bb6
Compare
I know I'm not supposed to force, but I did it to fix typo in commit message. |
Thanks @ErixenCruz! |
Fixed
Ellipsoid.geodeticSurfaceNormal
dividing by zero when given origin as input.Added
Ellipsoid.surfaceArea
for computing the surface area under given rectangle.