Bug 4366 - On Wayland, selecting a message list row does not immediately highlight the row
Summary: On Wayland, selecting a message list row does not immediately highlight the row
Status: RESOLVED FIXED
Alias: None
Product: Claws Mail
Classification: Unclassified
Component: wayland (show other bugs)
Version: 3.99.0
Hardware: PC Linux
: P3 minor
Assignee: users
URL:
Depends on:
Blocks:
 
Reported: 2020-07-18 20:30 CEST by M
Modified: 2021-04-14 14:05 CEST (History)
3 users (show)

See Also:


Attachments
Patch that calls gtk_widget_queue_draw on input handling (2.72 KB, patch)
2020-12-05 02:00 CET, M
Details | Diff
Patch that calls gtk_widget_queue_draw from the gtkcmctree object itself (2.02 KB, patch)
2020-12-09 18:28 CET, M
Details | Diff
Patch that treats draw_X requests as queue_draw_X if not inside widget draw event (6.76 KB, patch)
2020-12-12 05:40 CET, M
Details | Diff

Description M 2020-07-18 20:30:32 CEST
If you run Claws Mail on Weston with Xwayland disabled, and click on a row in the message list to select a message, the application does not immediately highlight the row in blue; instead, the message list appears to be unchanged. If you scroll or resize the message list, the list then updates to show the selected row correctly.

A very inefficient workaround for this issue is to change the code to redraw the entire message list pane whenever a row is selected.
Comment 1 Duncan 2020-12-03 18:30:50 CET
Seeing this too.

Do you have that "inefficient workaround" patch you implied you tried and may be using?

FWIW, running on kde/wayland (on gentoo) here, default as of a couple weeks ago, and today was the upgrade to live-git gtk3 claws and see how it does on wayland, day, complete with new bugzi account to cc-to and file bugs as necessary.  This is the first I've looked at of about three so far.

Not a dev but between being a gentooer for over a decade now and routinely running the kernel and most of kde (plus $random_pkg like claws) from live-git and following the git logs, I've found I can often hack-patch a workaround when I have to.

And that "inefficient workaround" you mentioned could save me from having to try to hack-patch possibly the same workaround.
Comment 2 Duncan 2020-12-03 18:53:50 CET
(In reply to M from comment #0)
> click on a row in the message list to select a message

Seeing it in the folder list too.  Click a folder, the message list changes but the highlight in the folder list doesn't.  Scrolling the folder list repaints so the new highlight is visible.

The worst is trying to click in the status/mark columns in the message list to toggle mark or read status, tho.  It won't show the new status until a scroll-to-repaint or the like. =:^(
Comment 3 M 2020-12-05 02:00:05 CET
Created attachment 2144 [details]
Patch that calls gtk_widget_queue_draw on input handling
Comment 4 M 2020-12-05 02:06:40 CET
When writing the above patch, to debug redraw operations, I used Weston, plus a debug binding (mod+shift+space, F; as per `man 7 weston-bindings`) to show the areas which the application indicated had been repainted. This isn't the right way to do things -- that would be, unless I'm mistaken, to ensure `gtk_widget_queue_draw_area` is called _somewhere_ inside gtkcmclist.c to mark the selected/unselected rows as needing a repaint.
Comment 5 M 2020-12-05 02:08:06 CET
> This isn't the right way to do things

(By "this" I mean the above attachment's approach, not the way I used to display damaged window areas.)
Comment 6 andyrtr 2020-12-05 22:08:41 CET
I'm also affected. Your patch doesn't solve it for me. I see the stderr comments but for me the message list isn't updated when clicking on a folder. Not the main mail folder and also not the rss folder.

But it seems to solve the "V" preview is now redrawn on every press.

Is there some main input loop somewhere? I couldn't find it so far. I see a full refresh/redraw when moving the cursor outside the window and back.

Using Arch Linux + swaywm here.
Comment 7 M 2020-12-09 18:28:34 CET
Created attachment 2154 [details]
Patch that calls gtk_widget_queue_draw from the gtkcmctree object itself

I've attached a new works a bit better for me, and calls redraw from the gtkcmctree object itself. I don't understand exactly what functions get called when, so there's probably more cases to handle. Clicking on row expand icons does not quite work, but now Ctrl+A does.
Comment 8 Duncan 2020-12-11 12:57:29 CET
(In reply to M from comment #7)
> Created attachment 2154 [details]
> Patch that calls gtk_widget_queue_draw from the gtkcmctree object itself

This is /much/ better than current-shipped (didn't get to your first try).

I see the expander-redraw case you mentioned still, and I can add two more redraw-needed cases to the list:

* With prefs>display>summaries>message-list>mark-message-as-read>when-selected-after-<0>-seconds set, a first click on a message doesn't change the message-list display to show the message as read.  Triggering a redraw displays it as read, however, so it's marked-read, only the redraw is missed.  A second click on the same message (re-select) also redraws the message-list to include the message-read status.

* The folder-list still needs some love.  Having a message marked-read in the message-list doesn't trigger a folder-list redraw, so the unread-message-count gets stale.

But at least I'm not having to scroll the message list to force redraws and see where I'm at all the time now.  I have the RSSyl plugin enabled with well over a hundred new messages a day coming in on one of the feeds, so keeping track of where I am in the message-list and what is read and what isn't as I read down the list a message at a time is kinda critical, and that generally works now (except for the current message not being marked read at the the selection redraw, but it is when I leave it for the next one, and the current message hilite at least works now).

So agreeing with what you already said, the concept's proven and works on the caught cases.  There's just a few more cases to catch.
Comment 9 andyrtr 2020-12-11 15:03:07 CET
I doubt it's a good idea to add gtk_widget_queue_draw at random places. There should be some main entry point where all user input and viewable changes are read and probably there the central refresh should happen.
Comment 10 M 2020-12-12 05:40:28 CET
Created attachment 2156 [details]
Patch that treats draw_X requests as queue_draw_X if not inside widget draw event

This last patch finally seems to solve the problem in the right place. As a general rule, GTK3 + Wayland requires that all drawing be performed from the GtkWidget draw event, and NOT in response to other events, as can be done using gdk_cairo_create. The patch converts all draw requests outside the draw event into calls to gtk_widget_queue_draw, which makes GTK send a draw event.

Based on the uses of `gdk_cairo_create`, there are two animations that don't work quite right: an "xor line", and the vertical line that appears when resizing columns. These animations also flicker on X11. I don't have the time to fix these. Similarly, I use gtk_widget_queue_draw() instead of calculating exactly which rectangles need to be redrawn. Only asking to redraw the necessary region could significantly reduce the latency of responding to a button press.

See also:
https://developer.gnome.org/gtk3/stable/ch26s02.html#id-1.6.3.4.11
And the following: (yes, this isn't a Qt app, but the ideas are the same)
https://doc.qt.io/archives/qt-4.8/porting4.html#painting-and-redrawing-widgets
And:
https://en.wikipedia.org/wiki/Retained_mode

> I doubt it's a good idea to add gtk_widget_queue_draw at random places. There should be some main entry point where all user input and viewable changes are read and probably there the central refresh should happen.

No, widget-based GUI systems are structured hierarchically/by composition, and recursively route events through the tree of widgets. The right place to handle input/updates/drawing for a custom widget implementation is inside that implementation. There is no main entry point.
Comment 11 M 2020-12-12 05:45:00 CET
> There is no main entry point.

OK, technically there are main entry _points_, all inside the widget class. But yes, the random gtk_widget_queue_draw approach was fundamentally wrong.
Comment 12 Paul 2020-12-12 07:16:39 CET
I think, rather than trying to get the clist code to behave, like bug bug #4344 'folderview rewrite using GtkTreeView', the summaryview also needs a similar rewrite.

However, in the meantime the patch could be good, but it's short-termism.
Comment 13 Duncan 2020-12-12 18:28:19 CET
(In reply to M from comment #10)
> Created attachment 2156 [details]
> Patch that treats draw_X requests as queue_draw_X if not inside widget draw
> event

This one's working very well for me here.  The column-resize line thing is workable /enough/ without the line, due to the switch to two-headed cursor when dragging the column separator, and I'm not even sure what the xor line is or where it's used but so far I'm not missing it. =:^)

Looking forward to being able to drop the patch locally when it's committed. =:^)


Between this patch and the patch for bug #4365 claws-core is pretty much fully wayland-workable for me.  Some of the plugins I use still have wayland bugs to work thru, tho.

(FWIW, the "pretty much" is due to an appmenu issue.  But I /think/ that's more likely a kde/plasma-wayland bug than a claws-gtk3/wayland bug.)
Comment 14 Thorsten Maerz 2021-03-21 19:22:10 CET
(In reply to M from comment #10)

For the records: The same redraw problems happen on the gtk3 Windows version as well, so I'll include this patch for that branch (until we get the ctree transition done).

Thanks for digging into this!
Comment 15 Paul 2021-04-14 14:05:00 CEST
Modifed version of the attached patch now pushed to git. Thanks!

Note You need to log in before you can comment on or make changes to this bug.