~anteater added vgmms 3 years ago

~anteater added ui 3 years ago

~anteater added needs research 3 years ago

~anteater 3 years ago

I did some debugging, manually grabbing stack traces in the middle of these jumps/redraws.

When opening a chat, backtraces often look like this:

<a href="/~anteater/mms-stack-bugs/4" title="~anteater/mms-stack-bugs#4: cannot delete chats or messages">#4</a>  0x00007ffff661437c in png_process_data () at /usr/lib/libpng16.so.16
<a href="/~anteater/mms-stack-bugs/5" title="~anteater/mms-stack-bugs#5: cannot [re]name chats">#5</a>  0x00007ffff414198a in  () at /usr/lib/gdk-pixbuf-2.0/2.10.0/loaders/libpixbufloader-png.so
<a href="/~anteater/mms-stack-bugs/6" title="~anteater/mms-stack-bugs#6: message UI is ugly (no bubbles)">#6</a>  0x00007ffff73c2f5c in gdk_pixbuf_loader_write () at /usr/lib/libgdk_pixbuf-2.0.so.0
<a href="/~anteater/mms-stack-bugs/7" title="~anteater/mms-stack-bugs#7: timestamp format should be configurable">#7</a>  0x00005555555bc6ef in vgmms::image::load ()
<a href="/~anteater/mms-stack-bugs/8" title="~anteater/mms-stack-bugs#8: no system contacts integration (for avatars/names)">#8</a>  0x0000555555633ac2 in vgmms::types::Attachment::with_data ()
<a href="/~anteater/mms-stack-bugs/9" title="~anteater/mms-stack-bugs#9: no UI to save attachments to a chosen filename">#9</a>  0x00005555555ae150 in vgmms::cell_renderer_message::CellRendererMessagePriv::get_pixbuf ()
<a href="/~anteater/mms-stack-bugs/10" title="~anteater/mms-stack-bugs#10: prevent SMS and MMS from being missed when phone off/offline">#10</a> 0x00005555556059a9 in vgmms::cell_renderer_message::CellRendererMessagePriv::content_draw_items ()
<a href="/~anteater/mms-stack-bugs/11" title="~anteater/mms-stack-bugs#11: no notifications on message receipt">#11</a> 0x00005555555aef35 in vgmms::cell_renderer_message::CellRendererMessagePriv::all_draw_items ()
<a href="/~anteater/mms-stack-bugs/12" title="~anteater/mms-stack-bugs#12: chat logs don&#39;t scroll automatically">#12</a> 0x000055555560631c in <vgmms::cell_renderer_message::CellRendererMessagePriv as gtk::subclass::cell_renderer::CellRendererImpl>::get_preferred_height_for_width ()
<a href="/~anteater/mms-stack-bugs/13" title="~anteater/mms-stack-bugs#13: select chat dialog is ugly">#13</a> 0x00005555555ebb1f in gtk::subclass::cell_renderer::cell_renderer_get_preferred_height_for_width ()
<a href="/~anteater/mms-stack-bugs/14" title="~anteater/mms-stack-bugs#14: offline mode">#14</a> 0x00007ffff7730df9 in gtk_cell_renderer_get_preferred_height_for_width () at /usr/lib/libgtk-3.so.0
<a href="/~anteater/mms-stack-bugs/15" title="~anteater/mms-stack-bugs#15: daemon mode">#15</a> 0x00007ffff7731528 in gtk_cell_area_request_renderer () at /usr/lib/libgtk-3.so.0

So we should see about being lazier to load images by storing aspect ratio somewhere or having a load fastpath that only obtains dimensions (gdkpixbuf does support this, but it might not be enough of a speedup, depending on how eager loaders are). The latter will be a smaller diff, so it's worth trying first.

In the same situation for chats without images, I also see a lot of backtraces pointing into pixman, where I see a lot of scalar, raster-looking code (e.g. and w4, w4, #0xff00ff; lsr w1, w9, <a href="/~anteater/mms-stack-bugs/8" title="~anteater/mms-stack-bugs#8: no system contacts integration (for avatars/names)">#8</a>; add w4, w4, w11; and w1, w1, #0xff00ff; add w1, w1, w9) in pixman_image_composite32 inside cairo_fill. It seems likely that the PinePhone's (Manjaro-ARM's, in this case) pixman is missing SIMD support.

When adding messages, backtraces often look like this:

<a href="/~anteater/mms-stack-bugs/5" title="~anteater/mms-stack-bugs#5: cannot [re]name chats">#5</a>  0x0000fffff73a61f8 in pango_parse_markup () at /usr/lib/libpango-1.0.so.0
<a href="/~anteater/mms-stack-bugs/6" title="~anteater/mms-stack-bugs#6: message UI is ugly (no bubbles)">#6</a>  0x0000fffff739d8c0 in pango_layout_set_markup_with_accel () at /usr/lib/libpango-1.0.so.0
<a href="/~anteater/mms-stack-bugs/7" title="~anteater/mms-stack-bugs#7: timestamp format should be configurable">#7</a>  0x0000aaaaaaca8e7c in pango::auto::layout::Layout::set_markup ()
<a href="/~anteater/mms-stack-bugs/8" title="~anteater/mms-stack-bugs#8: no system contacts integration (for avatars/names)">#8</a>  0x0000aaaaaab4877c in vgmms::cell_renderer_message::CellRendererMessagePriv::all_draw_items ()
<a href="/~anteater/mms-stack-bugs/9" title="~anteater/mms-stack-bugs#9: no UI to save attachments to a chosen filename">#9</a>  0x0000aaaaaab50214 in <vgmms::cell_renderer_message::CellRendererMessagePriv as gtk::subclass::cell_renderer::CellRendererImpl>::get_preferred_height_for_width ()
<a href="/~anteater/mms-stack-bugs/10" title="~anteater/mms-stack-bugs#10: prevent SMS and MMS from being missed when phone off/offline">#10</a> 0x0000aaaaaab54880 in gtk::subclass::cell_renderer::cell_renderer_get_preferred_height_for_width ()
<a href="/~anteater/mms-stack-bugs/11" title="~anteater/mms-stack-bugs#11: no notifications on message receipt">#11</a> 0x0000fffff7627ca4 in gtk_cell_renderer_get_preferred_height_for_width () at /usr/lib/libgtk-3.so.0
<a href="/~anteater/mms-stack-bugs/12" title="~anteater/mms-stack-bugs#12: chat logs don&#39;t scroll automatically">#12</a> 0x0000fffff761cb74 in gtk_cell_area_request_renderer () at /usr/lib/libgtk-3.so.0
<a href="/~anteater/mms-stack-bugs/13" title="~anteater/mms-stack-bugs#13: select chat dialog is ugly">#13</a> 0x0000fffff761ecec in  () at /usr/lib/libgtk-3.so.0
<a href="/~anteater/mms-stack-bugs/14" title="~anteater/mms-stack-bugs#14: offline mode">#14</a> 0x0000fffff761efd0 in  () at /usr/lib/libgtk-3.so.0
<a href="/~anteater/mms-stack-bugs/15" title="~anteater/mms-stack-bugs#15: daemon mode">#15</a> 0x0000fffff790168c in gtk_tree_view_column_cell_get_size () at /usr/lib/libgtk-3.so.0

or this:

<a href="/~anteater/mms-stack-bugs/4" title="~anteater/mms-stack-bugs#4: cannot delete chats or messages">#4</a>  0x0000fffff73a0afc in pango_layout_get_pixel_size () at /usr/lib/libpango-1.0.so.0
<a href="/~anteater/mms-stack-bugs/5" title="~anteater/mms-stack-bugs#5: cannot [re]name chats">#5</a>  0x0000aaaaaaca8e1c in pango::auto::layout::Layout::get_pixel_size ()
<a href="/~anteater/mms-stack-bugs/6" title="~anteater/mms-stack-bugs#6: message UI is ugly (no bubbles)">#6</a>  0x0000aaaaaab4fb64 in vgmms::cell_renderer_message::CellRendererMessagePriv::content_draw_items ()
<a href="/~anteater/mms-stack-bugs/7" title="~anteater/mms-stack-bugs#7: timestamp format should be configurable">#7</a>  0x0000aaaaaab48970 in vgmms::cell_renderer_message::CellRendererMessagePriv::all_draw_items ()
<a href="/~anteater/mms-stack-bugs/8" title="~anteater/mms-stack-bugs#8: no system contacts integration (for avatars/names)">#8</a>  0x0000aaaaaab50214 in <vgmms::cell_renderer_message::CellRendererMessagePriv as gtk::subclass::cell_renderer::CellRendererImpl>::get_preferred_height_for_width ()
<a href="/~anteater/mms-stack-bugs/9" title="~anteater/mms-stack-bugs#9: no UI to save attachments to a chosen filename">#9</a>  0x0000aaaaaab54880 in gtk::subclass::cell_renderer::cell_renderer_get_preferred_height_for_width ()
<a href="/~anteater/mms-stack-bugs/10" title="~anteater/mms-stack-bugs#10: prevent SMS and MMS from being missed when phone off/offline">#10</a> 0x0000fffff7627ca4 in gtk_cell_renderer_get_preferred_height_for_width () at /usr/lib/libgtk-3.so.0
<a href="/~anteater/mms-stack-bugs/11" title="~anteater/mms-stack-bugs#11: no notifications on message receipt">#11</a> 0x0000fffff761cb74 in gtk_cell_area_request_renderer () at /usr/lib/libgtk-3.so.0
<a href="/~anteater/mms-stack-bugs/12" title="~anteater/mms-stack-bugs#12: chat logs don&#39;t scroll automatically">#12</a> 0x0000fffff761ecec in  () at /usr/lib/libgtk-3.so.0
<a href="/~anteater/mms-stack-bugs/13" title="~anteater/mms-stack-bugs#13: select chat dialog is ugly">#13</a> 0x0000fffff761efd0 in  () at /usr/lib/libgtk-3.so.0
<a href="/~anteater/mms-stack-bugs/14" title="~anteater/mms-stack-bugs#14: offline mode">#14</a> 0x0000fffff790168c in gtk_tree_view_column_cell_get_size () at /usr/lib/libgtk-3.so.0

Which might be addressed by caching heights for individual Messages at a fixed width/style. I also see a number of backtraces into pixman_image_composite32 inside cairo_fill again.

~anteater 3 years ago*

I have a prototype fix for the second issue (jumping when adding messages), which works for the simple case of adding a new message at the end without the chat jumping and just needs validation that it correctly diffs the message log for other scenarios.

The issue was not precisely as I described in my last comment (and was closer to what the issue description suggests): GTK itself caches cell renderer heights, but we were re-creating the ListStore on every message add. We'll instead have it be a field (but not a vgtk Property) of the ChatLog struct, and update it minimally in ChatLog's impl of Component::update.

The plan for the first issue is still to see how much a size-only pixbuf loading pass helps.

~anteater 3 years ago

I've pushed the fix for the second issue, and done a lot of iteration trying to improve the first one.

A size-only pixbuf loading pass helps a considerable amount, and I've also significantly optimized size computation for CellRendererMessage. The only thing left to do is make actual image loading asynchronous, loading with GdkPixbuf in another thread.

This is necessary because the design of is GtkTreeView is inherently racy: it uses a timer to decide when to start rendering rows, and if not all rows have had their size computed when this timer fires, then the wrong (non-bottom) rows will get rendered. This is a problem if these take a long time to render (as is the case with high-resolution images that get loaded and downscaled synchronously), because they mean we'll miss the timer a second (or more) times.

~anteater REPORTED IMPLEMENTED 3 years ago

Asynchronous (multithreaded) pixbuf loading merged in be46febc.

This still isn't perfect, but it's usable.