GNOME Bugzilla – Bug 165246
gtk_tree_view_set_cursor does not always do the scroll right
Last modified: 2011-02-04 16:18:46 UTC
Calling gtk_tree_view_set_cursor to select the first row of a treeview just after its contents have been modified, does not always end up in the selected row being actually visible. Doing the _set_cursor after the treeview contents have been modified in an idle works around the problem. My impression is that gtk_tree_view_scroll_to_cell does not always get the right coordinates, perhaps because the rows are not yet validated? I don't know much about treeview internals, just guessing .. I have spent the past few hours trying to come up with a standalone testcase for this, however I cannot get it reproduced. It is however 100% reproducible using the Muine (muine CVS module) music player.. just load some music into it, press 'play album', and type some things, press backspace, type some more.. at some point it will come up. Only reproducable using CVS, I believe the latest release still has the workarounds in place. My eternal gratitude goes out to whoever fixes this, it is a really annoying bug.
Created attachment 36528 [details] Testcase Managed to come up with a test case after all. Not very small, but it is now at least consistently reproducable: press 'f' in the entry, and a second later 'backspace'. The first row should be selected, but it is not.
Using latest gtk-2-6 CVS, Muine even crashes here. The testcase doesn't, however. What follows is a trace from the crash: Program received signal SIGSEGV, Segmentation fault.
+ Trace 54942
Thread NaN (LWP 23340)
Created attachment 37093 [details] Another testcase I've just meet similar problems. Here is my testcase. What is going there: 1. Tree view is realized if it's added to visible container but is still not shown. 2. After addition of rows presize_handler shedule gtk_tree_view_size_request_columns and it updates header height of tree view 3. scroll to cell function using wrong header height calculates visible area of tree view with height less then zero - {0, 0, 1, -27} 4. After that it's scrolls incorrectly. Now we should somehow fix it :)
Same problem here (CVS version): http://glsof.sourceforge.net 'Search' shows a "two rows scrolldown". You can see it on 'single or double list' choice. Problem is for 'gtk_tree_view_scroll_to_cell' too.
Created attachment 37309 [details] [review] Patch to CVS Definitely it's two different problems. One is that scroll tries to find cell on widget that is really hidded and not allocated properly. Second is about race condition with scrolling to scroll_row and scrolling to top_row. But this small patch fixes both.
Rock! I can confirm that this patch fixes both of the problems.
Actually, it doesn't fix the crash- the crash just occurs less frequently now.
Well, probably there remain some problems, but it needs some debugging. Two questions: 1. Is it reproduced with release of gtk like 2.6.1? 2. Could you provide additional information, at least values of parametrs of gdk_window_move? This it too complicated bug - three problem at one report :)
1. Actually, yes- all gtk 2.6 releases. 2. The crash happens in gdk_window_move (the same trace is still valid), parameters are (0, 0) which is right. Running a GDK_IS_WINDOW assert on tree_view->priv->bin_window right before the gdk_window_move doesn't complain. I'd be happy to provide any more needed info.
It's interesing but it's not reproduced on some machines but is reproduced on others. The most strange that I see it only on Redhat 9 with standard XFree86. What is your XFree version?
This is FC3, xorg 6.8.1-12.FC3.21
Doing some debugging, this is pretty strange: The crash happens while calling gdk_window_move, but: - All the parameters passed to the function seem correct, or at least don't cause a crash when accessing them separately right before the call. - gdk_window_move itself is not actually entered Smells like some kind of memory corruption?
More information: The gtk_adjustment_changed () where it crashes is not actually the one caused by scroll_to_point (), instead it is caused by the gtk_adjustment_set_value () in validate_visible_area (), which in turn is called by bin_expose (). It is odd that it wants to scroll, as the row is perfectly visible. Just before the call to _scroll_to_cell (), the relevant row is changed, and thus invalidated. Perhaps there is another race here?
I think that it's normal situation that adjustment changes several times. Anyhow, it should not crash whatever race condition is there. Unfortunately, I still can't reproduce crash with my version of muine 0.8.2 and gtk 2.6.1. Probably, another testcase is possible?
The way I can reproduce it, using Gtk+ CVS, 2.6 branch, with your patch and Muine CVS, is by queueing up a few albums and songs and pressing "next" a couple of times.
Another consistent way to reproduce it here is by trying to scroll any of the treeviews using the arrow keys.
Further digging reveals that the bug only occurs when both a) Using Gtk# and b) Gtk+ is compiled with -march=pentium4 So I guess this is not really a Gtk issue. In this case, Nickolay's patch fixes all relevant issues.
Moving to 2.8, as Kris thinks his patch is risky
Jorn's problem has been fixed with the fix in #163214. The problem Nickolay's testcase describes is caused by this piece of code in gtk_tree_view_scroll_to_cell: if (cell_rect.y + cell_rect.height > vis_rect.y + vis_rect.height) dest_y = cell_rect.y + cell_rect.height - vis_rect.height; with the problem being that this code does not work correctly when the treeview has not been allocated yet. (vis_rect.height does not have a usable value at this time). A fix which might be correct is probably not doing this correction when vis_rect.height does not have a usable value. But this still gives us the problem on how to decide whether vis_rect.height is usable or not. I need to sleep about this before I can make a final decision.
Thanks for review, Kris - this part of patch should do exactly what you need - checks if vis_rect.height has usable value. +#include "gtkprivate.h" #define GTK_TREE_VIEW_PRIORITY_VALIDATE (GDK_PRIORITY_REDRAW + 5) #define GTK_TREE_VIEW_PRIORITY_SCROLL_SYNC (GTK_TREE_VIEW_PRIORITY_VALIDATE + 2) @@ -10195,12 +10196,20 @@ gtk_tree_view_scroll_to_cell (GtkTreeVie * scrolling code, we short-circuit validate_visible_area's immplementation as * it is much slower than just going to the point. */ - if (! GTK_WIDGET_REALIZED (tree_view) || + if (!GTK_WIDGET_VISIBLE (tree_view) || GTK_WIDGET_ALLOC_NEEDED (tree_view) || GTK_RBNODE_FLAG_SET (tree_view->priv->tree->root, GTK_RBNODE_DESCENDANTS_INVALID))
Okay, I gave it some more thought. It's indeed better to have the scrolling handled via validate_visible_area() if the treeview is not visible or needs reallocation. Committed your patch to HEAD (together with another fix, which makes your testcase fully work). Thanks!