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 <kyosh...@novell.com>
>From 530449e89eb76b536aeb1a49582cee2d882b0075 Mon Sep 17 00:00:00 2001 From: Kohei Yoshida <kyosh...@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
_______________________________________________ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice