On 15/12/2012 15:39, Markus Mohrhard wrote:
Hey,
I noticed these lines in file sc/source/core/data/colorscale.cxx
(because cppcheck report indicates that bNeedUpdate is reassigned before to
be used line 641)
637 void ScDataBarFormat::DataChanged(const ScRange& rRange)
638 {
639 bool bNeedUpdate = false;
640
641 bNeedUpdate = NeedUpdate(mpFormatData->mpUpperLimit.get());
642 bNeedUpdate&= NeedUpdate(mpFormatData->mpLowerLimit.get());
643
644 bNeedUpdate&= GetRange().Intersects(rRange);
645
646 if(bNeedUpdate)
647 {
648 mpDoc->RepaintRange(GetRange());
649 }
650 }
First I wonder if we shouldn't avoid to bitwise and and logical and. But
above all, shouldn't it be logical OR?
Only for the first one. For the second one the OR is totally wrong and
will force always a repainting which is a preformance nightmare.
Ok. I thought that 1 NeedUpdate should have triggered a RepaintRange.
The function really should be called NeedPerhapsUpdate (just kidding of
course :-))
So I would rather put:
if( (NeedUpdate(mpFormatData->mpUpperLimit.get()))
|| (NeedUpdate(mpFormatData->mpLowerLimit.get())) ||
(GetRange().Intersects(rRange)) )
{
mpDoc->RepaintRange(GetRange());
}
What do you think?
Implemented a correct version with 4f901a2f451a552853c1dd38309dd55b22616fdd.
Thank you Markus for the patch and for your feedback.
Julien
_______________________________________________
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice
