[PATCH] coverity#983561: Arguments in wrong order

Regina Henschel rb.henschel at t-online.de
Sun Mar 31 06:09:44 PDT 2013


Hi Julian,

I put you in CC, because I don't know whether my post to the list needs 
moderation again.

julien2412 schrieb:
> Hi Regina,
>
> I knew I was right to submit this patch for review and not push it directly
> :-)
> In fact, I just picked up a coverity report and took a look to Opengrok (see
> http://opengrok.libreoffice.org/search?q=getPointFromCartesian&project=core&defs=&refs=&path=&hist=).
> Ovbiously, I know nothing about 3D coordinate system, now could argument
> names be changed a little so it wouldn't confuse Coverity scan (and also me
> :-) ) ? (perhaps include your explanation in source code or a link to
> Wikipedia or something). I had searched on Wikipedia but didn't find/didn't
> know how to search about this.

The function name getPointFromCartesian is misleading and the comment is 
not correct. In fact this helper function does not convert _from_ but 
_to_ cartesian coordinates. In addition, I'm not sure about the ranges 
of fVer and fHor given in the comment. I think, it is the other way round.

The whole part has been implemented with CWS aw033. Unfortunately Armin 
is very busy with the sidebar, otherwise I would have said "ask Armin".

The Wikipedia article is 
http://en.wikipedia.org/wiki/Spherical_coordinate_system
There you find (search for it),"Conversely, the Cartesian coordinates 
may be retrieved from the spherical coordinates (radius r, inclination 
θ, azimuth φ), where r ∈ [0, ∞), φ ∈ [0, 2π], θ ∈ [0, π], by:

     x=r  sin θ cos φ
     y=r sin θ sin φ
     z=r cos θ

The radius r is 1 for the helper function, because it is used to 
generate a unit sphere.

But it does not fit directly, because the reference plane is not 
x-y-plane, but x-z-plane, and the orientation of the angles are 
different. Therefore I would not use "inclination" and "azimuth".

Perhaps it is enough to rename the parameters in the helper function to 
(fVerAngle, fHorAngle) or (fAngleXZ, fAngleY) or something of this kind?

Kind regards
Regina


More information about the LibreOffice mailing list