Opened 17 months ago

Closed 17 months ago

Last modified 17 months ago

#14852 closed defect (fixed)

wxListCtrl not redrawn correctly when scrolling

Reported by: trivia21 Owned by:
Priority: normal Milestone: 2.9.5
Component: wxGTK Version: stable-latest
Keywords: regression Cc: ericj
Blocked By: Blocking:
Patch: no

Description

wxListCtrl handles wxEVT_SCROLLWIN_LINEUP line up (and down) events incorrectly. It scrolls two lines instead of one, and the second line is only partly or not at all redrawn.
The bug was introduced in revision 72939 by VZ. I soved the problem by changing line 206 of trunk/src/generic/scrlwing.cpp back to "if ( processed )" form "if ( processed && event.IsCommandEvent())". Though I'm not aware of all the consequences of this change.
I suspect this problem (at least the scrolling two lines part) is somehow related to wxListCtrl sending duplicate events (see http://forums.wxwidgets.org/viewtopic.php?f=1&t=36358).

Change History (9)

comment:1 Changed 17 months ago by trivia21

I'm creating it with style = wxLC_REPORT.

comment:2 Changed 17 months ago by vadz

  • Cc ericj added
  • Keywords regression added
  • Status changed from new to confirmed

This is indeed visible in the listctrl sample with virtual list control, thanks for reporting.

Right now I don't see what is exactly the problem here (but it's certainly unrelated to key down events as the bug happens if you scroll with the mouse too). Eric, would you have any ideas?

comment:3 Changed 17 months ago by ericj

I can have a look if i can see it under Windows. Is there an easy way to make the MSW build use the generic list ctrl?

comment:4 follow-up: Changed 17 months ago by ericj

Without being able to reproduce the problem, i just took a look at the source. The problem is most likely that wxGenericListCtrl::OnScroll() calls HandleOnScroll(), and then in wxScrollHelperEvtHandler::ProcessEvent() it gets called again.

Maybe we would need something like this:

if ( processed && ( event.IsCommandEvent() || event.IsScrollEvent() ) )
        return true;

But it looks a little hackish, maybe the code in wxScrollHelperEvtHandler::ProcessEvent() needs some cleanup here.

comment:5 in reply to: ↑ 4 ; follow-up: Changed 17 months ago by trivia21

wxScrollEvent derives from wxCommandEvent.

comment:6 in reply to: ↑ 5 Changed 17 months ago by ericj

Replying to trivia21:

wxScrollEvent derives from wxCommandEvent.

Yes. But not wxScrollWinEvent which is the one that matters here. So the naming of the imaginary method IsScrollEvent() is not perfect.

comment:7 Changed 17 months ago by vadz

I think we should remove event.IsCommandEvent() test entirely and only continue handling the events that we must really handle even if the user code already did handle them, e.g. wxEVT_SIZE or wxEVT_ENTER_WINDOW. But wxEVT_SCROLLWIN_XXX are probably not in this category -- if the user code decided to handle them itself, we shouldn't do anything extra.

But while from this point the current wxGenericListCtrl code is correct and the code in wxScrollHelperEvtHandler is not, let me change the former to make it work again for now and avoid touching the latter in case I'm missing something again here...

comment:8 Changed 17 months ago by VZ

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

(In [73019]) Fix wxGenericListCtrl scrolling by not processing the events twice.

Scrolling wxGenericListCtrl was broken since r72939 because its OnScroll()
event handler explicitly called wxScrollHelper::HandleOnScroll() which was
also called by the base class ProcessEvent().

Arguably, wxScrollHelper::ProcessEvent() should be updated to allow handling
events directly like this by not processing it again if the event wasn't
skipped but for now just do skip the event and let the default handling take
place which at least makes wxGenericListCtrl work correctly again.

Closes #14852.

comment:9 Changed 17 months ago by trivia21

Thanks. It works now without changing anything.

Note: See TracTickets for help on using tickets.