Add "Don't emit size-changed" patch #1

mmstick · 2019-06-11T22:51:44Z

Reduces CPU usage by 22% when dragging windows.

Upstream: https://gitlab.gnome.org/GNOME/mutter/merge_requests/568

Reduces CPU usage by 22% when dragging windows. Upstream: https://gitlab.gnome.org/GNOME/mutter/merge_requests/568

mmstick · 2019-06-14T17:56:58Z

I think this might offset some of the choppiness of dragging OpenGL-accelerated windows (Chrome and Electron applications). Related upstream bug report, which NVIDIA is aware of: https://bugs.launchpad.net/ubuntu/+source/nvidia-graphics-drivers-390/+bug/1799679. People who experience choppy animations when dragging an OpenGL-accelerated window are because their CPU isn't fast enough, spending all of their time in the Xorg process.

We want to clip it away if 1) The window is fully opaque or 2) If it's translucent but has a frame (as explained in the comment above). The code didn't quite match and we were only applying it on case #2. Case #1 is far more common, and saves us from pushing some drawing that we know will be covered in the end. https://gitlab.gnome.org/GNOME/mutter/merge_requests/698

Otherwise we'll end up crashing if we had two connected monitors, unplugged both, then replugged in a single one. Avoids the following error: #0 _g_log_abort () #1 g_logv () #2 g_log () #3 meta_monitor_manager_get_logical_monitor_from_number () #4 meta_window_get_work_area_for_monitor () #5 meta_window_get_tile_area () #6 constrain_maximization () #7 do_all_constraints () #8 meta_window_constrain () #9 meta_window_move_resize_internal () #10 meta_window_move_resize_frame () #11 meta_window_move_resize_now () #12 idle_move_resize () #13 call_idle_later () https://bugzilla.redhat.com/show_bug.cgi?id=1767703 https://gitlab.gnome.org/GNOME/mutter/merge_requests/912

Otherwise we'll end up trying to access the out of date state later. Fixes the following test failure backtrace: #0 _g_log_abort () #1 g_logv () #2 g_log () #3 meta_monitor_manager_get_logical_monitor_from_number () #4 meta_window_get_work_area_for_monitor () #5 meta_window_get_tile_area () #6 constrain_maximization () #7 do_all_constraints () #8 meta_window_constrain () #9 meta_window_move_resize_internal () #10 meta_window_tile () https://gitlab.gnome.org/GNOME/mutter/merge_requests/912

Otherwise we'll end up crashing if we had two connected monitors, unplugged both, then replugged in a single one. Avoids the following error: #0 _g_log_abort () #1 g_logv () #2 g_log () #3 meta_monitor_manager_get_logical_monitor_from_number () #4 meta_window_get_work_area_for_monitor () #5 meta_window_get_tile_area () #6 constrain_maximization () #7 do_all_constraints () #8 meta_window_constrain () #9 meta_window_move_resize_internal () #10 meta_window_move_resize_frame () #11 meta_window_move_resize_now () #12 idle_move_resize () #13 call_idle_later () https://bugzilla.redhat.com/show_bug.cgi?id=1767703 https://gitlab.gnome.org/GNOME/mutter/merge_requests/912

Otherwise we'll end up trying to access the out of date state later. Fixes the following test failure backtrace: #0 _g_log_abort () #1 g_logv () #2 g_log () #3 meta_monitor_manager_get_logical_monitor_from_number () #4 meta_window_get_work_area_for_monitor () #5 meta_window_get_tile_area () #6 constrain_maximization () #7 do_all_constraints () #8 meta_window_constrain () #9 meta_window_move_resize_internal () #10 meta_window_tile () https://gitlab.gnome.org/GNOME/mutter/merge_requests/912

According to the cogl_bitmap_new_for_data documentation, the data is not copied, so the application must keep the buffer alive for the lifetime of the CoglBitmap. Freeing it too early led to a use-after-free in the cogl unit tests. With that fixed, the test passes, so remove the known failure annotation. This AddressSanitizer trace is from the original cogl, but the bug and fix apply equally to mutter's fork of cogl: ==6223==ERROR: AddressSanitizer: heap-use-after-free on address 0x62100001a500 at pc 0x7f3e2d4e7f4e bp 0x7ffcd9c41f30 sp 0x7ffcd9c416e0 READ of size 4096 at 0x62100001a500 thread T0 #0 0x7f3e2d4e7f4d (/usr/lib/x86_64-linux-gnu/libasan.so.5+0x96f4d) #1 0x7f3e260c7f6b in util_copy_box ../src/gallium/auxiliary/util/u_surface.c:131 #2 0x7f3e268c6c10 in u_default_texture_subdata ../src/gallium/auxiliary/util/u_transfer.c:67 #3 0x7f3e26486459 in st_TexSubImage ../src/mesa/state_tracker/st_cb_texture.c:1480 #4 0x7f3e26487029 in st_TexImage ../src/mesa/state_tracker/st_cb_texture.c:1709 #5 0x7f3e26487029 in st_TexImage ../src/mesa/state_tracker/st_cb_texture.c:1691 #6 0x7f3e2644bdba in teximage ../src/mesa/main/teximage.c:3105 #7 0x7f3e2644bdba in teximage_err ../src/mesa/main/teximage.c:3132 #8 0x7f3e2644d84f in _mesa_TexImage2D ../src/mesa/main/teximage.c:3170 #9 0x7f3e2cd1f7df in _cogl_texture_driver_upload_to_gl driver/gl/gl/cogl-texture-driver-gl.c:347 #10 0x7f3e2ccd441b in allocate_from_bitmap driver/gl/cogl-texture-2d-gl.c:255 #11 0x7f3e2ccd441b in _cogl_texture_2d_gl_allocate driver/gl/cogl-texture-2d-gl.c:462 #12 0x7f3e2ce3a6c0 in cogl_texture_allocate cogl/cogl-texture.c:1398 #13 0x7f3e2ce3e116 in _cogl_texture_pre_paint cogl/cogl-texture.c:359 #14 0x7f3e2cdee177 in _cogl_pipeline_layer_pre_paint cogl/cogl-pipeline-layer.c:864 #15 0x7f3e2cd574af in _cogl_rectangles_validate_layer_cb cogl/cogl-primitives.c:542 #16 0x7f3e2cdd742f in cogl_pipeline_foreach_layer cogl/cogl-pipeline.c:735 #17 0x7f3e2cd5c8b0 in _cogl_framebuffer_draw_multitextured_rectangles cogl/cogl-primitives.c:658 #18 0x7f3e2cd60152 in cogl_rectangle cogl/cogl-primitives.c:858 #19 0x5570a71ed6a0 in check_texture tests/conform/test-premult.c:103 #20 0x5570a71ed946 in test_premult tests/conform/test-premult.c:159 #21 0x5570a71df0d6 in main tests/conform/test-conform-main.c:58 #22 0x7f3e2bcd809a in __libc_start_main ../csu/libc-start.c:308 #23 0x5570a71e0869 in _start (/home/smcv/src/debian/cogl/tests/conform/.libs/test-conformance+0x33869) 0x62100001a500 is located 0 bytes inside of 4096-byte region [0x62100001a500,0x62100001b500) freed by thread T0 here: #0 0x7f3e2d5581d7 in __interceptor_free (/usr/lib/x86_64-linux-gnu/libasan.so.5+0x1071d7) #1 0x5570a71ed58b in make_texture tests/conform/test-premult.c:69 previously allocated by thread T0 here: #0 0x7f3e2d558588 in malloc (/usr/lib/x86_64-linux-gnu/libasan.so.5+0x107588) #1 0x7f3e2d384500 in g_malloc ../../../glib/gmem.c:99 This was originally cogl!12. https://gitlab.gnome.org/GNOME/mutter/-/merge_requests/1274 Signed-off-by: Simon McVittie <smcv@debian.org>

When a touch sequence was rejected, we'd update the event timestamps of incoming touch events to help with implementing grabs. This was done by sending a ClientMessage with a counter, and comparing the counter to decide whether we're seing a replayed event or not. This had the unforseen consequence that we would potentially end up destroying all actors including the stage, since, when mutter receives a ClientMessage event, it would assume that it's a WM_PROTOCOLS event, and handle it as such. The problem with this approach is that it would ignore fact that there might be other ClientMessage types sent to it, for example the touch synchronization one. What could happen is that the touch count value would match up with the value of the WM_DELETE_WINDOW atom, clutter would treat this as WM_PROTOCOLS:WM_DELETE_WINDOW, which it'd translate to clutter_actor_destroy(stage). Destroying the stage in such a way is not expected, and caused wierd crashes in different places depending on what was going on. This commit make sure we only treat WM_PROTOCOLS client messages as WM_PROTOCOLS client messages effectively avoiding the issue. This fixes crashes such as: #0 meta_window_get_buffer_rect (window=0x0, rect=rect@entry=0x7ffd7fc62e40) at core/window.c:4396 #1 0x00007f1e2634837f in get_top_visible_window_actor (compositor=0x297d700, compositor=0x297d700) at compositor/compositor.c:1059 #2 meta_compositor_sync_stack (compositor=0x297d700, stack=<optimized out>, stack@entry=0x26e3140) at compositor/compositor.c:1176 #3 0x00007f1e263757ac in meta_stack_tracker_sync_stack (tracker=0x297dbc0) at core/stack-tracker.c:871 #4 0x00007f1e26375899 in stack_tracker_sync_stack_later (data=<optimized out>) at core/stack-tracker.c:881 #5 0x00007f1e26376914 in run_repaint_laters (laters_list=0x7f1e2663b7d8 <laters+24>) at core/util.c:809 #6 run_all_repaint_laters (data=<optimized out>) at core/util.c:826 #7 0x00007f1e26b18325 in _clutter_run_repaint_functions (flags=flags@entry=CLUTTER_REPAINT_FLAGS_PRE_PAINT) at clutter-main.c:3448 #8 0x00007f1e26b18fc5 in master_clock_update_stages (master_clock=0x32d6a80, stages=0x4e5a740) at clutter-master-clock-default.c:437 #9 clutter_clock_dispatch (source=<optimized out>, callback=<optimized out>, user_data=<optimized out>) at clutter-master-clock-default.c:567 #10 0x00007f1e27e48049 in g_main_dispatch (context=0x225b8d0) at gmain.c:3175 #11 g_main_context_dispatch (context=context@entry=0x225b8d0) at gmain.c:3828 #12 0x00007f1e27e483a8 in g_main_context_iterate (context=0x225b8d0, block=block@entry=1, dispatch=dispatch@entry=1, self=<optimized out>) at gmain.c:3901 #13 0x00007f1e27e4867a in g_main_loop_run (loop=0x24e29f0) at gmain.c:4097 #14 0x00007f1e2636a3dc in meta_run () at core/main.c:666 #15 0x000000000040219c in main (argc=1, argv=0x7ffd7fc63238) at ../src/main.c:534 and #0 0x00007f93943c1f25 in raise () at /usr/lib/libc.so.6 #1 0x00007f93943ab897 in abort () at /usr/lib/libc.so.6 #2 0x00007f9393e1e062 in g_assertion_message (domain=<optimized out>, file=<optimized out>, line=<optimized out>, func=0x7f93933e6860 <__func__.116322> "meta_x11_get_stage_window", #3 0x00007f9393e4ab1d in g_assertion_message_expr () #4 0x00007f939338ecd7 in meta_x11_get_stage_window (stage=<optimized out>) at ../mutter/src/backends/x11/meta-stage-x11.c:923 #5 0x00007f939339e599 in meta_backend_x11_cm_translate_device_event (x11=<optimized out>, device_event=0x55bc8bcfd6b0) at ../mutter/src/backends/x11/cm/meta-backend-x11-cm.c:381 #6 0x00007f939339f2e2 in meta_backend_x11_translate_device_event (device_event=0x55bc8bcfd6b0, x11=0x55bc89dd5220) at ../mutter/src/backends/x11/meta-backend-x11.c:179 #7 0x00007f939339f2e2 in translate_device_event (device_event=0x55bc8bcfd6b0, x11=0x55bc89dd5220) at ../mutter/src/backends/x11/meta-backend-x11.c:208 #8 0x00007f939339f2e2 in maybe_spoof_event_as_stage_event (input_event=0x55bc8bcfd6b0, x11=0x55bc89dd5220) at ../mutter/src/backends/x11/meta-backend-x11.c:284 #9 0x00007f939339f2e2 in handle_input_event (event=0x7fff62d60490, x11=0x55bc89dd5220) at ../mutter/src/backends/x11/meta-backend-x11.c:309 #10 0x00007f939339f2e2 in handle_host_xevent (event=0x7fff62d60490, backend=0x55bc89dd5220) at ../mutter/src/backends/x11/meta-backend-x11.c:413 #11 0x00007f939339f2e2 in x_event_source_dispatch (source=<optimized out>, callback=<optimized out>, user_data=<optimized out>) at ../mutter/src/backends/x11/meta-backend-x11.c:467 #12 0x00007f9393e6c39e in g_main_dispatch (context=0x55bc89dd03e0) at ../glib/glib/gmain.c:3179 #13 0x00007f9393e6c39e in g_main_context_dispatch (context=context@entry=0x55bc89dd03e0) at ../glib/glib/gmain.c:3844 #14 0x00007f9393e6e1b1 in g_main_context_iterate (context=0x55bc89dd03e0, block=block@entry=1, dispatch=dispatch@entry=1, self=<optimized out>) at ../glib/glib/gmain.c:3917 #15 0x00007f9393e6f0c3 in g_main_loop_run (loop=0x55bc8a042640) at ../glib/glib/gmain.c:4111 #16 0x00007f9393369a0c in meta_run () at ../mutter/src/core/main.c:676 #17 0x000055bc880f2426 in main (argc=<optimized out>, argv=<optimized out>) at ../gnome-shell/src/main.c:552 Related: https://gitlab.gnome.org/GNOME/mutter/-/issues/338 Closes: https://gitlab.gnome.org/GNOME/mutter/-/issues/951 https://gitlab.gnome.org/GNOME/mutter/-/merge_requests/1317

When a touch sequence was rejected, we'd update the event timestamps of incoming touch events to help with implementing grabs. This was done by sending a ClientMessage with a counter, and comparing the counter to decide whether we're seing a replayed event or not. This had the unforseen consequence that we would potentially end up destroying all actors including the stage, since, when mutter receives a ClientMessage event, it would assume that it's a WM_PROTOCOLS event, and handle it as such. The problem with this approach is that it would ignore fact that there might be other ClientMessage types sent to it, for example the touch synchronization one. What could happen is that the touch count value would match up with the value of the WM_DELETE_WINDOW atom, clutter would treat this as WM_PROTOCOLS:WM_DELETE_WINDOW, which it'd translate to clutter_actor_destroy(stage). Destroying the stage in such a way is not expected, and caused wierd crashes in different places depending on what was going on. This commit make sure we only treat WM_PROTOCOLS client messages as WM_PROTOCOLS client messages effectively avoiding the issue. This fixes crashes such as: #0 meta_window_get_buffer_rect (window=0x0, rect=rect@entry=0x7ffd7fc62e40) at core/window.c:4396 #1 0x00007f1e2634837f in get_top_visible_window_actor (compositor=0x297d700, compositor=0x297d700) at compositor/compositor.c:1059 #2 meta_compositor_sync_stack (compositor=0x297d700, stack=<optimized out>, stack@entry=0x26e3140) at compositor/compositor.c:1176 #3 0x00007f1e263757ac in meta_stack_tracker_sync_stack (tracker=0x297dbc0) at core/stack-tracker.c:871 #4 0x00007f1e26375899 in stack_tracker_sync_stack_later (data=<optimized out>) at core/stack-tracker.c:881 #5 0x00007f1e26376914 in run_repaint_laters (laters_list=0x7f1e2663b7d8 <laters+24>) at core/util.c:809 #6 run_all_repaint_laters (data=<optimized out>) at core/util.c:826 #7 0x00007f1e26b18325 in _clutter_run_repaint_functions (flags=flags@entry=CLUTTER_REPAINT_FLAGS_PRE_PAINT) at clutter-main.c:3448 #8 0x00007f1e26b18fc5 in master_clock_update_stages (master_clock=0x32d6a80, stages=0x4e5a740) at clutter-master-clock-default.c:437 #9 clutter_clock_dispatch (source=<optimized out>, callback=<optimized out>, user_data=<optimized out>) at clutter-master-clock-default.c:567 #10 0x00007f1e27e48049 in g_main_dispatch (context=0x225b8d0) at gmain.c:3175 #11 g_main_context_dispatch (context=context@entry=0x225b8d0) at gmain.c:3828 #12 0x00007f1e27e483a8 in g_main_context_iterate (context=0x225b8d0, block=block@entry=1, dispatch=dispatch@entry=1, self=<optimized out>) at gmain.c:3901 #13 0x00007f1e27e4867a in g_main_loop_run (loop=0x24e29f0) at gmain.c:4097 #14 0x00007f1e2636a3dc in meta_run () at core/main.c:666 #15 0x000000000040219c in main (argc=1, argv=0x7ffd7fc63238) at ../src/main.c:534 and #0 0x00007f93943c1f25 in raise () at /usr/lib/libc.so.6 #1 0x00007f93943ab897 in abort () at /usr/lib/libc.so.6 #2 0x00007f9393e1e062 in g_assertion_message (domain=<optimized out>, file=<optimized out>, line=<optimized out>, func=0x7f93933e6860 <__func__.116322> "meta_x11_get_stage_window", #3 0x00007f9393e4ab1d in g_assertion_message_expr () #4 0x00007f939338ecd7 in meta_x11_get_stage_window (stage=<optimized out>) at ../mutter/src/backends/x11/meta-stage-x11.c:923 #5 0x00007f939339e599 in meta_backend_x11_cm_translate_device_event (x11=<optimized out>, device_event=0x55bc8bcfd6b0) at ../mutter/src/backends/x11/cm/meta-backend-x11-cm.c:381 #6 0x00007f939339f2e2 in meta_backend_x11_translate_device_event (device_event=0x55bc8bcfd6b0, x11=0x55bc89dd5220) at ../mutter/src/backends/x11/meta-backend-x11.c:179 #7 0x00007f939339f2e2 in translate_device_event (device_event=0x55bc8bcfd6b0, x11=0x55bc89dd5220) at ../mutter/src/backends/x11/meta-backend-x11.c:208 #8 0x00007f939339f2e2 in maybe_spoof_event_as_stage_event (input_event=0x55bc8bcfd6b0, x11=0x55bc89dd5220) at ../mutter/src/backends/x11/meta-backend-x11.c:284 #9 0x00007f939339f2e2 in handle_input_event (event=0x7fff62d60490, x11=0x55bc89dd5220) at ../mutter/src/backends/x11/meta-backend-x11.c:309 #10 0x00007f939339f2e2 in handle_host_xevent (event=0x7fff62d60490, backend=0x55bc89dd5220) at ../mutter/src/backends/x11/meta-backend-x11.c:413 #11 0x00007f939339f2e2 in x_event_source_dispatch (source=<optimized out>, callback=<optimized out>, user_data=<optimized out>) at ../mutter/src/backends/x11/meta-backend-x11.c:467 #12 0x00007f9393e6c39e in g_main_dispatch (context=0x55bc89dd03e0) at ../glib/glib/gmain.c:3179 #13 0x00007f9393e6c39e in g_main_context_dispatch (context=context@entry=0x55bc89dd03e0) at ../glib/glib/gmain.c:3844 #14 0x00007f9393e6e1b1 in g_main_context_iterate (context=0x55bc89dd03e0, block=block@entry=1, dispatch=dispatch@entry=1, self=<optimized out>) at ../glib/glib/gmain.c:3917 #15 0x00007f9393e6f0c3 in g_main_loop_run (loop=0x55bc8a042640) at ../glib/glib/gmain.c:4111 #16 0x00007f9393369a0c in meta_run () at ../mutter/src/core/main.c:676 #17 0x000055bc880f2426 in main (argc=<optimized out>, argv=<optimized out>) at ../gnome-shell/src/main.c:552 Related: https://gitlab.gnome.org/GNOME/mutter/-/issues/338 Closes: https://gitlab.gnome.org/GNOME/mutter/-/issues/951 https://gitlab.gnome.org/GNOME/mutter/-/merge_requests/1317 (cherry picked from commit e5ea8f5)

This would without 'clutter/timeline: Clear stage view listener when actor destroyed' result in backtraces such as Program terminated with signal SIGSEGV, Segmentation fault. #0 on_stage_stage_views_changed () #1 g_closure_invoke () #2 signal_emit_unlocked_R () #3 g_signal_emit_valist () #4 g_signal_emit () #5 update_stage_views () #6 clutter_actor_update_stage_views () #7 clutter_stage_update_actor_stage_views () #8 handle_frame_clock_frame () #9 clutter_frame_clock_dispatch () #10 frame_clock_source_dispatch () #11 g_main_dispatch () Part-of: <https://gitlab.gnome.org/GNOME/mutter/-/merge_requests/1719>

The X server generates a property change notification whenever it processes a property change request, even if the value of the property is not changing. This triggers libgdk to probe all display outputs, which can be slow depending on which display driver and hardware are in use. #0 0x00007f8e4d5e91a0 in XRRUpdateConfiguration () at /usr/lib/libXrandr.so.2 #1 0x00007f8e505208da in _gdk_x11_screen_size_changed (screen=0x5566e4b7e080, event=0x7ffe0e44bd60) at ../gdk/x11/gdkscreen-x11.c:1199 #2 0x00007f8e505066d1 in gdk_x11_display_translate_event (translator=0x5566e4b5b110, display=0x5566e4b5b110, event=0x7f8dec001b20, xevent=0x7ffe0e44bd60) at ../gdk/x11/gdkdisplay-x11.c:1201 #3 0x00007f8e505135a0 in _gdk_x11_event_translator_translate (translator=0x5566e4b5b110, display=0x5566e4b5b110, xevent=0x7ffe0e44bd60) at ../gdk/x11/gdkeventtranslator.c:51 #4 0x00007f8e50512c97 in gdk_event_source_translate_event (event_source=0x5566e4b764a0, xevent=0x7ffe0e44bd60) at ../gdk/x11/gdkeventsource.c:243 #5 0x00007f8e50512f57 in _gdk_x11_display_queue_events (display=0x5566e4b5b110) at ../gdk/x11/gdkeventsource.c:341 #6 0x00007f8e50497644 in gdk_display_get_event (display=0x5566e4b5b110) at ../gdk/gdkdisplay.c:442 #7 0x00007f8e5051301f in gdk_event_source_dispatch (source=0x5566e4b764a0, callback=0x0, user_data=0x0) at ../gdk/x11/gdkeventsource.c:363 #8 0x00007f8e516ecf9c in g_main_context_dispatch () at /usr/lib/libglib-2.0.so.0 #9 0x00007f8e51740a49 in () at /usr/lib/libglib-2.0.so.0 #10 0x00007f8e516ec503 in g_main_loop_run () at /usr/lib/libglib-2.0.so.0 #11 0x00007f8e508ef5fd in meta_run_main_loop () at ../src/core/main.c:928 #12 0x00007f8e508ef60e in meta_run () at ../src/core/main.c:943 #13 0x00005566e450842a in () #14 0x00007f8e50649b25 in __libc_start_main () at /usr/lib/libc.so.6 When GNOME is animating a display fade when the night light feature is toggled on or off, it sends a lot of change requests for the CTM property in the process, which triggers a lot of display probes from gdk. In the case of the night light feature, the CTM itself is not actually changing, so these requests are redundant. Fix this by caching the CTM value in the MetaOutputXrandr and skipping the server requests if it's not being changed. Fixes: https://gitlab.gnome.org/GNOME/gnome-shell/-/issues/3978 Signed-off-by: Aaron Plattner <aplattner@nvidia.com> Part-of: <https://gitlab.gnome.org/GNOME/mutter/-/merge_requests/1816>

The failure to allocate was not properly handled, causing crashes later on due to the offscreen being NULL. #0 cogl_gl_framebuffer_bind (target=36160, gl_framebuffer=0x0) #1 _cogl_driver_gl_flush_framebuffer_state (...) #2 cogl_context_flush_framebuffer_state (read_buffer=0x55f48f386780, draw_buffer=0x55f48f386780, ...) #3 cogl_framebuffer_clear4f (framebuffer=0x55f48f386780, ...) #4 clutter_layer_node_pre_draw (...) #5 clutter_paint_node_paint (...) ... Part-of: <https://gitlab.gnome.org/GNOME/mutter/-/merge_requests/1942>

…_cpu It was dropping to zero after the first frame because it hadn't been incremented high enough. So the second frame would crash with: ``` #0 g_type_check_instance_cast #1 META_DRM_BUFFER #2 copy_shared_framebuffer_cpu ``` That's the CPU-copy path (fallback-fallback) that probably no one is using but it does work after this fix. Exactly the same issue as was fixed in `copy_shared_framebuffer_primary_gpu` by 36352f4. Part-of: <https://gitlab.gnome.org/GNOME/mutter/-/merge_requests/2104> (cherry picked from commit acadf5b)

…_cpu It was dropping to zero after the first frame because it hadn't been incremented high enough. So the second frame would crash with: ``` #0 g_type_check_instance_cast #1 META_DRM_BUFFER #2 copy_shared_framebuffer_cpu ``` That's the CPU-copy path (fallback-fallback) that probably no one is using but it does work after this fix. Exactly the same issue as was fixed in `copy_shared_framebuffer_primary_gpu` by 36352f4. Part-of: <https://gitlab.gnome.org/GNOME/mutter/-/merge_requests/2104>

Client connections may linger after the test driver is teared down; handle this gracefully by unsetting the user data on the wl_resource, and make the resource destructor a no-op, instead of where it would otherwise remove itself from the resource list. This fixes this crash seen in CI: Received signal 11 (SIGSEGV) #0 g_list_remove() at ../glib/glist.c:596 #1 test_driver_destructor() at ../src/tests/meta-wayland-test-driver.c:219 #2 destroy_resource() at ../src/wayland-server.c:730 #3 for_each_helper() at ../src/wayland-util.c:416 #4 wl_map_for_each() at ../src/wayland-util.c:430 #5 wl_client_destroy() at ../src/wayland-server.c:889 #6 wl_display_destroy_clients() at ../src/wayland-server.c:1482 #7 meta_wayland_compositor_prepare_shutdown() at ../src/wayland/meta-wayland.c:441 #8 meta_context_dispose() at ../src/core/meta-context.c:667 #9 g_object_unref() at ../gobject/gobject.c:3863 #9 g_object_unref() at ../gobject/gobject.c:3780 #10 glib_autoptr_clear_GObject() at /usr/include/glib-2.0/gobject/gobject-autocleanups.h:29 #10 glib_autoptr_clear_MetaContext() at ../src/meta/meta-context.h:32 #10 glib_autoptr_cleanup_MetaContext() at ../src/meta/meta-context.h:32 #10 main() at ../src/tests/wayland-unit-tests.c:707 #11 __libc_start_call_main() in /usr/lib64/libc.so.6 #12 __libc_start_main() in /usr/lib64/libc.so.6 #13 _start() in /builds/GNOME/mutter/build/src/tests/mutter-wayland-unit Part-of: <https://gitlab.gnome.org/GNOME/mutter/-/merge_requests/2601> (cherry picked from commit c7954be56edec947f3b649ce88420214bf9d4961)

We have no way to sanely add safe modes if there are no modes we can compare with, thus don't try. Fixes the following crash: #0 are_all_modes_equally_sized at ../src/backends/native/meta-output-kms.c:284 #1 maybe_add_fallback_modes at ../src/backends/native/meta-output-kms.c:310 #2 init_output_modes at ../src/backends/native/meta-output-kms.c:347 #3 meta_output_kms_new at ../src/backends/native/meta-output-kms.c:414 #4 init_outputs at ../src/backends/native/meta-gpu-kms.c:332 #5 meta_gpu_kms_read_current at ../src/backends/native/meta-gpu-kms.c:368 #6 meta_gpu_kms_new at ../src/backends/native/meta-gpu-kms.c:403 #7 create_gpu_from_udev_device at ../src/backends/native/meta-backend-native.c:461 #8 init_gpus at ../src/backends/native/meta-backend-native.c:551 #9 meta_backend_native_initable_init at ../src/backends/native/meta-backend-native.c:632 Fixes: 877cc3e Related: https://bugzilla.redhat.com/show_bug.cgi?id=2127801 Part-of: <https://gitlab.gnome.org/GNOME/mutter/-/merge_requests/2646> (cherry picked from commit 2c8adb19660cb2cd53381372833e088962437d3b)

In fcfe90a, multiple for loops were replaced with META_WAYLAND_SURFACE_FOREACH_SUBSURFACE. However, this substitution was not side-effect free, and introduced a null-pointer dereference risk as shown in the example below: Old: for (n = g_node_first_child (surface->subsurface_branch_node); n; n = g_node_next_sibling (n)) { if (G_NODE_IS_LEAF (n)) continue; meta_wayland_surface_update_outputs_recursively (n->data); } n is checked for NULL during each loop in the condition expression. Therefore, when `G_NODE_IS_LEAF (n)` is called, `n` is guaranteed not to be NULL. Note also that g_node_first_child is also NULL-safe since it performs a NULL check internally. New: META_WAYLAND_SURFACE_FOREACH_SUBSURFACE (surface, subsurface_surface) meta_wayland_surface_update_outputs_recursively (subsurface_surface); = for (GNode *G_PASTE(__n, __LINE__) = meta_get_first_subsurface_node ((surface)); \ (subsurface = (G_PASTE (__n, __LINE__) ? G_PASTE (__n, __LINE__)->data : NULL)); \ G_PASTE (__n, __LINE__) = meta_get_next_subsurface_sibling (G_PASTE (__n, __LINE__))) In the new logic `subsurface` is still checked for NULL in the loop condition. However, in the new loop init: ... meta_get_first_subsurface_node (MetaWaylandSurface *surface) ... n = g_node_first_child (surface->subsurface_branch_node); if (!G_NODE_IS_LEAF (n)) ... The above implementation performs a `G_NODE_IS_LEAF` call, which performs a dereference on `n`, without first checking for NULLs. This NULL dereference triggers the following gnome-shell crash: Core was generated by `/usr/bin/gnome-shell'. Program terminated with signal SIGSEGV, Segmentation fault. #0 meta_get_first_subsurface_node (surface=0x55d589623450) at ../src/wayland/meta-wayland-surface.h:399 #1 pointer_can_grab_surface (pointer=0x7f6dc4012700, surface=0x55d589623450) at ../src/wayland/meta-wayland-pointer.c:1306 #2 0x00007f6ddb94d509 in meta_wayland_pointer_can_grab_surface (pointer=<optimized out>, surface=surface@entry=0x55d589623450, serial=serial@entry=996) at ../src/wayland/meta-wayland-pointer.c:1321 #3 0x00007f6ddb950d05 in meta_wayland_seat_get_grab_info (seat=seat@entry=0x55d586c24f20, surface=0x55d589623450, serial=996, require_pressed=require_pressed@entry=0, x=x@entry=0x0, y=y@entry=0x0) at ../src/wayland/meta-wayland-seat.c:467 Part-of: <https://gitlab.gnome.org/GNOME/mutter/-/merge_requests/2655> (cherry picked from commit 87e1d72cd4edad1ddb33f8aa38794d80e3a9ca16)

It is generally assumed here and there that the pointer at all point in time is within some logical monitor, if there is any logical monitor to be within. With the input thread, this was for a short amount of time not reliable, resulting in crashes in combination with hotplugging or suspend/resume, where monitors come and go quickly. What happens is that the pointer at first is within a logical monitor, but when that logical monitor is removed, while the new monitor viewports are handed to the input thread, the constraining happens asynchronously, meaning there is a time between between the new viewports are sent, and before clutter_seat_query_state() starts reporting the constrained position. If a new client mapped a maximized window during this short time frame, we'd crash with #0 meta_window_place at ../src/core/place.c:883 #1 place_window_if_needed at ../src/core/constraints.c:562 #2 meta_window_constrain at ../src/core/constraints.c:310 #3 meta_window_move_resize_internal at ../src/core/window.c:3869 #4 meta_window_force_placement at ../src/core/window.c:2120 #5 xdg_toplevel_set_maximized at ../src/wayland/meta-wayland-xdg-shell.c:429 #6 ffi_call_unix64 at ../src/x86/unix64.S:105 #7 ffi_call_int at ../src/x86/ffi64.c:672 #8 wl_closure_invoke at ../src/connection.c:1025 #9 wl_client_connection_data at ../src/wayland-server.c:437 The fix for this is to make sure that the viewports are updated and pointers constrained synchronously, i.e. the main thread will wait until after the input thread is done constraining before continuing. Related: https://bugzilla.redhat.com/show_bug.cgi?id=2147502 Part-of: <https://gitlab.gnome.org/GNOME/mutter/-/merge_requests/2711> (cherry picked from commit 1645171d4b8d50940286adb2d5b6b2cfc5a05beb)

If two X11 windows were the last two, we'd remove them from the stack while unmanaging them. That'd hit an assert in meta_stack_tracker_restack_managed(), resulting in the following crash when Xwayland exited unexpectedly with two or more X11 windows being the only windows on the stack: #1 g_assertion_message() at ../glib/gtestutils.c:3256 #2 g_assertion_message_expr() at ../glib/gtestutils.c:3282 #3 meta_stack_tracker_restack_managed() at ../src/core/stack-tracker.c:1210 #4 on_stack_changed() at ../src/core/stack.c:142 #5 _g_closure_invoke_va() at ../gobject/gclosure.c:895 #6 g_signal_emit_valist() at ../gobject/gsignal.c:3456 #7 g_signal_emit() at ../gobject/gsignal.c:3606 #8 meta_stack_changed() at ../src/core/stack.c:265 #9 meta_stack_remove() at ../src/core/stack.c:324 #10 meta_window_unmanage() at ../src/core/window.c:1542 #11 meta_x11_display_unmanage_windows() at ../src/x11/meta-x11-display.c:111 #12 meta_x11_display_dispose() at ../src/x11/meta-x11-display.c:141 #13 g_object_run_dispose() at ../gobject/gobject.c:1448 #14 meta_display_shutdown_x11() at ../src/core/display.c:831 The added test specifically checks that this scenario is handled gracefully. Related: https://bugzilla.redhat.com/show_bug.cgi?id=2143637 Part-of: <https://gitlab.gnome.org/GNOME/mutter/-/merge_requests/2704>

mmstick changed the title ~~✨ Add "Don't emit size-changed" patch~~ Add "Don't emit size-changed" patch Jun 11, 2019

mmstick requested a review from brs17 June 11, 2019 22:52

mmstick force-pushed the master_disco branch from b16dc75 to 4572156 Compare June 11, 2019 23:19

mmstick force-pushed the window-drag-cpu-reduction_disco branch from 6760433 to 4f15e16 Compare June 11, 2019 23:20

Add "Don't emit size-changed" patch

761a0f3

Reduces CPU usage by 22% when dragging windows. Upstream: https://gitlab.gnome.org/GNOME/mutter/merge_requests/568

mmstick force-pushed the window-drag-cpu-reduction_disco branch from 4f15e16 to 761a0f3 Compare June 12, 2019 15:51

brs17 requested a review from jackpot51 June 13, 2019 15:08

Merge branch 'master_disco' into window-drag-cpu-reduction_disco

093e2a8

mmstick merged commit d12942a into master_disco Jun 18, 2019

jackpot51 deleted the window-drag-cpu-reduction_disco branch June 25, 2019 14:51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add "Don't emit size-changed" patch #1

Add "Don't emit size-changed" patch #1

mmstick commented Jun 11, 2019

mmstick commented Jun 14, 2019

Add "Don't emit size-changed" patch #1

Add "Don't emit size-changed" patch #1

Conversation

mmstick commented Jun 11, 2019

mmstick commented Jun 14, 2019