After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 639765 - a11y: visual alert only works per-window, not screen
a11y: visual alert only works per-window, not screen
Status: RESOLVED FIXED
Product: mutter
Classification: Core
Component: general
unspecified
Other Linux
: Normal normal
: ---
Assigned To: mutter-maint
mutter-maint
[gnome3-important]
Depends on:
Blocks:
 
 
Reported: 2011-01-17 17:21 UTC by Matthias Clasen
Modified: 2011-03-18 17:55 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
bell: add a composited flash-screen function (3.97 KB, patch)
2011-03-18 13:59 UTC, Dan Winship
committed Details | Review

Description Matthias Clasen 2011-01-17 17:21:22 UTC
there's a setting to choose whether visual alerts (ie bell) should flash just the window border or the entire screen.

Flashing the border works, flashing the screen doesn't. iirc, this used to work in metacity.
Comment 1 Matthias Clasen 2011-01-20 19:06:12 UTC
The relevant function that indeed does work in metacity, but doesn't in mutter, is
src/core/bell.c:bell_flash_screen
Comment 2 André Klapper 2011-03-03 21:16:00 UTC
[Removing GNOME3.0 target as decided in release-team meeting on March 03, 2011.
This report has an "important" categorisation for GNOME3.0 but is not considered a hard blocker. For querying use the corresponding whiteboard entry added.]
Comment 3 Dan Winship 2011-03-18 13:59:11 UTC
Created attachment 183719 [details] [review]
bell: add a composited flash-screen function

The old bell_flash_screen() has no effect when compositing. Add
meta_compositor_flash_screen(), and use that instead.

====

easiest way I found to test it was 'printf "\a"' in a terminal. I played
around with the timing, etc a bit to get something that looked good.
Darkening all the way to full opacity seemed noticeably more distracting.

EASE_IN on the second half is so we get
  __           _
 /  \         / |
|    |  not  |   \_
Comment 4 Colin Walters 2011-03-18 14:05:39 UTC
Review of attachment 183719 [details] [review]:

::: src/compositor/compositor.c
@@ +1189,3 @@
 }
+
+#define FLASH_TIME 50

Can you add a unit to this? In this case, _MS ?

@@ +1224,3 @@
+  clutter_actor_set_size (flash, width, height);
+  clutter_actor_set_opacity (flash, 0);
+                         "signal-after::completed", flash_out_completed, flash,

We're relying here on the fact that the actor will be on top after the add, by default? 

While clearly it'd be unlikely for something else to appear in the 50 milliseconds, I can see a libmutter consumer like gnome-shell wanting to put the flash in an explicit layer, which I guess would mean this would just be a signal and we'd implement it in gnome-shell.

It's not a big deal, just pointing this out in case you think it's motivation enough to change it.
Comment 5 Dan Winship 2011-03-18 14:25:31 UTC
(In reply to comment #4)
> We're relying here on the fact that the actor will be on top after the add, by
> default? 

Hm... I just didn't think to add a raise(), and it worked without it. Probably should put it there to be safe.

> While clearly it'd be unlikely for something else to appear in the 50
> milliseconds, I can see a libmutter consumer like gnome-shell wanting to put
> the flash in an explicit layer, which I guess would mean this would just be a
> signal and we'd implement it in gnome-shell.

Ideally this would be part of the plugin interface or something, but I didn't feel like doing that 3 days before hard code freeze. :)

(Also, the shell doesn't put many actors in the stage, other than Main.uiGroup, so...)
Comment 6 Owen Taylor 2011-03-18 15:51:03 UTC
(In reply to comment #5)
> (In reply to comment #4)
> > We're relying here on the fact that the actor will be on top after the add, by
> > default? 
> 
> Hm... I just didn't think to add a raise(), and it worked without it. Probably
> should put it there to be safe.

It's defined that adding something to a group is on top by default.
 
> > While clearly it'd be unlikely for something else to appear in the 50
> > milliseconds, I can see a libmutter consumer like gnome-shell wanting to put
> > the flash in an explicit layer, which I guess would mean this would just be a
> > signal and we'd implement it in gnome-shell.
> 
> Ideally this would be part of the plugin interface or something, but I didn't
> feel like doing that 3 days before hard code freeze. :)
> 
> (Also, the shell doesn't put many actors in the stage, other than Main.uiGroup,
> so...)

Since the flash is uniform across the whole screen and brief, I don't think we need to worry about mangnifiying it or the interaction with the magnified cursor. I think if we wanted to do better, we'd make this hookable, but I'm not sure that the plugin interface is the right place/way to do hooks moving forward. We shouldn't have to funnel every hook point through some random interface.
Comment 7 Dan Winship 2011-03-18 17:55:39 UTC
Attachment 183719 [details] pushed as 3597035 - bell: add a composited flash-screen function