Bug 4656 - Massive overdraw when scrolling in text editor
Summary: Massive overdraw when scrolling in text editor
Status: RESOLVED FIXED
Alias: None
Product: Xamarin Studio
Classification: Desktop
Component: Text Editor ()
Version: Trunk
Hardware: PC Mac OS
: Low normal
Target Milestone: ---
Assignee: Mike Krüger
URL:
Depends on:
Blocks:
 
Reported: 2012-04-25 19:32 UTC by Mikayla Hutchinson [MSFT]
Modified: 2012-10-11 02:52 UTC (History)
3 users (show)

Tags: gtk
Is this bug a regression?: ---
Last known good build:


Attachments
Video showing screen updates (7.68 MB, video/mpeg)
2012-04-26 01:44 UTC, Mike Krüger
Details
Make gtk_window_paint use the region when available (1.19 KB, patch)
2012-04-30 11:19 UTC, Kristian Rietveld (inactive)
Details
Updated patch with the mentioned fixes (1003 bytes, patch)
2012-05-11 03:08 UTC, Michael Natterer
Details


Notice (2018-05-24): bugzilla.xamarin.com is now in read-only mode.

Please join us on Visual Studio Developer Community and in the Xamarin and Mono organizations on GitHub to continue tracking issues. Bugzilla will remain available for reference in read-only mode. We will continue to work on open Bugzilla bugs, copy them to the new locations as needed for follow-up, and add the new items under Related Links.

Our sincere thanks to everyone who has contributed on this bug tracker over the years. Thanks also for your understanding as we make these adjustments and improvements for the future.


Please create a new report on Developer Community or GitHub with your current version information, steps to reproduce, and relevant error messages or log files if you are hitting an issue that looks similar to this resolved bug and you do not yet see a matching new report.

Related Links:
Status:
RESOLVED FIXED

Description Mikayla Hutchinson [MSFT] 2012-04-25 19:32:58 UTC
The text editor is doing an awful lot more drawing when scrolling than it need to do. This makes it feel quite laggy with smooth scrolling.

For a demonstration, enable Quartz Debug screen update highlighting, then go to Preferences->Text Editor->Code Templates and select the Exception template. We're looking at this because it uses a standard ScrolledWindow, so eliminates SmartScrolledWindow from the equation.

When scrolling the text editor based preview, the entire lower half of the window is updated. When scrolling the treeviews, the scrolling is blitted efficiently.
Comment 1 Mikayla Hutchinson [MSFT] 2012-04-25 19:34:39 UTC
Note - i was trying to debug this, and it looked like the FoldMarkerMargin was doing a lot of unnecessary invalidation, which got enlarged the expose region a lot, but it didn't seem to be the primary cause.
Comment 2 Mike Krüger 2012-04-26 01:17:12 UTC
Works perfectly on linux / gtk parasite.
Comment 3 Mike Krüger 2012-04-26 01:44:56 UTC
Created attachment 1748 [details]
Video showing screen updates

Video is attached.
Comment 4 Mikayla Hutchinson [MSFT] 2012-04-26 13:59:00 UTC
This looks like a GTK bug then?
Comment 5 Mikayla Hutchinson [MSFT] 2012-04-26 17:02:56 UTC
Ok, this doesn't appear to affect the other GTK+ widgets (TextView, ScrollView) because they all call gdk_window_process_updates in their scroll handlers. When I add that call to Mono.TextEditor, the rendering updates shown by Quartz Debug look similar to other GTK+ widgets - it's just updating the areas that could not be blitted by gdk_window_scroll.

Interestingly, a recent commit to GTK+ removes the gdk_window_process_updates call from scroll handlers because it causes perf problems with smooth scrolling: http://git.gnome.org/browse/gtk+/commit/?id=917ca6a802af574232f413fdf904e1633d706b52 Should we be patching our GTK+ for this too, or is it a fix for a Linux or GTK3 specific problem? I had a quick shot at doing so and did notice some minor scrolling artifacts in TreeView, so it's probably not trivial to do so.

Something strange is going on in GTK+, it seems to be drawing a lot more to the screen than it needs to, although it's not actually exposing it. Perhaps some size/clipping issue in the quartz backend's buffering, or a timing issue in Quartz Debug showing multiple updates?

* When I remove the gdk_window_process_update from GtkTreeview, it has the same complete apparent re-draw issue as Mono.TextEditor.
* The actual area of the expose event in Mono.TextEditor matches the areas we expect to be invalidated by the gdk_window_scroll, even though Quartz Debug makes it look like the whole window is being redrawn.
* When I scroll a widget without the gdk_window_process_updates call in the prefs dialog, it appears to redraw most of the dialog
Comment 6 Kristian Rietveld (inactive) 2012-04-29 19:55:56 UTC
> Interestingly, a recent commit to GTK+ removes the gdk_window_process_updates
> call from scroll handlers because it causes perf problems with smooth
> scrolling:
> http://git.gnome.org/browse/gtk+/commit/?id=917ca6a802af574232f413fdf904e1633d706b52
> Should we be patching our GTK+ for this too, or is it a fix for a Linux or GTK3
> specific problem? I had a quick shot at doing so and did notice some minor
> scrolling artifacts in TreeView, so it's probably not trivial to do so.

I would recommend against patching GTK+ 2.x, I think this could only have been done since it is GTK+3 and quite some of the drawing/clipping code changed. For GTK+ 2.x, I do know that GtkTreeView needed the gdk_window_process_updates() call in order to render properly.  This was mentioned in the code with a comment.

For GTK+ 3.x they simply removed this comment, I hope this means they also tested it ...
Comment 7 Kristian Rietveld (inactive) 2012-04-29 20:00:52 UTC
If I test with GDK debug updates enabled, then only the parts you would expect to update are marked in red by GDK.  For some reason this also seems to decrease the amount of "half of the window" updates indicated by Quartz Debug.

On the other hand, if I simply hover the mouse over the add/edit/remove buttons, Quartz Debug indicates screen updates ranging from the button to the bottom of the screen.  Similarly if I move over the vertical scroll bar of the text editor.  From this, I get the impression that this is not a Text editor problem but rather a GTK+ problem.

I can reproduce likewise behavior in the "button boxes" demo in gtk-demo. Quartz shows updates from the button that is hovered to the bottom of the window. GDK only indicates that the button itself is being repainted.  Something is wrong here.
Comment 8 Kristian Rietveld (inactive) 2012-04-30 07:59:52 UTC
I investigated some more using the button boxes demo.  I dumped the coordinates of all rects that were requested to be drawn through GdkQuartzView's drawRect: method.  It turns out that there always was an additional rectangle being drawn that I could not explain.

It turns out this additional rectangle originates from the call to displayIfNeeded in _gdk_windowing_after_process_all_updates() in gdkwindow-quartz.c.  displayIfNeeded conditionally calls setNeedsDisplayInRect internally.

If the call to displayIfNeeded is removed, the overdraw disappears in the button boxes demo.  Similarly, if I change the call to displayIfNeededIgnoringOpacity, the overdraw disappears as well.  GdkQuartzView IsOpaque: seems to usually return YES; I guess this makes sense since GTK+ applications usually draw a solid background. However, it needs to be clear that this additional rectangle is added by Cocoa and not by GDK (otherwise it would have been visible using GDK debug updates).

In further investigation, the overdraw appears to occur with NO_WINDOW widgets, but not with WINDOW widgets. This kind of makes sense to me, since NO_WINDOW widgets need their parent to draw their background. An inherent danger here is that I am mixing up WINDOW/NO_WINDOW versus opaque/not-opaque; we do have a hierarchy of GdkWindows but not a hierarchy of NSViews.

The final thing that I do not understand yet is that Mono.TextEditor is a WINDOW widget, so we cannot yet explain why overdraw is happening here.  I need to get myself a local build of MonoDevelop to further investigate this and to see if disabling displayIfNeeded removes the overdraw in MonoDevelop.  If so, I need to discuss with Mitch if it is theoretically seen possible to change to e.g. displayIfNeededIgnoringOpacity (could this for instance be plausible because GDK should handle opaque background drawing already?).
Comment 9 Kristian Rietveld (inactive) 2012-04-30 10:52:00 UTC
More analysis:

 1. When I disable the vertical scrollbar in the Code Template Preferences Window, the massive overdraw on vertical scrolling (you will have to scroll with the keyboard) is gone.  This gives a pointer to the fact that the scrollbar redraw might be the responsible for this behavior.

 2. In comment 8 I mentioned the additional rectangles being drawn.  These were rectangles at the bottom of the window, along the entire width and a 4 pixel height.  It turns out that this draw is needed to update the rounded corners at the bottom of the window.

 3. When one replaces displayIfNeeded with displayIfNeededIgnoringOpacity, then the rounded corners at the bottom are no longer drawn properly.

 4. testtext does not show the massive overdraw behavior, even if the vertical scrollbar is visible. The "Application Window" demo in gtk-demo does as soon as the vertical scroll bar is active.

 5. Even with displayIfNeededIgnoringOpacity, 'Application Window' seems to show the massive redraw behavior.


From 2 and 3 we now know it is very likely displayIfNeeded is functioning properly, by adding a small strip at the bottom of the window as additional rectangle.  This rectangle is reported separately in the drawRect: method of GdkQuartzView.

The remaining question whether Cocoa is doing some kind of optimization by deciding to redraw a full region instead of separate regions even though the separate regions are reported through getRectsBeingDrawn: (this would not be surprising to me, because in the past I have seen cases where drawing 1 large area was faster than drawing several (say > 14) small ones) or whether GDK is drawing outside these regions (in that case, Cocoa would only clip the bounding box of the separate rects).
Comment 10 Kristian Rietveld (inactive) 2012-04-30 11:19:56 UTC
Created attachment 1779 [details]
Make gtk_window_paint use the region when available

GTK+ is at fault, GtkWindow is using event->area to redraw the window background even when event->region is available.  This patch removes the cases of overdrawing I have seen in the various GTK+ demos.  I cannot test with MonoDevelop at the moment because I don't have a 32-bit build of the GTK+ stack at hand, but I am confident it is exactly the same issue.

I have no real idea if this patch is upstreamable, I need to discuss that with Mitch.
Comment 11 Michael Natterer 2012-05-10 05:28:35 UTC
This looks absolutely right in order to avoid excessive drawing, but
the patch leaks the rectangles returned by gdk_region_get_rectangles(),
and a GdkEventExpose always has a region, so the if() is not needed.

We use the same method on the GIMP canvas to reduce drawing to what
has actually been exposed.
Comment 12 Mikayla Hutchinson [MSFT] 2012-05-10 17:35:27 UTC
Could you please update the patch with those changes, then I'll test it with MD?
Comment 13 Michael Natterer 2012-05-11 03:08:41 UTC
Created attachment 1858 [details]
Updated patch with the mentioned fixes
Comment 14 Michael Natterer 2012-05-11 03:27:12 UTC
As a nice side effect, this patch also seems to significantly reduce
overdrawing artifacts with the kinetic scrolling patch :)
Comment 15 Mikayla Hutchinson [MSFT] 2012-05-11 16:38:01 UTC
That's a lot better, especially the MD preferences dialog. I've applied it to bockbuild.

I'm still not sure why Quartz Debug shows updates for the exposed areas of builtin GTK widgets but the exposed *and* scroll-blitted areas of the MD text editor. The this might just be an artifact of the way it delays/displays updates when gdk_window_process_updates is used. Scrolling in Cocoa apps show whole widgets updating, and perf seems fine on MD, both of which support this theory. So I'm not really concerned about the text editor redraw.

The clipping fixes are important more because they fix actual redraw artifacts. For example, this also fixes an old redraw bug in the source editor's search widgets that can be seen in http://screencast.com/t/iXACJLSJvY0q. Note that MD has a workaround for that issue, packing the widgets into visible eventboxes - I disabled it for the demonstration. The workaround is imperfect as it makes the background color slightly wrong in some states.

However, I am still seeing a little bit of redraw leak along the bottom and bottom corners of the window. It looks like it's the resize areas.
Comment 16 Kristian Rietveld (inactive) 2012-05-13 09:16:03 UTC
> However, I am still seeing a little bit of redraw leak along the bottom and
> bottom corners of the window. It looks like it's the resize areas.

I am not sure this is a leak; in comment 9 I mentioned that these redraws appear to be there to update the rounded corners at the left and right bottom corners. When displayIfNeededIgnoringOpacity is used instead, these corners are not redrawn.
Comment 17 Mikayla Hutchinson [MSFT] 2012-05-14 12:09:23 UTC
Ah, I didn't see that, sorry. Well, it's not a big deal.
Comment 18 Mike Krüger 2012-10-11 02:00:30 UTC
I guess that can be closed ?
Comment 19 Michael Natterer 2012-10-11 02:52:29 UTC
If you don't see the issue any longer, it's because of the attached patch,
so yes.