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
Please register or sign in to comment