mate-desktop / mate-power-manager

Power management tool for the MATE desktop
https://mate-desktop.org
GNU General Public License v2.0
59 stars 51 forks source link

Fix brightness applet scroll #383

Open balazs-endresz opened 1 year ago

balazs-endresz commented 1 year ago

This PR makes it possible to use the mouse wheel scroll to adjust brightness when hovering over the applet icon (without opening the popup).

There was code that was supposed to handle these scroll events already but it repeatedly bumped it by one percent, which only caused very high cpu usage, while most changes didn't even take effect.

This PR reuses gpm_applet_slide_cb and gpm_applet_slide_delayed_cb to throttle all adjustments regardless of how they were triggered.

One thing I don't fully understand is why we get a 10% change when scrolling over the slider in the popup. I don't see where that's specified, maybe that's the GTK default.

Also, the scroll event isn't triggered always/everywhere as the description in gpm_applet_scroll_cb suggests. If the popup is open then scroll doesn't work over the applet icon, or outside the slider area in the popup. That's a problem without this PR as well. I couldn't make it work but happy to try if anyone has suggestions.

I've tested this only on Ubuntu Mate 22.04 / Thinkpad T480.

raveit65 commented 1 year ago

The applet crashes when i try to scroll over the applet icon and i get this general protection fault from my kernel backtrace.

traps: mate-brightness[3833] general protection fault ip:7f91fa4341c8 sp:7ffe8355b3a8 error:0 in libgobject-2.0.so.0.7600.2[7f91fa407000+36000]
[rave@satellite Mate]$ uname -r
6.2.13-300.fc38.x86_64
[rave@satellite Mate]$ rpm -qa glib2
glib2-2.76.2-1.fc38.x86_64

Hardware: never Thinkbook with amd graphics. Steps to reproduce (always):

  1. Try scrolling over the applet icon after session start or loading the applet to the panel, boom....
  2. Reload the applet to panel and try again 2-3 times the applet doesn't crash anymore but nothing happens when scrolling.
  3. Scroll the brightness with the slider which is working well.
  4. Scroll the brightness again over the applet icon, ...........now it works.

When i first scroll with the slider and second over the icon it works well and i got no crashes.

raveit65 commented 1 year ago

One thing I don't fully understand is why we get a 10% change when scrolling over the slider in the popup. I don't see where that's specified, maybe that's the GTK default.

Not sure, but maybe it is defined as a general step width. https://github.com/mate-desktop/mate-power-manager/blob/master/src/gpm-brightness.c#L105 Or at another place in this file.

balazs-endresz commented 1 year ago

That general protection fault sounds pretty bad. Do you think it might be a hardware specific issue, or should I try to reproduce it with Quickemu (and Fedora I guess)?

I've got these on my machine btw:

$ uname -r
5.15.0-67-generic
$ pkg-config --modversion glib-2.0
2.72.4
raveit65 commented 1 year ago

I don't think it is hardware specific, because scrolling over the applet works fine when i first use scrolling over the slider. So it looks like a problem with the new code. And fedora kernel is compiled that it use general protection fault. I did noticed that with another Mate bug a while ago. Don't know if kernels from other distros do the same.

raveit65 commented 1 year ago

I tried again and now i got a backtrace from the applet. Crash happens when i used the scrollwheel. Also GtkRange is mentioned in trace.

[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib64/libthread_db.so.1".
Core was generated by `/usr/libexec/mate-brightness-applet'.
Program terminated with signal SIGSEGV, Segmentation fault.
warning: Section `.reg-xstate/28748' in core file too small.
#0  0x00007f0dc6c6eeee in gtk_range_set_value (range=0x55c3c15b38d0, value=0) at ../gtk/gtkrange.c:1554
1554      g_return_if_fail (GTK_IS_RANGE (range));
[Current thread is 1 (Thread 0x7f0dc50c3680 (LWP 28748))]

Thread 1 (Thread 0x7f0dc50c3680 (LWP 28748)):
#0  0x00007f0dc6c6eeee in gtk_range_set_value (range=0x55c3c15b38d0, value=0) at ../gtk/gtkrange.c:1554
        __inst = 0x55c3c15b38d0
        __r = <optimized out>
        _g_boolean_var_34 = <optimized out>
        priv = <optimized out>
        __func__ = "gtk_range_set_value"
#1  0x000055c3bfa0d74b in gpm_applet_scroll_cb ()
#2  0x00007f0dc6a99f51 in _gtk_marshal_BOOLEAN__BOXED (closure=0x55c3c154ee10, return_value=0x7ffed9795610, n_param_values=<optimized out>, param_values=0x7ffed9795670, invocation_hint=<optimized out>, marshal_data=<optimized out>) at gtk/gtkmarshalers.c:84
        cc = 0x55c3c154ee10
        data1 = 0x55c3c150e320
        data2 = <optimized out>
        callback = 0x55c3bfa0d700 <gpm_applet_scroll_cb>
        v_return = <optimized out>
        __func__ = "_gtk_marshal_BOOLEAN__BOXED"
#3  0x00007f0dc65864ea in g_closure_invoke (closure=0x55c3c154ee10, return_value=0x7ffed9795610, n_param_values=2, param_values=0x7ffed9795670, invocation_hint=0x7ffed97955f0) at ../gobject/gclosure.c:832
        marshal = 0x7f0dc6a99ee0 <_gtk_marshal_BOOLEAN__BOXED>
        marshal_data = 0x0
        in_marshal = 0
        real_closure = 0x55c3c154edf0
        __func__ = "g_closure_invoke"
#4  0x00007f0dc65b4d36 in signal_emit_unlocked_R.isra.0 (node=<optimized out>, detail=detail@entry=0, instance=instance@entry=0x55c3c150e320, emission_return=emission_return@entry=0x7ffed9795780, instance_and_params=instance_and_params@entry=0x7ffed9795670) at ../gobject/gsignal.c:3812
        tmp = <optimized out>
        handler = 0x55c3c154edb0
        accumulator = 0x55c3c14d20e0
        emission = {next = 0x0, instance = 0x55c3c150e320, ihint = {signal_id = 82, detail = 0, run_type = (G_SIGNAL_RUN_FIRST | G_SIGNAL_ACCUMULATOR_FIRST_RUN)}, state = EMISSION_RUN, chain_type = 0x4}
        handler_list = 0x55c3c154edb0
        return_accu = 0x7ffed9795610
        accu = {g_type = 0x14, data = {{v_int = 0, v_uint = 0, v_long = 0, v_ulong = 0, v_int64 = 0, v_uint64 = 0, v_float = 0, v_double = 0, v_pointer = 0x0}, {v_int = 0, v_uint = 0, v_long = 0, v_ulong = 0, v_int64 = 0, v_uint64 = 0, v_float = 0, v_double = 0, v_pointer = 0x0}}}
        signal_id = 82
        max_sequential_handler_number = 258
        return_value_altered = <optimized out>
#5  0x00007f0dc65a5702 in g_signal_emit_valist (instance=<optimized out>, signal_id=<optimized out>, detail=<optimized out>, var_args=var_args@entry=0x7ffed9795830) at ../gobject/gsignal.c:3575
        return_value = {g_type = 0x14, data = {{v_int = 0, v_uint = 0, v_long = 0, v_ulong = 0, v_int64 = 0, v_uint64 = 0, v_float = 0, v_double = 0, v_pointer = 0x0}, {v_int = 0, v_uint = 0, v_long = 0, v_ulong = 0, v_int64 = 0, v_uint64 = 0, v_float = 0, v_double = 0, v_pointer = 0x0}}}
        error = 0x0
        rtype = 0x14
        static_scope = 0
        instance_and_params = <optimized out>
        signal_return_type = <optimized out>
        param_values = <optimized out>
        node = <optimized out>
        i = <optimized out>
        n_params = <optimized out>
        __func__ = "g_signal_emit_valist"
#6  0x00007f0dc65a5e53 in g_signal_emit (instance=instance@entry=0x55c3c150e320, signal_id=<optimized out>, detail=detail@entry=0) at ../gobject/gsignal.c:3622
        var_args = {{gp_offset = 32, fp_offset = 48, overflow_arg_area = 0x7ffed9795910, reg_save_area = 0x7ffed9795850}}
#7  0x00007f0dc6d6f9f4 in gtk_widget_event_internal.part.0.lto_priv.0 (widget=0x55c3c150e320, event=0x55c3c16e33e0) at ../gtk/gtkwidget.c:7812
        signal_num = <optimized out>
        return_val = <optimized out>
        handled = 0
#8  0x00007f0dc6c07e10 in propagate_event_up (topmost=<optimized out>, event=<optimized out>, widget=0x55c3c150e320) at ../gtk/gtkmain.c:2588
        tmp = <optimized out>
        handled_event = <optimized out>
        handled_event = 0
#9  propagate_event (widget=widget@entry=0x55c3c150e320, event=event@entry=0x55c3c16e33e0, captured=captured@entry=0, topmost=topmost@entry=0x0) at ../gtk/gtkmain.c:2691
        handled_event = 0
#10 0x00007f0dc6c07f3f in gtk_propagate_event (widget=widget@entry=0x55c3c150e320, event=event@entry=0x55c3c16e33e0) at ../gtk/gtkmain.c:2725
        __func__ = "gtk_propagate_event"
#11 0x00007f0dc6c089ba in gtk_main_do_event (event=0x55c3c16e33e0) at ../gtk/gtkmain.c:1921
        grab_widget = 0x55c3c150e320
        window_group = 0x55c3c15b6ea0
        rewritten_event = <optimized out>
        device = <optimized out>
        tmp_list = <optimized out>
        event_widget = 0x55c3c150e320
        topmost_widget = <optimized out>
        __func__ = "gtk_main_do_event"
#12 gtk_main_do_event (event=<optimized out>) at ../gtk/gtkmain.c:1691
        __func__ = "gtk_main_do_event"
#13 0x00007f0dc6948677 in _gdk_event_emit (event=0x55c3c16e33e0) at ../gdk/gdkevents.c:73
#14 _gdk_event_emit (event=0x55c3c16e33e0) at ../gdk/gdkevents.c:67
#15 0x00007f0dc699a4ee in gdk_event_source_dispatch.lto_priv () at ../gdk/x11/gdkeventsource.c:354
#16 0x00007f0dc648539c in g_main_dispatch (context=0x55c3c149bfa0) at ../glib/gmain.c:3460
        dispatch = 0x7f0dc699a4c0 <gdk_event_source_dispatch.lto_priv>
        prev_source = 0x0
        begin_time_nsec = 7575453853853
        was_in_call = 0
        user_data = 0x0
        callback = 0x0
        cb_funcs = 0x0
        cb_data = 0x0
        need_destroy = <optimized out>
        source = 0x55c3c149bc40
        current = 0x55c3c14c1d10
        i = 0
#17 g_main_context_dispatch (context=0x55c3c149bfa0) at ../glib/gmain.c:4200
#18 0x00007f0dc64e3438 in g_main_context_iterate.isra.0 (context=0x55c3c149bfa0, block=1, dispatch=1, self=<optimized out>) at ../glib/gmain.c:4276
        max_priority = 0
        timeout = 0
        some_ready = 1
        nfds = 4
        allocated_nfds = <optimized out>
        fds = <optimized out>
        begin_time_nsec = 7575453827063
#19 0x00007f0dc648499f in g_main_loop_run (loop=0x55c3c1508350) at ../glib/gmain.c:4479
        __func__ = "g_main_loop_run"
#20 0x00007f0dc6c06305 in gtk_main () at ../gtk/gtkmain.c:1329
        loop = 0x55c3c1508350
#21 0x00007f0dc71f5e25 in _mate_panel_applet_factory_main_internal (factory_id=0x55c3bfa0f364 "BrightnessAppletFactory", out_process=1, applet_type=<optimized out>, callback=<optimized out>, user_data=0x0) at /usr/src/debug/mate-panel-1.27.1-2.fc38.x86_64/libmate-panel-applet/mate-panel-applet.c:2443
        factory = 0x55c3c1507eb0
        closure = 0x55c3c1507ab0
        __func__ = "_mate_panel_applet_factory_main_internal"
#22 0x000055c3bfa0ce7e in main ()

Full backtrace at https://www.dropbox.com/s/5ssln0oeqnt71mi/brightness-appet-crashes_backtrace?dl=0 Maybe this helps to debug the problem. Note, this is on a box without brighness control. I will test it with my thinkbook and fedora too, but here i need to add the patch to mpm-1.26.