Hi there,
The attached patch fixes
https://bugs.freedesktop.org/show_bug.cgi?id=36690
and I would like to push this to the -3-4 branch and possibly to the
-3-4-0 branch as well. The patch is against vcl.
Calc races during print preview when the sheet contains buttons with
colors. The reason is as follows:
1. Print preview, which is the output device, tries to paint its own
window which draws the buttons. (ScPreview implements the print preview
window for Calc.)
2. PushButton::Draw() updates the style setting of the output device in
order to change the face color by calling SetSettings() of the output
device.
3. SetSettings() is virtual (!), and Window::SetSettings() receives the
call instead of OutputDevice::SetSettings().
4. Window::SetSettings() calls DataChanged() virtual method to broadcast
the setting change.
5. ScPreView::DataChanged() receives this, sees the style has changed,
and tries to re-paint the whole thing.
6. The cycle continues. :-(
When the PushButton::Draw changes the style setting, I'm pretty sure it
never intends to broadcast the change to anyone, yet it does because
SetSettings is virtual. So what my patch does is to allow it to simply
change the style setting without broadcasting it, and that's enough to
end this infinite re-paint cycle.
As an aside, it really bugs me to have a simple accessor method being
virtual, but that's another story...
Reviews appreciated.
Kohei
--
Kohei Yoshida, LibreOffice hacker, Calc
<kyoshida@novell.com>
From 530449e89eb76b536aeb1a49582cee2d882b0075 Mon Sep 17 00:00:00 2001
From: Kohei Yoshida <kyoshida@novell.com>
Date: Fri, 20 May 2011 00:15:01 -0400
Subject: [PATCH] fdo#36690: Don't broadcast setting changes during painting of button.
Calling SetSettings() when the output device is Window causes it to
broadcast data change. PushButton::Draw() in fact calls this method
when the button contains colors, which ends up broadcasting its change
via Window::DataChanged call. But depending on the output device
this DataChanged call may end up painting it again, which basically
causes a recursive loop.
The solution is to provide a way to only change the settings without
doing anything else, and use that when settings need to be changed
during the painting of a control (like PushButton).
---
vcl/inc/vcl/outdev.hxx | 7 +++++++
vcl/source/control/button.cxx | 2 +-
vcl/source/gdi/outdev.cxx | 7 ++++++-
3 files changed, 14 insertions(+), 2 deletions(-)
diff --git a/vcl/inc/vcl/outdev.hxx b/vcl/inc/vcl/outdev.hxx
index b624cd9..f4f7475 100644
--- a/vcl/inc/vcl/outdev.hxx
+++ b/vcl/inc/vcl/outdev.hxx
@@ -917,6 +917,13 @@ public:
void SetTextAlign( TextAlign eAlign );
TextAlign GetTextAlign() const { return maFont.GetAlign(); }
+ /**
+ * Unlike SetSettings() which is virtual & may do more than just updating
+ * the settings, this method only sets the new settings and do nothing
+ * extra. This method should be called instead of SetSettings() when
+ * changing the settings during painting of controls.
+ */
+ void SetSettingsOnly( const AllSettings& rSettings );
virtual void SetSettings( const AllSettings& rSettings );
const AllSettings& GetSettings() const { return maSettings; }
diff --git a/vcl/source/control/button.cxx b/vcl/source/control/button.cxx
index c77d14a..b0ce14f 100644
--- a/vcl/source/control/button.cxx
+++ b/vcl/source/control/button.cxx
@@ -1522,7 +1522,7 @@ void PushButton::Draw( OutputDevice* pDev, const Point& rPos, const Size&
rSize,
else
aStyleSettings.SetFaceColor( GetSettings().GetStyleSettings().GetFaceColor() );
aSettings.SetStyleSettings( aStyleSettings );
- pDev->SetSettings( aSettings );
+ pDev->SetSettingsOnly( aSettings );
}
pDev->SetTextFillColor();
diff --git a/vcl/source/gdi/outdev.cxx b/vcl/source/gdi/outdev.cxx
index da9ddc3..96a5e1d 100644
--- a/vcl/source/gdi/outdev.cxx
+++ b/vcl/source/gdi/outdev.cxx
@@ -2558,7 +2558,7 @@ void OutputDevice::EnableOutput( sal_Bool bEnable )
// -----------------------------------------------------------------------
-void OutputDevice::SetSettings( const AllSettings& rSettings )
+void OutputDevice::SetSettingsOnly( const AllSettings& rSettings )
{
maSettings = rSettings;
@@ -2566,6 +2566,11 @@ void OutputDevice::SetSettings( const AllSettings& rSettings )
mpAlphaVDev->SetSettings( rSettings );
}
+void OutputDevice::SetSettings( const AllSettings& rSettings )
+{
+ SetSettingsOnly(rSettings);
+}
+
// -----------------------------------------------------------------------
sal_uInt16 OutputDevice::GetBitCount() const
--
1.7.3.4
Context
- [Libreoffice] [REVIEW] fix for fdo#36690 · Kohei Yoshida
Privacy Policy |
Impressum (Legal Info) |
Copyright information: Unless otherwise specified, all text and images
on this website are licensed under the
Creative Commons Attribution-Share Alike 3.0 License.
This does not include the source code of LibreOffice, which is
licensed under the Mozilla Public License (
MPLv2).
"LibreOffice" and "The Document Foundation" are
registered trademarks of their corresponding registered owners or are
in actual use as trademarks in one or more countries. Their respective
logos and icons are also subject to international copyright laws. Use
thereof is explained in our
trademark policy.