Bug 1249

Summary: NEWT X11: setVisible(false) IconicState not listening to _NET_WM_STATE_HIDDEN; setVisible(true) not restoring from _NET_WM_STATE_HIDDEN
Product: [JogAmp] Newt Reporter: Xerxes Rånby <xerxes>
Component: x11Assignee: Sven Gothel <sgothel>
Status: RESOLVED FIXED    
Severity: blocker    
Priority: P1    
Version: 2.3.2   
Hardware: All   
OS: linux   
Type: DEFECT SCM Refs:
jogl dca5d36370ec5eb44998bae593880e3b10cc9a4e jogl e418a665756af52fe2ca691ca220644e9b27c22c jogl 8ce5203a581e731d5aa7d553455b04213d94115c jogl ecdaa59aa48c16a558ec7a0b50df9419a9ccedc1 jogl c61fa44d9a2e2049fec7833990f7bb699545bd15
Workaround: ---
Bug Depends on: 1250    
Bug Blocks:    

Description Xerxes Rånby 2015-10-07 13:33:07 CEST
Reproducible using
Fedora 22 + Gnome shell + OpenJDK 8
Ubuntu 14.04.3 LTS+ Compiz Unity + OpenJDK 7 & OpenJDK 8

Testcase:
com.jogamp.opengl.test.junit.newt.parenting.TestParenting01NEWT
test01CreateVisibleDestroy

This testcase and many similar testcases fail at the assert when transisting from STATE_BIT_VISIBLE true to false. The STATE_BIT_VISIBLE do not get cleared.

        glWindow1.setVisible(false);
        Assert.assertEquals(false, glWindow1.isVisible());

the testcase also fail when using absurd timeouts

        glWindow1.setVisible(false);
        Thread.sleep(7000);
        Assert.assertEquals(false, glWindow1.isVisible());

the window DO get hidden by the setVisible(false)


Example output:
https://gist.github.com/xranby/0d1751b0b0a0647fb576

IRC discussion:
http://jogamp.org/log/irc/jogamp_20151006050515.html#l412

All of these gets fixed on my machine by adding a patch such as this one https://github.com/xranby/jogl/commit/c0179c95a6ec57a618522d6baeadb804d7d1ab9a
Comment 1 Xerxes Rånby 2015-10-07 17:51:38 CEST
branch ready to merge:  workaround the issue in Fedora 22 and Ubuntu 
https://github.com/xranby/jogl/commits/Bug1249
Comment 2 Sven Gothel 2015-10-07 23:24:08 CEST
1) visibility true -> false only fails on mentioned platforms for child-windows?
2) where visibility false -> true works well for child _and_ top-level windows?
3) hence visibility false -> true always works, and true -> false not for child windows on those platforms?
Comment 3 Sven Gothel 2015-10-07 23:35:25 CEST
(In reply to Xerxes Rånby from comment #1)
> branch ready to merge:  workaround the issue in Fedora 22 and Ubuntu 
> https://github.com/xranby/jogl/commits/Bug1249

this is ofc not acceptable, i.e. we should wait for actual visibility
where it works.
Comment 4 Sven Gothel 2015-10-08 00:37:58 CEST
(In reply to Xerxes Rånby from comment #0)
> This testcase and many similar testcases fail at the assert when transisting
> from STATE_BIT_VISIBLE true to false. The STATE_BIT_VISIBLE do not get
> cleared.

Reproducible w/ Debian 8 and Gnome Shell 3.14.4-1~deb8u1,
while working on same platform w/ KDE
Comment 5 Sven Gothel 2015-10-08 00:43:07 CEST
#1 - TestParenting01NEWT.test01CreateVisibleDestroy failure 
  - parent top-level glWindow1.setVisible(false) 
    misses visibleChanged(false).
  - glWindow1 has a child window glWindow2
Comment 6 Sven Gothel 2015-10-08 00:58:03 CEST
(In reply to Sven Gothel from comment #5)
> #1 - TestParenting01NEWT.test01CreateVisibleDestroy failure 
>   - parent top-level glWindow1.setVisible(false) 
>     misses visibleChanged(false).
>   - glWindow1 has a child window glWindow2

the setVisible(false) on parent is performed via:
  WM_CHANGE_STATE_IDX + IconicState -> _MASK_NET_WM_STATE_HIDDEN

and indeed, we do receive an event other than UnmapNotify:

X11: reconfigureWindow0 X (full)
X11: event . ConfigureNotify call 0x2800002 (parent 0x2800002, above (nil)) 2879/206 640x480 0, child-event: 0
NewtWindows_updateInsets: insets by _NET_FRAME_EXTENTS [ l 1, r 1, t 39, b 1 ]
...... [0] -> [8/24]: _NET_WM_STATE_HIDDEN
**************** X11: WM_STATE of 0x2800002: 0x100


This is a regression of commit 2d837a7a7130702ad36b694875613fae77c7ef06,
which utilizes WM_CHANGE_STATE_IDX + IconicState for visibility
on parent windows.

Notice commit 4a9f65b176d618a8816eff6d24e683c56a4d8086,
which also disabled WM_CHANGE_STATE_IDX + IconicState
for child windows due to similar issue.
Well, we can leave that in, i.e. using Unmap, since child windows
disappearance shall not be animated.

Proposed fix:
  We also have to listen to _NET_WM_STATE_HIDDEN to trigger visibilityChanged()
Comment 7 Sven Gothel 2015-10-08 00:59:27 CEST
This is now a P1 + blocker, b/c this is confirmed to be a regression
which can break NEWT on many platforms, see comment 6.

Thx to Xerxes for this crucial finding!
Comment 8 Sven Gothel 2015-10-08 20:15:30 CEST
commit dca5d36370ec5eb44998bae593880e3b10cc9a4e:

Using Gnome Shell 3.14.4-1~deb8u1 disclosed an issue w/ our newly utilized IconicState/_NET_WM_STATE_HIDDEN,
i.e. visibleChanged(false) was never received.

This is a regression of commit 2d837a7a7130702ad36b694875613fae77c7ef06,
which utilizes WM_CHANGE_STATE_IDX + IconicState for visibility
on top-level windows.

This bug consist out of _two_ isssue:

1) setVisible(false) IconicState not listening to _NET_WM_STATE_HIDDEN
  Here, we 'listen' to _NET_WM_STATE_HIDDEN when receiving ConfigureNotify
  if supported _and_ XMapWindow has been issued.
  In such case existence/non-existence of _NET_WM_STATE_HIDDEN determines visibility.
  Otherwise, we have wait for MapNotify/UnmapNotify.

  The 'XMapWindow has been issued' criteria is tracked by new field 'JavaWindow.isMapped'
  and set/cleared when we actually issue XMapWindow/XUnmapWindow!

2) setVisible(true) not restoring from _NET_WM_STATE_HIDDEN
  It has been observed that restoring IconicState/_NET_WM_STATE_HIDDEN
  via XMapWindow or even NormalState may not work reliably on WMs.
  See <https://stackoverflow.com/questions/30192347/how-to-restore-a-window-with-xlib>

  Hence we restore from this WM state via NormalState _and_ _NET_ACTIVE_WINDOW.

Both strategies seem to work well on KDE as well as on Gnome.
Comment 9 Sven Gothel 2015-10-08 21:28:29 CEST
Reopening ..

1/10 tests w/ gnome-shell fails to detect visible -> false,
see:

XXX VISIBLE.1 -> FALSE
X11: reconfigureWindow0 dpy 0x1b5b1a0, scrn 0, parent (nil)/0x177, win 0x2800002, 1405/157 512x512, parentChange 0, isChild 0, undecorated[change 0, val 0], fullscreen[change 0, val 0 (span 0)], alwaysOn[Top[change 0, val 0], Bottom[change 0, val 0]], visible[change 1, val 0, tempInvisible 0], resizable[change 0, val 1], sticky[change 0, val 0], fsEWMHFlags 0
X11: reconfigureWindow0 setPosSize 1405/157 512x512
X11: reconfigureWindow0 VISIBLE OFF
X11: setVisible -> 0, useWM: 1, wait 0, window 0x2800002
X11: reconfigureWindow0 X (full)
X11: event . ConfigureNotify call 0x2800002 (parent 0x2800002, above (nil)) 1406/196 512x512 0, child-event: 0
...... [0] -> [13/25]: _NET_WM_STATE_FOCUSED
**************** X11: WM_STATE of 0x2800002: 1 props -> 0x2000
X11: event . ConfigureNotify call 0x2800002 - isMapped 1, visibleChanged 1, map_state 2
NewtWindows_updateInsets: insets by _NET_FRAME_EXTENTS [ l 1, r 1, t 39, b 1 ]
X11: event . FocusOut call 0x2800002
++++ TestCase.tearDown: com.jogamp.opengl.test.junit.newt.TestGLWindows00NEWT - test11WindowSimpleToggleVisibility
Comment 10 Sven Gothel 2015-10-09 01:57:14 CEST
NEWT X11: setVisible(*) _NET_WM_STATE_HIDDEN update not received 
at ConfigureNotify event (2)

On gnome shell WM, sometimes KDE WM,
it has been observed that the _NET_WM_STATE_HIDDEN update (visible or invisible)
is not received at ConfigureNotify event.

Turns out the state is finally updated at FocusOut!

This change tests _NET_WM_STATE_HIDDEN visibility hint
for mapped window also for FocusIn and FocusOut events,
besides the ConfigureNotify event.

Further more, NormalState to restore a hidden but mapped
window did not work, so it is no more being sent.
We limit us here to _NET_ACTIVE_WINDOW.

2 unit tests are prepared to test this issue:
  - TestGLWindows00NEWT
  - TestParenting01NEWT
Comment 11 Sven Gothel 2015-10-09 02:04:19 CEST
jogl 8ce5203a581e731d5aa7d553455b04213d94115c:
Revert TestParenting01NEWT durationPerTest back to 600 (otherwise breaks test)
Comment 12 Sven Gothel 2015-10-09 02:04:56 CEST
On KDE .. this time ..

X11: setVisible -> 1, useWM: 1, wait 0, window 0x6600004
X11: reconfigureWindow0 setPosSize.2 0/1649 320x240
X11: reconfigureWindow0 X (full)
X11: event . ConfigureNotify call 0x6600004 (parent 0x6600004, above (nil)) 2/1676 320x240 0, child-event: 0
...... [0] -> [8/25]: _NET_WM_STATE_HIDDEN
...... [1] -> [12/25]: _NET_WM_STATE_DEMANDS_ATTENTION
**************** X11: WM_STATE of 0x6600004: 2 props -> 0x1100
X11: event . ConfigureNotify call 0x6600004 - isMapped 1, visibleChanged 0, map_state 0
NewtWindows_updateInsets: insets by _NET_FRAME_EXTENTS [ l 2, r 2, t 27, b 4 ]
X11: event . ConfigureNotify call 0x6600004 (parent 0x6600004, above (nil)) 2/1676 320x240 0, child-event: 0
...... [0] -> [8/25]: _NET_WM_STATE_HIDDEN
...... [1] -> [12/25]: _NET_WM_STATE_DEMANDS_ATTENTION
**************** X11: WM_STATE of 0x6600004: 2 props -> 0x1100
X11: event . ConfigureNotify call 0x6600004 - isMapped 1, visibleChanged 0, map_state 0
NewtWindows_updateInsets: insets by _NET_FRAME_EXTENTS [ l 2, r 2, t 27, b 4 ]
XXXX: Visibility not reached as requested within 1000ms : requested true, is false
XXXX: FREEZE
Comment 13 Sven Gothel 2015-10-09 02:26:39 CEST
jogl ecdaa59aa48c16a558ec7a0b50df9419a9ccedc1:

NEWT X11: setVisible(false) IconicState/_NET_WM_STATE_HIDDEN: Handle case where KDE unmaps the window

Update our internal JavaWindow.isMapped according to MapNotify and UnmapNotify.
This takes care of a situation (KDE) where a window is unmapped
during IconicState.

For unmapped windows we cannot interprete _NET_WM_STATE_HIDDEN
and we have to issue XMapWindow for restoring the window.

+++

Closing for now .. seems to be working well on KDE and Gnome.

Pls reopen, if issues still exists.
Comment 14 Sven Gothel 2015-10-09 22:24:04 CEST
Bug 1250 documents same issue ..
Comment 15 Sven Gothel 2015-10-10 02:30:51 CEST
*** Bug 1250 has been marked as a duplicate of this bug. ***
Comment 16 Sven Gothel 2015-10-10 02:31:21 CEST
commit c61fa44d9a2e2049fec7833990f7bb699545bd15:

NEWT/X11 Visibility: Listening to more events for updated state
and Adding QUIRK_BIT_VISIBILITY

1) More visibility detection on post ConfigureNotify events,
since the latter may not yet contain the updated visibility state
as it whould (WM bug!):
  - EnterNotify
  - LeaveNotify
  - Disabled
    - Expose
    - VisibilityNotify

2) Introducing quirks.
   Setting QUIRK_BIT_VISIBILITY to handle the issue where
   visibility -> false could not even be set.

These changes also fix issues of Bug 1250
via the QUIRK: 
  - Gnome TestDisplayLifecycle02 setVisible(false) failure