Dropsdl #1175

Closed
wants to merge 25 commits into
from

Projects

None yet

6 participants

@elupus
Team Kodi member

This pull request rids us of SDL for events and window system.

I'm a bit hesitant to the fix for separate screen's. It get a bit messy with extending resolution structure for this. Wasn't sure what the call the value also it's still broken since it need a skin refresh since we apparently have not implemented OnDeviceReset() callbacks on GLX systems.

This tries to replace #848, but it isn't fully complete. So @FernetMenta may have to re-add anything that i've missed.

It clearly need testing.

@theuni
Team Kodi member

Nice! Drop it from the tallest building... :)

(Only glanced through it, not qualified to comment on the X stuff)

@mkortstiege
Team Kodi member

There's a problem with the title and icon here. Within the task-switcher (alt-tab) the title is "unkown" and no icon is shown. Maybe its a gnome-shell issue, not sure yet.

@elupus
Team Kodi member
@mkortstiege
Team Kodi member

That's odd :)

@FernetMenta
Team Kodi member

There's a couple of things obviously got broken. e.g.

  • toggle full screen, can't size window in horizontal direction
  • XBMC icon broken: caused by a change in CTexture
  • minimize window: alt-tab: XBMC stays in background instead of getting minimized

A couple of others which I had already fixed:

  • listen to external xrandr events and take action: consider a user who starts XBMC, then turns on an av receiver or projector. This version does not update resolutions and can't switch to the new output
  • gui corruption on AMD is back. The correct sequence is to first change resolution, then refresh window on xrandr event. There was a reason why I re-created the window on this event.
  • It must be distinguished between systems running a window manager and systems which do not. If no WM is present the redirect_override flag has to be set instead of dealing with NET_WM*. Not doing this can decrease performance on NVdias ystems running vdpau.
  • I don't see the patch for the screensaver, If the scrren has switched off due to inactivity XBMC won't be able to activate the screen again.

I'm a bit hesitant to the fix for separate screen's.

This is the use case which is even more likely than having a single screen. On NVidia systems you get tearing when having two monitors connected to a single screen.

It get a bit messy with extending resolution structure for this

Why? No need to load resolutions of a screen where XBMC is not displayed.

@elupus
Team Kodi member
@FernetMenta
Team Kodi member

Don't have time to answer the other points, but this I know for sure as it was tested and proved. Weird but this is like it works in reality. (There are a couple of hundred users tested this already and I collected feedback)

A tester:

Finally:
"swa.override_redirect = False;" gives many drops, how describes above.

"swa.override_redirect = fullscreen ? True : False;" gives smooth picture in fullscreen.

I tested twice.

@elupus
Team Kodi member
@FernetMenta
Team Kodi member

Right. He is using a windowmanager, which is redirecting even fullscreen windows. (many do). So we might have to go that route for all situation. I was just saying that it would not make sense to only do it for the non windowmanager case. It's the with windowmanager case that is problematic.

No, he was no WM (OpenElec). I have solved it by detecting the presents of a WM with this function:
FernetMenta@02289bb

BTW: setting the overrive_redirect flag when a WM is present (like KDE), then it kills performance in that case.

@FernetMenta
Team Kodi member

Testing on Ubuntu 12.04 with Unity and Fluxbox:

1)
toggle full-screen:
Unity:
When switching to windowed mode XBMC is horizontally stretched, docked at the menu bar and to the right border of the screen. I can resize and move the window only in vertical direction.
Fluxbox:
The origin of XBMC is outside the visible area of the screen, somewhere behind the upper left. I can resize the window in both direction but not move because the title bar is outside the screen.

2)
alt-tab
This is not working when XBMC is full-screen. When hitting alt-tab XBMC becomes the wall paper in the background. Same behavior on Unity and Fluxbox.

Because that is not how all other arches work. I don't mind changing it, but it must be done for all arches. I don't want a bunch of special cases for X11.

Different architectures have different functions and terminology. Why confusing the user with naming an output a screen just for the sake it is called a screen on a different platform.

@elupus elupus and 1 other commented on an outdated diff Jul 19, 2012
xbmc/windowing/WinEventsX11.cpp
+ {
+ break;
+ }
+
+ for (unsigned int i = 0; i < keys.length() - 1; i++)
+ {
+ newEvent.key.keysym.sym = XBMCK_UNKNOWN;
+ newEvent.key.keysym.unicode = keys[i];
+ newEvent.key.state = xevent.xkey.state;
+ newEvent.key.type = xevent.xkey.type;
+ ret |= ProcessKey(newEvent, 500);
+ }
+ if (keys.length() > 0)
+ {
+ newEvent.key.keysym.scancode = xevent.xkey.keycode;
+ xkeysym = XLookupKeysym(&xevent.xkey, 0);
@elupus
elupus Jul 19, 2012

@FernetMenta Could you explain this? Shouldn't Xutf8LookupString should have filled this in earilier?

@FernetMenta
FernetMenta Jul 19, 2012

That's what I have expected too in the first iteration but it broke behavior to what we had with SDL. SDL calls a depreciated function at this place: XKeycodeToKeysym. So I replaced this with XLookupKeysym.
The problem was key combinations like ctrl-s or ctrt-S. Without XLookupKeysym the character was always recognized as lower case (or upper case, don't remember exactly. So user defined keycodes in keyboard.xml were broken.

@elupus
elupus Jul 19, 2012

So we never want to use what Xutf8... has returned then? Ie i can move that call to right after with a comment?

@FernetMenta
FernetMenta Jul 19, 2012

We want to use the string we get from Xutf8... but not the keysym

@elupus
elupus Jul 19, 2012
@FernetMenta
FernetMenta Jul 19, 2012

Sorry, didn't get this right. Sounds like a good idea.

@elupus
Team Kodi member

some further along. i've hooked up running xbmc with override_redirect when run as standalone. it's really messy to use since you have to handle input focus yourself.

It should be possible to only use override_redirect in fullscreen only, like was done before. That might be for the best since so many window manager are slow otherwise.

@FernetMenta
Team Kodi member

Not sure if the issues mentioned above are supposed to be fixed. alt-tab and toggle fullscreen do still not work.

@elupus
Team Kodi member

I assume you where not running standalone? (that is broken in this anyway). What window manager where you using? cause i don't even get that keypress when running under unity. it swallows it and handles it itself.

@FernetMenta
Team Kodi member

I tested on Ubuntu 12.04 with Unity 2D and Fluxbox. Still the same behavior as described above.

@mkortstiege
Team Kodi member

What's the status here? Are we going to drop SDL before frodo?

@elupus
Team Kodi member
FernetMenta and others added some commits May 20, 2012
@FernetMenta FernetMenta X11: add SDL joystick until we have a better solution 9aef2c8
@FernetMenta FernetMenta X11: Add xbmc icon
This does not take icon transparancy into account, so
could look weird.
0a51b45
@elupus elupus X11: factor out code that reset device after lost 5b6caeb
@elupus elupus X11: move handling of xrandr events 18311d8
@elupus elupus X11: support multiple xserver screens c7c6cb2
@elupus elupus X11: support multiple xrandr displays 0b80d65
@elupus elupus X11: replace SDL window and event handling with native xlib 25decc3
@elupus elupus X11: simplify set and reset of locale 886280b
@elupus elupus X11: XWMHints must be allocated by XAllocVMHints since it can be exte…
…nded
d33967f
@FernetMenta FernetMenta add missing keys to xbmc keytable a9f2e95
@elupus elupus X11: support rotated screens. fd0e37a
@elupus elupus [win32] move m_nScreen windowing variable to only system using it. 4988dbd
@elupus elupus X11: re-evaluate vsync method after having modified window
It could have moved between outputs on different hardware
bfef61d
@elupus elupus x11: refactor WinEventsX11 to be a singleton class
Also
X11: fix mixup of what XBMC_KeyboardEvent state/type should contain
X11: allow fallback to XLookupString to also support long strings
b87b5ff
@elupus elupus X11 (squash): don't resize window after (full)screen switch if we hav…
…e wm

Resizing can trigger VM to set window as maximized or other.
5dee1c7
@elupus elupus X11: use override_redirect if window manager doesn't support full scr…
…een or standalone
daa9d27
@elupus elupus X11: make sure we set resolution before we create window
Unity messes up on resolution switches and fails to re-fullscreen
937edd9
@elupus elupus debug stuff 99e7201
@elupus elupus squash into drop sdl 9aa3f92
@elupus
Team Kodi member

So rebased, not all squashed together yet. I'm not fully happy with it yet, but @FernetMenta would be nice with some testing and feedback on what works and what doesn't.

It should now detect if we have a window manager that support fullscreen. If not it uses override_redirect. It will also switch to that if you run it with --standalone switch.

@elupus
Team Kodi member

Some known things I've not resolved yet.
1. Alt+tab when override redirect is used (we must handle Window switches ourselves)
2. Switching between x11 screen lead to loss of textures. I wonder if one can avoid the texture loss by moving the Window without recreating it. It aught to work for when both are in same gfx at least.

@theuni
Team Kodi member

@elupus worth mentioning that i'm in the middle of a complete rewrite for egl, which will be followed by a rewrite of rendering. The goal is to get a clean abstraction where everything is dynamic as possible, meaning that you could run the same binary on X, wayland, fbdev, etc, and switch between gl/gles/directfb/etc all on the fly.

I'm getting reasonably far along, I'll push it up somewhere once it runs on a few platforms.

I don't think it's directly relevant here (except for the egl+X case), but I figured I'd throw it out there.

@theuni
Team Kodi member

Whew, I wrote my comment, then like a genius hit the green 'merge pull request' button. Thanks for making me verify, Github!

@FernetMenta
Team Kodi member

Toggle full screen and minimize do not work on my system. Will investigate ...

@elupus
Team Kodi member
@FernetMenta
Team Kodi member

I had to fix a compile error in WinEventsX11: CApplicationMessenger.

When started in windowed mode togglefullscreen works, it does not the other way round. It becomes windowed but with size of full screen. No chance to resize then.

@elupus
Team Kodi member

Hehe, i thought you actually did merge it :)

@elupus
Team Kodi member

Right.. that would mean that our resolution setup for windowed is getting b0rk some where. it should be respecting that.

@FernetMenta FernetMenta and 1 other commented on an outdated diff Sep 23, 2012
xbmc/windowing/WinEventsX11.cpp
+ return true;
+
+ default:
+ return false;
+ }
+ }
+ return false;
+}
+
+bool CWinEventsX11::ProcessKeyRepeat()
+{
+ if (WinEvents && (m_lastKey.type == XBMC_KEYDOWN))
+ {
+ if (m_repeatKeyTimeout.IsTimePast())
+ {
+ return ProcessKey(m_lastKey, 10);
@FernetMenta
FernetMenta Sep 23, 2012

Should we set this to 100ms and/or make it configurable by advancedsettings? I see no reason why this is 10ms. MCE remotes registered as keyboards don't work that well.

@elupus
elupus Sep 23, 2012

I have found some code that actually uses the X11 key-repeats instead of generating them ourselves. I think that is much better.

@elupus
elupus Sep 23, 2012

Check the ProcessKeyRelease. That will ignore keyup's that are actually key repeats.

@Memphiz
Team Kodi member

The problem is not the green button - its the blue one afterwards ;)

@FernetMenta
Team Kodi member

Right.. that would mean that our resolution setup for windowed is getting b0rk some where. it should be respecting that.

resolution is ok, it just gets not set. SetFullScreen just resets NET_WM_STATE_FULLSCREEN without doing a ResizeWindow.

elupus added some commits Sep 23, 2012
@elupus elupus X11: real XConfigureEvents have position relative parent 0ac9a81
@elupus elupus squash fix compile error 2e2db91
@elupus elupus squash somewhere
Don't re-write left/top in fullscreen since that is only way of
knowing normal window size.
8b33140
@elupus elupus X11: create _NET_WM_STATE_FULLSCREEN windows with windowed size
Window manager uses created size to store original size before going
fullscreen. This makes it possible to restore size after fullscreen
again.
f51b37d
@elupus
Team Kodi member

The problem was that the WM uses the XCreateWindow size when it restores from fullscreen. So we must create the window with a windowed size, and let the WM do it fullscreening job

@elupus
Team Kodi member

Could you try with mce remotes now and see if it works allright?

@FernetMenta
Team Kodi member

ToggleFullScreen still does not work on my system. Ubuntu 12.04 with Ubuntu 2D. Window width/height is always 1855x1056 when switching from fullscreen to windowed (starting xbmc fullscreen).
The window attributes seem to be unreliable. Why not explicitly resize the window when entering windowed mode?

@FernetMenta
Team Kodi member

MCE remote seems to work fine.

@elupus
Team Kodi member
@FernetMenta
Team Kodi member

I remember I had trouble with that too. Once fullscreen I found no reliable way to get back in windowed mode and ended up in recreating the window. SDL used 3 windows, mapped/unmapped the fullscreen and wm window, and reparented the gl window. I preferred to have a single window.

@MartijnKaijser
Team Kodi member

see #3505

@tru tru added a commit to plexinc/plex-home-theater-public that referenced this pull request May 19, 2014
@tru tru Fix black screen when music screensaver is activated.
This was due to the fact that XBMC code always called
ActivateWindow(WINDOW_VIS) when we wanted to call ActivateVisualizer()
which handles NOW_PLAYING and vis.

Fixes #1175
d028dab
@tru tru added a commit to RasPlex/plex-home-theatre that referenced this pull request Aug 21, 2014
@tru tru Fix black screen when music screensaver is activated.
This was due to the fact that XBMC code always called
ActivateWindow(WINDOW_VIS) when we wanted to call ActivateVisualizer()
which handles NOW_PLAYING and vis.

Fixes #1175
542ce83
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment