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

-i (inactive_opacity) does not work on windows just mapped & focused #39

Closed
richardgv opened this issue Sep 12, 2012 · 11 comments
Closed

Comments

@richardgv
Copy link
Collaborator

This is a pretty simple problem. I created an issue for it just because my fix is ugly and possibly slow and I wonder if you guys have a better way.

The problem:

If a window gets mapped and instantly gets focus, compton will not notice it. When using -i (inactive_opacity), the window will be rendered as inactive. To reproduce it, start compton with -i, minimize a full screen window, then restore it.

The fix:

My fix will be committed to richardgv-dev branch soon. A description of the my guess about the cause is included in comments:

/*
 * Occasionally compton does not seem able to get a FocusIn event from a
 * window just mapped. I suspect it's a timing issue again when the
 * XSelectInput() is called too late. If this is the case, I could think
 * of two fixes: To monitor the focus events from the root window, and
 * to determine if the current window is focused in map_win(). Looks
 * like the XFocusChangeEvent sent to the root window contains no
 * information about where the WM frame of the focused window is, and 
 * XGetInputFocus() often returns an application window instead of the
 * WM frame, which compton keeps track of, in either way I believe we
 * have to travel through the ancestors of the focused window it
 * returns. The latter choice looks cheaper, so I'm doing it here.
 * But still, this could anyway be costly.
 *
 * An alternative route might be relying on _NET_ACTIVE_WINDOW.
 * Unfortunately as it's set by WM I'm not completely sure if it's
 * reliable and will be updated on the very moment a window is mapped.
 */

The problem of the fix:

I guess it would be slow. Querying X server for many times is not the fastest thing in the world.

richardgv added a commit that referenced this issue Sep 12, 2012
More info in the issue description. This also fixes the problem for
--inactive-dim.
@chjj
Copy link
Owner

chjj commented Sep 12, 2012

  • Occasionally compton does not seem able to get a FocusIn event from a
    • window just mapped. I suspect it's a timing issue again when the
    • XSelectInput() is called too late.

So, just to make sure I'm understanding... A window gets mapped, compton calls XSelectInput, but the XSelectInput actually completes after the FocusIn event for the window is fired? That is tricky. This seems kind of messy though. It'd be really nice to find a cleaner way. Maybe we should put this fix on hold.

By the way, I do really like the idea of a focused member on the win struct as it allows for some other interesting functionality to be added.

@richardgv
Copy link
Collaborator Author

So, just to make sure I'm understanding... A window gets mapped, compton calls XSelectInput, but the XSelectInput actually completes after the FocusIn event for the window is fired? That is tricky. This seems kind of messy though. It'd be really nice to find a cleaner way. Maybe we should put this fix on hold.

Yeah, that's what I suspect. X maps a window then immediately emits a FocusIn event, before compton has time to execute XSelectInput() to capture the event. It's a guess, nevertheless.

The code looks somehow messy, indeed. Hmm, if there's indeed a cleaner fix... I don't even know where I should ask a question about Xlib? A mailing list?

By the way, I'm using a similar messy piece of code to determine the currently focused window on startup in the opacity bug fix, the sole difference is it utilizes an array of the children of the root window just returned above for better performance.

I did another commit, stopping compton from tracking focus changes if neither inactive_opacity or inactive_dim is enabled, minimizing the performance impact.

By the way, I do really like the idea of a focused member on the win struct as it allows for some other interesting functionality to be added.

:-) I'm actually thinking about applying binomial / gaussian blur to inactive windows, yet it's probably not practically very useful, and much harder to perfectly implement. What are the functionalities you are thinking about, if you don't mind me asking?

@chjj
Copy link
Owner

chjj commented Sep 13, 2012

The code looks somehow messy, indeed. Hmm, if there's indeed a cleaner fix... I don't even know where I should ask a question about Xlib? A mailing list?

Hehe, not sure. I never had strong Xlib knowledge before this project either. All the knowledge I have came from me staring at the original xcompmgr code and reading whatever documentation and/or old mailing list posts I could find on xlib/xrender.

What are the functionalities you are thinking about, if you don't mind me asking?

That's just me being vague. Before, for code in paint_all for example, there was no reasonable way to tell if a window was focused or not since compton didn't store the focus. So it just allows for the logic to be arranged differently. I like it.

@richardgv
Copy link
Collaborator Author

Huh, sorry, I never realized compton actually maintains a field called client_win... I've rewritten the fix to utilize it, so it should look much cleaner right now.

An issue is client_win is frequently not reliable. I discovered with surprise that -e actually does not work for many newly created windows here. A little debugging reveals compton cannot get the width of the frame because it can't detect the client window from whose properties it gets frame width. Once I hook into the process with gdb and set up a few breakpoint the issue disappears, so very possibly again this is a timing-related issue. I plan on adding some code trying to fix the situation in ev_reparent_notify(), but not today -- it's closed to midnight here now.

Hehe, not sure. I never had strong Xlib knowledge before this project either. All the knowledge I have came from me staring at the original xcompmgr code and reading whatever documentation and/or old mailing list posts I could find on xlib/xrender.

I never touched Xlib, nor have I written anything serious with C under Linux, before 6 days ago when I started debugging the fvwm window stacking issue. :-) Documentation about Xlib looks extraordinarily rare, xlib manual on Christophe Tronche's website and the *proto.txt explaining X extensions on freedesktop.org are the only rather complete source of information I saw. And they all don't explain much details.

@chjj
Copy link
Owner

chjj commented Sep 13, 2012

Documentation about Xlib looks extraordinarily rare, xlib manual on Christophe Tronche's website and the *proto.txt explaining X extensions on freedesktop.org are the only rather complete source of information I saw.

Yeah, agreed.

@richardgv, I've merged all of your changes up until and changed a few things to keep with the overall coding style. It proved to be very difficult to cherry-pick individual fixes, so everything is going in master now. It'd be nice of you to grab the last commits from master so our branches don't diverge too much.

@chjj
Copy link
Owner

chjj commented Sep 13, 2012

Oh, and a new problem. Not with the inactive opacity changes, but with Awesome. Window managers that don't follow EWMH and don't set the window type properly are going to have a hard time with inactive transparency now. Before, since compton only lessened a window's transparency when it at least had been focused once, window managers like this had no problem. Now, the wibox/panel and menu in Awesome are showing up inactively transparent for me. Compton sees them as normal windows. In other words, the original misbehavior of compton actually helped with WM's that don't set _NET_WM_WINDOW_TYPE properly.

Anyway, @richardgv, thanks for all your hard work. These are all great improvements.

@richardgv
Copy link
Collaborator Author

Oh, and a new problem. Not with the inactive opacity changes, but with Awesome. Window managers that don't follow EWMH and don't set the window type properly are going to have a hard time with inactive transparency now. Before, since compton only lessened a window's transparency when it at least had been focused once, window managers like this had no problem. Now, the wibox/panel and menu in Awesome are showing up inactively transparent for me. Compton sees them as normal windows. In other words, the original misbehavior of compton actually helped with WM's that don't set _NET_WM_WINDOW_TYPE properly.

Menus of fvwm itself has the same issue. Oh, the things we could do are rather limited about the situation. I imagine there are a few choices:

  1. The most optimal solution is of course to detect a window of the window manager with 100% accuracy. But how, considering the amount of window managers could be beyond our wildest guesses? I tried querying the information about a fvwm menu time but no luck in finding a way to identify it perfectly.
  2. Attempt to identify a real normal window by checking certain proprieties -- like WM_STATE? -- or by detecting the reparenting process. I'm not terribly confident about accuracy, should I admit. This should work for normal windows and normal window managers, and what will happen on rather broken ones is what I do not know. And probably this would involve monitoring property changes of many windows, thus some performance penalty.
  3. Consider a window that has never been focused active, unless there's another window that received FocusIn but not yet FocusOut. This should work in some cases, or probably in many cases, but not necessarily in all cases.
  4. Do nothing except recommending against setting aggressive -i or --inactive-dim. It's not compton's fault, after all, so this is reasonable in some degree.

It'd be nice of you to grab the last commits from master so our branches don't diverge too much.

Did it.

@chjj
Copy link
Owner

chjj commented Sep 20, 2012

Attempt to identify a real normal window by checking certain proprieties -- like WM_STATE?

From just experimenting with this in Awesome, I honestly think this might be the best route. Detect normal windows by whether they have a WM_STATE property somewhere. This very quickly fixed all the problems I had with awesome.

richardgv added a commit that referenced this issue Sep 20, 2012
See chjj's comments on issue #39:
#39 (comment)

- Add a switch --mark-wmwin-focused that try to detect WM windows and
  mark them active.

- Fix a bug that causes BadDrawable, etc. if a window is mapped then
  immediately unmapped.

- Fix a bug in determine_evmask().

- Add a debug option DEBUG_CLIENTWIN.

- Force window repaint on window frame extent change.

- Code cleanup.
@richardgv
Copy link
Collaborator Author

From just experimenting with this in Awesome, I honestly think this might be the best route. Detect normal windows by whether they have a WM_STATE property somewhere. This very quickly fixed all the problems I had with awesome.

Implemented. (Do you have another implementation?) Use --mark-wmwin-focused to turn it on. Oops, I forgot to add a description of the new switch in usage(). It works for fvwm menus. Time will prove if this approach is reliable.

@chjj
Copy link
Owner

chjj commented Sep 20, 2012

Do you have another implementation?

Yes, instead of always focusing the non-client-containing windows, I simply changed the IS_NORMAL_WIN macro to ((w) && (w)->client_win). This obviously completely ignores EWMH types for anything that might use this macro, but it seems to work well enough in practice.

edit: Using the method above, it might be wise to only fallback to checking for WM_STATE if _NET_WM_WINDOW_TYPE is not set.

@richardgv
Copy link
Collaborator Author

Yes, instead of always focusing the non-client-containing windows, I simply changed the IS_NORMAL_WIN macro to ((w) && (w)->client_win). This obviously completely ignores EWMH types for anything that might use this macro, but it seems to work well enough in practice.

Yeah, that's more intuitive. My approach is to mark a window as active initially if it has no client window in map_win(). So if the window is wrongly determined as a WM window, once it receives a FocusOut, it "becomes" a normal window automatically.

Fat-Zer pushed a commit to Fat-Zer/tdebase that referenced this issue Apr 5, 2014
See chjj's comments on issue #39:
chjj/compton#39 (comment)

- Add a switch --mark-wmwin-focused that try to detect WM windows and
  mark them active.

- Fix a bug that causes BadDrawable, etc. if a window is mapped then
  immediately unmapped.

- Fix a bug in determine_evmask().

- Add a debug option DEBUG_CLIENTWIN.

- Force window repaint on window frame extent change.

- Code cleanup.
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

No branches or pull requests

2 participants