[Kstars-devel] Stars drawing (speed and color)

[Khudyakov Alexey]

В сообщении от 02 ноября 2009 22:03:33 Akarsh Simha написал:
> > After recent fix in star drawing code I looked into it. I think usage of
> > QString as keys for cache isn't the best thing to do. I've tried to
> > experiment with other indexing schemes. So far I got ~10% speedup for map
> > update with maximum star density. Sadly code become harder to read.
> 
> Wow! That's awesome. I think it is okay to sacrifice code readability
> for the speed increase, considering that it's annoyingly slow right
> now. I think it should be okay to commit it, as long as you have some
> comments explaining what's going on and what the indexing scheme is.
> 
I think speed and readability rarely conflict with each other. Unless you go 
into inner working of computers and even in that case it's not obvious. 
Any algorithm could be written in the form it's reasonably easy to read 
especially if one knows what he is doing.

BTW my experiment with profiling shown that about 8% of time KStars spent on 
QString allocation/deallocation. This might be a cause.

> > Another problematic place is StarObject::initImages. It uses pixmaps of
> > size 15*SkyMap::Instance()->scale(). Then algorithm which actaully create
> > pixmaps assume that they are 15x15 pixels. Which behaviour is correct?
> 
> I think they can be 15x15, since the star size is recomputed using the
> zoom factor anyway. If that's the screen scale, it still shouldn't
> matter. What we must preserve is the relative sizes of stars. I think
> it's okay to use 15x15, unless it looks awful on some very big
> displays. Which one will be faster?
> 
Variant with fixed resolution. Star image is 15x15 pixels anyway. If scale > 1. 
then star pixmap just have invisible margins. They are completely transparent.


> > Another problem I spotted is function StarObject::updateColors. It
> > appears only in commented code in the StarComponent which is marked
> > "REMOVE" [1] Moreover it doesn't know anything about color modes.
> >
> > So question is whether it a historical remnant which should be removed?
> 
> I've noticed that too. I think it's okay to remove it, although I
> really don't know. In any case, we can always revert the change if
> required.
> 
KStars compile without this function. I'll remove it. Unused code tends to rot 
and accumulate bugs over time.