GNOME Bugzilla – Bug 707244
pixelcache: gtktextview pixelcache does not invalidate textmarks in cached area
Last modified: 2013-09-09 08:49:38 UTC
We need to fix the cache invalidation when marks are updated or changed in an area that is already cached. A screenshot showing issue in gtksourceview is attached.
Created attachment 253781 [details] corrupted output
Created attachment 253784 [details] [review] textview: ensure non-visible but pixelcached regions are invalidated. In some cases, if an area is invalidated but is already pixelcached, it will render existing content to the widget. This ensures those regions are invalidated from the pixelcache.
Created attachment 253801 [details] [review] textview: drop pixelcache when textlayout is invalidated. It is difficult to determine everywhere within the pixelcache that content is invalidated, so just drop the entire thing for now.
Review of attachment 253801 [details] [review]: It seems like all this manual work shouldn't be needed due to the invalidate_handler. It should get all the changes that we previously sent to gdk, which should be enough to redraw correctly. The only thing we need to fix is places where the code is manually trying to figure out what is visible via gtk_text_view_get_visible_rect(). I would propose instead creating a gtk_text_view_get_rendered_rect(), which gets the visible rect + whatever extra area that the pixel cache has, and using that in changed_handler instead. ::: gtk/gtktextview.c @@ +4034,3 @@ + * pixelcache is to improve scrolling, this is not a huge loss. + */ + _gtk_pixel_cache_invalidate (priv->pixel_cache, NULL); Is it really that hard? I don't see why not. In fact, by just deleting gdk_rectangle_intersect() check below, would this not just work? @@ +9175,3 @@ + _gtk_pixel_cache_invalidate (priv->pixel_cache, region); + cairo_region_destroy (region); + } I don't see why this is needed, the gdk_window_invalidate_rect() call below should do this too via the text_window_invalidate_handler(). Is this not happening?
Created attachment 254320 [details] [review] textview: add gtk_text_view_get_rendered_rect() and use it for invalidation. (In reply to comment #4) > I would propose instead creating a gtk_text_view_get_rendered_rect(), which > gets the visible > rect + whatever extra area that the pixel cache has, and using that in > changed_handler instead. I think this greatly simplifies things. I've changed it to do so and things seem to be working well. The new areas rendered also stay rather small. This patch also stops using RGBA content and renders the background to the pixelcache.
Review of attachment 254320 [details] [review]: ::: gtk/gtktextview.c @@ +9207,3 @@ + cairo_region_destroy (region); + } + Everything looks right to me except this part, which seems to unnecessary, as the gdk_window_invalidate_rect() in text_window_invalidate_rect() should result in a call to text_window_invalidate_handler() which invalidates the rect. Is that not happening?
Created attachment 254361 [details] [review] textview: add gtk_text_view_get_rendered_rect() and use it for invalidation. You are correct, that was leftover from the previous patch and is no longer necessary since we are invalidating properly by checking the rendered rect.
Created attachment 254363 [details] [review] textview: use pixelcache rendered area to inform invalidation region. Actually, it turns out it is necessary. I ran a heavier test and the areas were not getting cleared correctly. Reverting to the previous patch as attachment. I find this most evident in gedit while view .patch's.
Oh, i see why this happens. The invalidate handler in set on on GtkTextWindow->window, and text_window_invalidate_rect() invalidates on GtkTextWindow->bin_window, which clips the region to the visible area when it propagates to the child window. I think this also explains the problem i've had with understanding why you need to special case the text view content. You should be using the bin window when drawing the pixel cache, not the child window in it. So the invalidate handler should be on that, and _gtk_pixel_cache_draw should also get it. That way the pixel cache should automatically pick up the bg from the bin window and know whether it needs an alpha or not.
Created attachment 254449 [details] [review] textview: use pixelcache rendered area to inform invalidation region. Use the pixelcache rendered area to inform what part of the cache should be invalidated upon changes to the underlying textlayout. By rendering the background to the pixelcache, we can avoid the need to use RGBA content. Also, we're using the pixel cache on the text windows bin_window (see gtk_text_view_get_window) so we need to register the invalidation handler on that, otherwise the region passed to the invalidate handler will get clipped to the visible region.
Attachment 254363 [details] pushed as 9a45712 - textview: use pixelcache rendered area to inform invalidation region. Attachment 254449 [details] pushed as 9a45712 - textview: use pixelcache rendered area to inform invalidation region.