GNOME Bugzilla – Bug 144269
GtkHPaned flickers and leaves widgets in child panels unrefreshed
Last modified: 2008-01-31 17:53:10 UTC
in gtk-demo drag the gtkvpaned around. The textview will not redraw correctly. create a window with an hpanel with a textview or a treeview as its left and right children. Drag the handle back and forth and the treeview headers will not always be redrawn correctly and cells in the treeview may not refresh.
I think the flickering is due to painting the background in the window procedure, specifically in gdkevents-win32.c: erase_background. This also causes flicker when a notebook is resized; try the notebook test in testgtk.exe and resize the window the notebook is in. The tabs flicker even when the window is resized so it's larger. Disabling erase_background by returning immediately eliminates the flicker -- but also may breaks background window pixmaps so this probably isn't a fix, but it is a good indication of where the problem is. How is the window background handled by the double buffer for updates? Note that I'm working on a fairly new 1.4 GHz laptop with an average or below average graphics card, so it's not the fastest display system around but it's also not hopelessly out of date.
Actually, it looks like the background is drawn to the double buffer pixmap by gdk_window_paint_init_bg and I don't see problems with erase_backgroud disabled, even when running with a pixmap based thame. Can erase_background simply be removed?
Will try to look into this tonight. It's quite possible that there are leftovers in gdk/win32 from pre-no-flicker-merge days that aren't needed any longer.
Disabling erase_background in gdkevents-win32.c does eliminate the flicker. However, it does not fix the problem with the textview refresing properly. So, there are still parts of the horizontal divider in the textview, but it is an improvement.
interesting if i allow erase_background event to fall through to the wm_paint message then all the gtk windows resize much smoother. case WM_ERASEBKGND: GDK_NOTE (EVENTS, g_print (" %p", (HANDLE) msg->wParam)); if (GDK_WINDOW_DESTROYED (window)) break; erase_background (window, (HDC) msg->wParam); return_val = TRUE; *ret_valp = 1; //break; // repaint case WM_PAINT: handle_wm_paint (msg, window, FALSE, NULL); break;
I think the problem with the textview not being refreshed properly is due to the child window for the pane divider being moved down over the top of the textview child window before the top of the textview is moved down. They momentarily overlap and when the textview child is repositioned, the displayed pixmap from the screen with the divider displayed over the text is copied downward. I would have thought that windows would invalidate this rectangle, but it's apparantly not happening -- perhaps because the paint message has already been generated and processed by the window procedure, so windows thinks the rectangle is up to date.
Just removing any action in WM_ERASEBG events causes GtkCList to break. Clist uses gtk_window_clear_area, which _generates_ a WM_ERASEBG event. A simple fix was to use _gdk_windowing_window_clear_area_e instead of _gdk_windowing_window_clear_area all the time, but I don't know what side effects this could have. Also, I needed to make _gdk_windowing_window_clear_area_e handle width and height of 0 (meaning full area). This change caused no visual regressions in any widgets that I could see. Not calling erase_background() sure makes Gtk[V|H]Paned feel fast!
It's not perfectly clear to me that the current implementation in gdk/win32 of _gdk_windowing_window_clear_area() that uses SendMessage (WM_ERASEBKGND) necessarily is the best one. It might well be that something else would be a better fit. I kinda have a feeling that avoiding SendMessage() might make the code clearner and easier to understand? If the WM_ERASEBKGND handling otherwise is unnecessary, and that SendMessage() is the only thing that really needs it, the code for the WM_ERASEBKGND handled (i.e., erase_background()) could be moved into _gdk_windowing_window_clear_area. (There are only three places in gdk/win32 where SendMessage() is used, and the two others are icon-related and not likely to cause any confusion.)
I don't think SendMessage is needed here; it would only be needed if the win32 default background drawing support was being used. I think the 0 should be returned from the window procedure when processing WM_ERASEBKGND to indicate that the background was not erased and it should be erased by the WM_PAINT handler, which is what gtk essentially does. I've spent some time looking at _gdk_window_move_resize_child. Does anyone understand this code? My thought is that it only needs to call SetWindowPos, but I may be missing something. Are there any problems with 32 bit coordinate values on win 95/98/ME?
> I've spent some time looking at _gdk_window_move_resize_child. > Does anyone understand this code? I think one can safely say, no, at least not any longer... I can remember scratching my hair out trying to figure out how all that hangs together in the X11 backend, and trying to figure out whatever might be correct on Win32... > Are there any problems with 32 bit coordinate > values on win 95/98/ME? Definitely yes. My opinion is that as the current half-hearted attempt to do it like on X11 (i.e., restricting the actual window sizes to 16 bits and use hairy code for windows that are larger from the GDK user's point of view) should be dropped. It doesn't work anyway. Instead just ignore Win9x, and rewrite the code to use unrestricted 32 bit coordinates... Should be much cleaner.
The win9x GDI is 16 bit so obviously there will be problems by assuming 32 bit coordinates. AFAIK even the winnt GDI does not support the full 32 bit range so 'unrestricted 32 bit coordinates' won't work there either. But the winnt range should be enough for real world apps anyway. [Try testgtk::big windows on NT and test if you are patient enough to wait for the window to pop up ;-] Given that both windoze plattforms need *some* coordinates restriction IMO the way t solve the issue is to remove all the 16->32-bit emulation code as Tor suggest, but add some dynamic, plattform dependent clipping to at least get reasonable error handling. Also the G_MAXINT usage related to big windows needs to be reviewed carefuly, i.e. replaced with the platform dependent limit. Should probably have a new bug report to track the issue ...
Created attachment 29253 [details] [review] Remove ERASEBKGND background redraw This fixes the flickering problem by removing the code that redraws the bg in response to the WM_ERASEBKGND message. It implements _gdk_windowing_window_clear_area via a begin_paint / end_paint pair, which trigger the code used to draw the background on the double buffer pixmap and then to the window. I used the CTree test in testgtk to check that it works with the old CList / CTree code. _gdk_windowing_window_clear_area_e is also changed to calculate height & width if the values passed in are 0 because the x11 implementation does this via the XClearArea call.
if the WM_ERASEBKGND message is allowed to fall through to the WM_PAINT message the window will resize smoothly as well.
I'm leery of doing a full paint when Windows is only asking for the background to be repainted. It does appear that windows is sending multiple WM_ERASEBKGND for every WM_PAINT, at least according to spy++ on XP.
I assume you have tested that background pixmaps, and parent relative backgrounds, still work? (For instance the marble background in testgtk's text test.)
I just tested the marble background in the text test and Wing with a pixmap theme. I'm reasonably sure the code called via begin_paint works because it's used to draw the background to the double buffer pixmap.
What's the significance of *ret_valp = 0; in WM_ERASEBKGND?
MSDN says that 0 should be returned from WM_ERASEBKGND if the background was not drawn, which will then cause the fErase member of PAINTSTRUCT to be true to indicate the app needs to draw the background.
The issue I see now with the patch applied is that there's a short, but percepitable time between when the window frame is drawn and the contents are drawn; during this time the window contains the previous contents of the screen. This is noticable for me when restoring a minimized window containing many child gdk windows such as the windows for gtk buttons. I think this is due to redrawing in the idle procedure. I'm experimenting with calling gdk_process_window_updates directly, which is an improvement but drawing is still not quite as smooth as a native application. Also, it looks like in handle_wm_expose, the hrgn region handle is never destroyed if return_exposes is true.
Maybe WM_ERASEBKGND could be treated like a WM_PAINT, but only when it is generated by a top-level window resizing?
Created attachment 29362 [details] [review] Paint windows directly from handle_wm_paint Here's a patch that reduces the delay between window presentation and drawing by calling gdk_window_process_updates from handle_wm_paint if it is called from the top level mainloop (that is, not in the middle of an event dispatch). This improves performance quite a bit. I've tried calling handle_wm_paint on a WM_ERASEBKGND message and it improves responsiveness somewhat, but I'm still a bit leery of painting in response to it. It seems that windows will dispatch WM_ERASEBKGND to all windows that need it before dispatching WM_PAINT to any of them. There is a problem with updates that is brought out by this patch -- if you move another window partially over a gtk window (the main gtktest window will do just fine) fast enough, moving it up, down and side to side to expose different parts of the gtk window, some parts of the gtk window are never updated. This also occurs with an unmodified build of the 2.4 branch, though it is harder to provoke. It seems to be related to cpu load because it's easier to provoke if I lower the speed that my laptop is running at.
John, I tested your patch. You need to check for negative width/height inside _gdk_windowing_window_clear_area, after: if (width == 0) width = impl->width - x; if (height == 0) height = impl->height - y; GtkCList generates negatives, causing gdk_window_end_paint to crash.
The drawing does feel snapper but it does not fix the issue with the GtkHPaned leaving parts of itself in the gtktextview. It also, i noticed breaks the TreeStore test. When you initially open the treestore test the tree is not drawn correctly.
Created attachment 34312 [details] [review] Remove ERASEBKGND background redraw unless a top level window is being resized interactively The attached patch is an alternative to (and includes part of) Patch 29253 above that maintains the original erase functionality when a top-level window is being resized interactively. It's nasty, in that it uses _sizemove_in_progress when it probably shouldn't, but it gives better results than the other suggestions, including falling through into WM_PAINT.
Created attachment 34313 [details] Test case (c file and glade file) With the erase background filcker removed, the attached test case shows up another flicker problem with GtkHPaned. Grab the pane and move it back and forth quickly. The buttons either side will flicker in to and out of prelight state. The modified colours show this better than the standard style, but it's still visible nomally. The GtkHPaned is doing a pointer grab, but for some reason it's not working correctly.
I am able to replicate this issue on my win32 environment. I am using Gtk+2.4.14 on WinXP. I work with the gtk2-perl bindings.
Any chance of applying one of the patches or are we waiting for someone to come up with a better way to solve this?
Hmm, so would it be Tim Evans's patch in comment #25 that should be applied? Are we sure now that it doesn't have any ill effects? I.e. has it been tested (when applied to relatively fresh GTK+ 2.6 code) with several different applications?
I've been using the patch attached to comment #12 (with the fix to support CList/CTree) successfully for a while in Wing and have recently updated to 2.6.7, but I have not tested with other applications. I would be happy to update it or to see Tim Evan's patch go in because that would be a step forward. Maybe it should go into the 2.7.x series so it sees testing before being in a production release.
Created attachment 65612 [details] [review] Updates the patch given by Tim Evans in Comment 24 to current CVS (16 May 2006) This bug still exists. The attached patch, which is simply an update of the patch given by Tim Evans in Comment 24 (20 Nov 2004) to patch cleanly into current CVS (16 May 2006), removes the flickering problem mentioned above and in Bug #331599. Is there a reason this patch has not gone in? I am sorry I don't know enough about the GDK internals to offer an opinion but it seems to make sense and it works. If there is no reason against it, it would be wonderful if this patch could go in to current CVS.
*** Bug 331599 has been marked as a duplicate of this bug. ***
(In reply to comment #30) > Created an attachment (id=65612) [edit] > Updates the patch given by Tim Evans in Comment 24 to current CVS (16 May 2006) > > This bug still exists. The attached patch, which is simply an update of the > patch given by Tim Evans in Comment 24 (20 Nov 2004) to patch cleanly into > current CVS (16 May 2006), removes the flickering problem mentioned above and > in Bug #331599. > > Is there a reason this patch has not gone in? I am sorry I don't know enough > about the GDK internals to offer an opinion but it seems to make sense and it > works. If there is no reason against it, it would be wonderful if this patch > could go in to current CVS. > If I recall correctly, the patch causes some regressions in GtkTextView... although those may have been fixed since 2.4/2.6 ...
First patch doesn't apply cleanly. Reviewing it would take a little while. Second patch doesn't apply cleanly. Reviewing it would take little time. (Working on http://mail.gnome.org/archives/gtk-devel-list/2007-March/msg00148.html)
GDK handles this in a different way than it used to. The current implementation in SVN fixes this bug, as far as I can tell, and the above patch is no longer necessary (nor can it really be applied due to the implementation changes). I think this bug should be closed, can the original bug-submitters confirm this is fixed in SVN?
(In reply to comment #34) > GDK handles this in a different way than it used to. The current > implementation in SVN fixes this bug, as far as I can tell, and the above patch > is no longer necessary (nor can it really be applied due to the implementation > changes). > > I think this bug should be closed, can the original bug-submitters confirm this > is fixed in SVN? > I don't have an environment setup to confirm the issue is fixed.
It is not fixed.
*** Bug 322385 has been marked as a duplicate of this bug. ***
*** Bug 302636 has been marked as a duplicate of this bug. ***
*** Bug 125597 has been marked as a duplicate of this bug. ***
*** Bug 361208 has been marked as a duplicate of this bug. ***
I can also confirm that this bug is not yet fixed as of GTK 2.12.1. The project I code for, Wireshark, exhibits this problem pretty seriously when dragging the pane dividers rapidly up and down. Please see my screenshots on duplicate bug #361208. This issue is also being tracked as Wireshark bug #958 (http://bugs.wireshark.org/bugzilla/show_bug.cgi?id=958).
Yeah, this is (to my knowledge) the last really serious drawing-related Win32 bug in gtk+, but unfortunately it's pretty difficult to debug and nobody seems to have a good idea of what's causing it yet. But it is definitely a known bug right now, and we can reproduce it from within the gtk-demo test suite.
Created attachment 99918 [details] [review] Proposed solution Sorry, this is really hacky. But it works. I've wasted too much time trying to find the correct solution, this is the best I can do I think.
2007-12-1 Cody Russell <bratsche@gnome.org> * gtk/gtkpaned.c: (gtk_paned_set_position) [Win32]: On Windows, queue a redraw of child2 whenever we set the pane handle position. This is unfortunately kind of hacky, but solves the visual artifacts that were occuring on at least certain types of child widgets (e.g., text views and tree views) that are inside horizontal or vertical panes. (#144269)
Also merged into gtk-2-12 branch.
Dear developers, could you reopen and fix this issue again, please? You see, there is problem that gtk_panen_set_position() function could be called before any child packed into paned widget. So, as solution you may just check if (paned->child2 != NULL) before calling gtk_widget_queue_resize() and get NULL assertion. Regards, -andrew Index: gtk/gtkpaned.c =================================================================== --- gtk/gtkpaned.c (revision 19041) +++ gtk/gtkpaned.c (working copy) @@ -1232,6 +1232,11 @@ g_object_thaw_notify (object); gtk_widget_queue_resize (GTK_WIDGET (paned)); + +#ifdef G_OS_WIN32 + /* Hacky work-around for bug #144269 */ ++ ++ if (paned->child2 != NULL) ++ + gtk_widget_queue_resize (paned->child2); +#endif } /**
Thanks Andrew. Incidentally, I also noticed recently that this fix doesn't seem to be working anymore. I'm not sure how I ended up with queue_resize() instead of queue_draw(), but when I test it now queue_draw() definitely seems to fix the bug and queue_resize() definitely does not. So I'm committing to trunk and gtk-2-12 branch. 2008-01-31 Cody Russell <bratsche@gnome.org> * gtk/gtkpaned.c (gtk_paned_set_position): Change queue_resize() to queue_draw(), and add a check for child2 != NULL in case someone calls this before there is a child packed in there. (#144269 again)