Hello Markus
Le Sat, 15 Dec 2012 15:39:26 +0100, Markus Mohrhard
<markus.mohrh...@googlemail.com> a écrit:
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.
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.
Sorry but what I see in 4f901a2f45 is the same thing as Julien pasted in
his mail.
It rollback the good version:
$ git diff
4f901a2f451a552853c1dd38309dd55b22616fdd..4f901a2f451a552853c1dd38309dd55b22616fdd^
diff --git a/sc/source/core/data/colorscale.cxx
b/sc/source/core/data/colorscale.cxx
index f5d23c1..2adf507 100644
--- a/sc/source/core/data/colorscale.cxx
+++ b/sc/source/core/data/colorscale.cxx
@@ -636,8 +636,10 @@ bool NeedUpdate(ScColorScaleEntry* pEntry)
void ScDataBarFormat::DataChanged(const ScRange& rRange)
{
- bool bNeedUpdate = NeedUpdate(mpFormatData->mpUpperLimit.get());
- bNeedUpdate |= NeedUpdate(mpFormatData->mpLowerLimit.get());
+ bool bNeedUpdate = false;
+
+ bNeedUpdate = NeedUpdate(mpFormatData->mpUpperLimit.get());
+ bNeedUpdate &= NeedUpdate(mpFormatData->mpLowerLimit.get());
bNeedUpdate &= GetRange().Intersects(rRange);
Could you check if I missed something ?
Regards
--
Mat M
_______________________________________________
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice
