#14917 closed defect (fixed)

Custom background doesn't work for wxScrolledWindow in wxMSW

Reported by: vadz Owned by:
Priority: normal Milestone: 2.9.5
Component: wxMSW Version:
Keywords: background erase Cc:
Blocked By: Blocking:
Patch: yes

Description

Erase sample shows several problems under Windows 7:

  1. With the default wxBG_STYLE_ERASE (i.e. after startup) scrolling doesn't work at all, display can be not updated or partially updated or worse.
  2. Scrolling works fine with wxBG_STYLE_SYSTEM but if a background bitmap is set too, then it's not redrawn correctly when scrolling.
  3. With wxBG_STYLE_PAINT and "Erase background in EVT_PAINT" option on, the custom background is not shown at all in the main window, of course, but it still shows through wxStaticText and wxStaticBitmap and is still corrupted when scrolling.

It would be great to fix at least the first problem as it's probably a regression. Any help with finding the change that broke it would be very welcome.

Attachments (2)

erase_sample_fix1.patch download (563 bytes) - added by ericj 19 months ago.
erase_sample_fix2.patch download (3.3 KB) - added by ericj 18 months ago.
patch to msw/window..cpp to fix misalignment of background brush in scrolled windows. Also refactors the "erase" sample a bit more to fix redraw problems

Download all attachments as: .zip

Change History (12)

Changed 19 months ago by ericj

comment:1 Changed 19 months ago by ericj

First of all i don't think it's a regression. Even in 2.8.12 the "erase" sample has redraw errors when the window gets or is scrolled. It's also not limited to Windows 7, occurs under XP, too.

I think there are several different problems which need to be fixed one after another.

The first problem is in the sample code itself as it doesn't handle scrolled content correctly. The small patch i added fixes this in one spot.

The next problem arises when "use background bitmap" is switched on. When the background is drawn, it doesn't take the scroll position into account, so it starts drawing at the top of the bitmap which doesn't line up with the rest of the drawing if only a portion is the window is updated. This is visible when you scroll or resize the window. I don't know how or where to fix this. Any hints?

comment:2 follow-up: Changed 19 months ago by vadz

  • Status changed from new to confirmed

Thanks for looking at this Eric! You risk becoming wxWidgets CDO (Chief Drawing Officer, that is) if you continue like this :-)

I find it quite reassuring that it's not a regression, somehow I assumed that this did work in 2.8 but seems this was indeed never the case. And your patch does fix (1) entirely, thanks again!

As for the background bitmap offset problem, I think the solution is to take scrolling into account when calling ::SetBrushOrgEx() in wxWindow::MSWGetBgBrushForChild() in src/msw/window.cpp, i.e. either add or subtract GetScrollPos(wxHSCROLL-or-wxVSCROLL) to the rc.left and rc.top respectively.

comment:3 Changed 19 months ago by VZ

(In [73428]) Fix background drawing in EVT_ERASE_BACKGROUND handler in erase sample.

Clear the DC before moving its offset to ensure that it's cleared entirely and
also draw the background using the virtual, not client, size.

See #14917.

comment:4 in reply to: ↑ 2 Changed 19 months ago by ericj

Replying to vadz:

Thanks for looking at this Eric! You risk becoming wxWidgets CDO (Chief Drawing Officer, that is) if you continue like this :-)

If i read wxScrolledWindow and MSW i feel kind of obliged to look into it, as it could be caused by my recent patch :)


As for the background bitmap offset problem, I think the solution is to take scrolling into account when calling ::SetBrushOrgEx() in wxWindow::MSWGetBgBrushForChild() in src/msw/window.cpp, i.e. either add or subtract GetScrollPos(wxHSCROLL-or-wxVSCROLL) to the rc.left and rc.top respectively.

Yes, that works in experimental code. But there is one problem: GetScrollPos() returns the offset in scroll units (maybe the doc for this method should mention it), but the method to retrieve the value, GetScrollPixelsPerUnit() is in wxScrolledWindow/wxScrollHelperBase. For a test, i used a dynamic cast and it worked, but i don't like this very much.

Is there a better solution?

comment:5 Changed 18 months ago by ericj

Trying to fix some more redraw problems, i found that the sample code would need some more refactoring, as the dc.Clear() in MyCanvas::DoPaint() is always called after the PrepareDC(dc) in MyCanvas::OnPaint().

I was wondering if the behavior of wxDC::Clear() should be changed, so that it always clears the visible area, regardless of scrollposition and whether PrepareDC has been called or not. I guess that's what a user of this method would expect.

Does anyone know how this behaves under GTK and/or OSX?

Changed 18 months ago by ericj

patch to msw/window..cpp to fix misalignment of background brush in scrolled windows. Also refactors the "erase" sample a bit more to fix redraw problems

comment:6 Changed 18 months ago by ericj

  • Patch set

I now added the patch to msw/window.cpp using wxDynamicCast to wxScrolledWindow to get the scroll units. I don't like this solution very much, but it works for now.

In the "erase" sample i added a new method ClearBackground() as substitute for dc.Clear(). Otherwise the code would become a little messy if dc.Clear() would always have to be called before PrepareDC().

As far as i can tell, this fixes all redraw problems in the "erase" sample.

comment:7 Changed 18 months ago by vadz

A nicer solution to dynamic casts is, as always, a virtual function, so I'll add one to do this. Thanks again for debugging and fixing this!

comment:8 Changed 18 months ago by VZ

(In [73493]) Erase the entire virtual area of the window in the erase sample.

Just clearing the DC is not enough when the window is scrolled, so clear the
entire virtual area. We should be able to optimize it by clearing just the
rectangle currently scrolled into view but this is at least correct, i.e.
doesn't result in corrupted display, even if it's suboptimal.

See #14917.

comment:9 Changed 18 months ago by VZ

(In [73494]) No changes, just factor out PrepareDC() call in the erase sample.

Call this only once in DoPaint() itself instead of calling it twice before
calling it.

See #14917.

comment:10 Changed 18 months ago by VZ

  • Resolution set to fixed
  • Status changed from confirmed to closed

(In [73495]) Account for scrolling when setting the background brush origin in wxMSW.

We must use physical coordinates for the brush origin to account for the
coordinates offset in scrolled windows, so add MSWAdjustBrushOrg() and call it
from MSWGetBgBrushForChild().

Closes #14917.

Note: See TracTickets for help on using tickets.