Page MenuHomePhabricator

ecore-evas/x: detect and track wm existence, apply visibility correctly
ClosedPublic

Authored by zmike on Sep 10 2019, 10:18 AM.

Details

Summary

this attempts to monitor the _NET_SUPPORTING_WM_CHECK atom to verify whether
a wm exists, and bypasses waiting for a configure event from a nonexistent wm
if the screen is not currently managed

fix T7838

Depends on D10014

Diff Detail

Repository
rEFL core/efl
Branch
master
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 13390
zmike created this revision.Sep 10 2019, 10:18 AM

It seems that this patch has no reviewers specified. If you are unsure who can review your patch, please check this wiki page and see if anyone can be added: https://phab.enlightenment.org/w/maintainers_reviewers/

zmike requested review of this revision.Sep 10 2019, 10:18 AM

I just applied your patch efl-1.22.4.
It works.
Thank you.

thierry1970 added a comment.EditedSep 16 2019, 1:30 AM

I removed the patch, I have an infinite loop that appears at the launch of applications, when starting E.
On the other hand the virtual keyboard is visible on lightdm.

zmike added a comment.Sep 16 2019, 6:56 AM

I removed the patch, I have an infinite loop that appears at the launch of applications, when starting E.
On the other hand the virtual keyboard is visible on lightdm.

Do you have a test case for this?

the output in xsession-errors with the patch applied. :


the output in xsession-errors with your modified patch:
patch modified:
If you need more precision ...

E version : 0.23.0+0aea2d23a757
EFL : 1.22.4

I start E and I run elementary_test

]$ top
  PID USER      PR  NI    VIRT    RES    SHR S  %CPU %MEM     TIME+ COMMAND                                                                                                                                                            
14777 ordissi+  20   0 2182996 710492  58424 R 100,0 18,0  74:42.05 enlightenment
zmike added a comment.Sep 17 2019, 5:24 AM

Hm the modified patch you've posted here seems like it is broken according to wm specs; the issue is that the window should be waiting for the wm to configure it once it is shown, but you've subverted that by toggling the configured flag if the environment is a compositing wm (and added a roundtrip to every event).

I'm testing here with the latest EFL from master (rEFL3932c6838292472b14f6dcfd90e74c59554c9524) and this patch applied on top: things seem to work as expected in Enlightenment 0.23 as well as other WMs. I'm wondering if this is an issue with the 1.22 branch somehow...

I have a few things to finish, then I will wear the top version of the EFL-1.23.xxxx,
I tell you that quickly.

zmike added a comment.Sep 17 2019, 5:53 AM

I've just tested on 1.22.4 and it does indeed seem to fail there. Will examine this a little more...

zmike added a comment.Sep 17 2019, 6:39 AM

Hm after quite some time looking into this, I've run out of time. There's some sort of X11 protocol handling race condition which triggers under enlightenment/efl in 1.22 but is resolved in 1.23. I'll skip backporting this, but it does seem to fix the issue in 1.23.

Ok, I test the efl 1.23,
Thank you

zmike updated this revision to Diff 25160.Sep 18 2019, 10:02 AM
zmike edited the summary of this revision. (Show Details)

use new prop monitor function

@thierry1970 hey, with the patches I've just added to this series everything should be fixed even in 1.22.

devilhorns requested changes to this revision.Wed, Sep 25, 12:46 PM
devilhorns added a subscriber: devilhorns.
devilhorns added inline comments.
src/modules/ecore_evas/engines/x/ecore_evas_x.c
1912

Huh ?? Wasn't this just defined in https://phab.enlightenment.org/D10013 ... I would think that you shouldn't need this declaration here (assuming ecore_evas_x is included ecore_x_private.h)....

This revision now requires changes to proceed.Wed, Sep 25, 12:46 PM
zmike requested review of this revision.Wed, Sep 25, 12:58 PM
zmike added inline comments.
src/modules/ecore_evas/engines/x/ecore_evas_x.c
1912

It isn't.

devilhorns accepted this revision.Wed, Sep 25, 1:20 PM

Ah ok, no worries then

This revision is now accepted and ready to land.Wed, Sep 25, 1:20 PM
This revision was automatically updated to reflect the committed changes.

@zmike, I just applied the patches (D10012, D10013, D10014, D9899 and D9900) on efl 1.22.5.
E does not panic and the keyboard works under ligthdm.
Congratulations for the work!

zmike added a comment.Fri, Sep 27, 8:09 AM

Great to hear!