Skip to content
Commit d572b2e8 authored by Akarsh Simha's avatar Akarsh Simha
Browse files

Make SkyObject::recomputeCoords() const by recomputing on a clone.

NOTE: This has the possibility of introducing bugs, although I expect
it to solve some hard-to-trace bugs. Hence, it would be good if the
patch were checked by multiple eyes. (Hence copying the mailing list)

The problem:
------------

SkyObject::recomputeCoords() is a method heavily used within SkyObject
and outside to compute the coordinates of a SkyObject at a different
time and location and return a SkyPoint corresponding to these
coordinates, without altering the coordinates of the object itself.

The old implementation of SkyObject::recomputeCoords() did the
following:
1. Save the current coordinates (RA/Dec) of the object
2. Compute coordinates for new context (i.e. location + time)
3. Put back the original coordinates of the object

SkyObject::updateCoords() is invoked to compute the coordinates for
the new context.

However, this can lead to many bugs, since the coordinates are not the
only context-dependent variables that are change. For example,
KSPlanetBase::updateCoords() may update not only the coordinates, but
also the magnitiude, angular size, phase and other parameters of the
planet. So this could lead to bugs where the moon's phase suddenly
changes because of some other computation or the like! (I have noticed
things like this happen earlier)

The solution:
-------------

The obvious solution is to make a true copy (this might make KStars a
bit slow, but the overhead is substantial only for the planets which
are few in number) and do the recomputation on the copy, i.e.
1. Make a copy of the SkyObject
2. Compute the coordinates for the next context on the copy
3. Assign the coordinates to a SkyPoint and return that

However, the solution is not very straightforward, since we cannot
copy just the SkyObject -- we would slice KSPlanet, for
example. Thankfully, Alexey implemented clone() methods in SkyObject
and its subclasses, allowing for true copies. (See previous commits)

Other advantages:
-----------------

Apart from the advantage of (hopefully) solving bugs with respect to
properties of planets being altered, another good side-effect of this
is that the method can be constified as it should be, and so can a
number of methods that use this (see future commits). This helps
ensure that tools don't modify the instances of SkyObject, so the
display on the SkyMap is not artificially altered.

CCMAIL: kstars-devel@kde.org
parent 9824b837
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment