Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix brightness applet scroll #383

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

balazs-endresz
Copy link

@balazs-endresz balazs-endresz commented Oct 31, 2022

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.

Copy link
Member

@lukefromdc lukefromdc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Builds and runs fine, had to transfer this to the laptop to test it

Works well, note that it can ONLY be tested on laptops because the effect of this is to adjust the brightness when scrolling over the applet WITHOUT showing the slider. On a desktop (no brightness control available), the slider works without doing anything but since the slider is not shown for scrolling over the applet this has no effect when brightness cannot be adjusted.

Only build warnings I got on rebuilding just the applet were the usual deprecation warnings, none of them were new.

@lukefromdc lukefromdc requested a review from a team November 5, 2022 18:26
@raveit65
Copy link
Member

raveit65 commented May 1, 2023

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
Copy link
Member

raveit65 commented May 1, 2023

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
Copy link
Author

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
Copy link
Member

raveit65 commented May 1, 2023

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
Copy link
Member

raveit65 commented Jun 2, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants