[Linux] EGL split up and cleanup #11986

lrusak · 2017-04-18T18:18:54Z

Description

windowing/egl is a mess, let's split it up and clean it up. This basically just splits all existing code into a separate directory. This isn't meant to change the code in any way, only how it is organized. The code can be cleaned up later (which it needs IMO).

RPi
Android
Amlogic
IMX6

My only concern here is the git history and how to properly preserve it.

Mainly I would like to preserve the EGLNativeTypeRaspberryPI.h -> RPIUtils.h change as github picks up the EGLNativeTypeRaspberryPI.cpp -> RPIUtils.c change automatically.
I'll see what I can do about this.

ping @popcornmix and @FernetMenta for review.

RPi is the most complicated of the bunch so I took that on first. The other targets should be relatively easy though I can only test amlogic (I have no ~~android~~ or imx6 devices available). If someone else wants to take one on that's great, otherwise I'll just try and work through it.

Motivation and Context

windowing/egl is a confusing mess and should die in a fire

How Has This Been Tested?

built on LibreELEC and tested to match all existing functionality.

Screenshots (if appropriate):

Types of change

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

My code follows the Code guidelines of this project
My change requires a change to the documentation, either Doxygen or wiki
I have updated the documentation accordingly
I have read the CONTRIBUTING document
I have added tests to cover my change
All new and existing tests passed

lrusak · 2017-04-19T07:38:11Z

Added amlogic to the clean up. Tested on my wetek hub and seems to work fine.

In progress android cleanup here https://github.com/lrusak/xbmc/tree/egl-cleanup-android
I'm just working on setting up the android dev environment to test it.

hopefully imx6 after that.

hudokkow · 2017-04-19T09:00:47Z

If you rename/move files around you'll lose (GitHub) history. There's no way around it currently. That happens because GitHub doesn't use git log --follow.

However, you can still take advantage of git blame if moving the file and changing its content is done in two separate commits. I suggest moving first. See #10446 for reference.

Right now you are losing everything. Test Blame and History in https://github.com/lrusak/xbmc/blob/a0353135e4576c70c32312ceee3f08e827462746/xbmc/windowing/amlogic/VideoSyncAML.cpp and you'll notice only one commit is shown.

For comparison, https://github.com/xbmc/xbmc/blob/master/CMakeLists.txt Historyonly shows commits after the move to root but Blame shows all the commits.

Mettbrot · 2017-04-19T10:35:00Z

Sorry to chime in, but this was a problem some time ago here #5481 (comment)
I wonder what the status of this "feature" is at github 2.5 years later?

lrusak · 2017-04-19T16:36:38Z

@hudokkow if you look at the commit changes at a whole you will see that github picks up the move and shows
xbmc/windowing/egl/VideoSyncAML.cpp → xbmc/windowing/amlogic/VideoSyncAML.cpp
quite a few files do this. Hopefully I can get all relevant files to show up like this.

stefansaraev · 2017-04-19T16:40:23Z

github shows file renames as a "whole" just in the pr when you go to browse changes tab. this is not what we care about in general.

you should not delete and re-add renamed files in two commits. do everything in one (rename + your changes) commit per platform and you are done with it. git log --follow will work. git blame will work ;)

another reason to go for one commit per platform is.. git bisect. it would cost you some extra work to keep every single commit "buildable". so please do not break it ;)

lrusak · 2017-04-19T17:14:20Z

@stefansaraev thanks for the explanation. I'll leave the commits separate for now as it's easier for review, I can squash when ready. 👍

FernetMenta · 2017-04-19T18:48:09Z

+1
even if this broke history completly, it would be better than what we have currently

lrusak · 2017-04-22T00:53:25Z

Pushed changes for android, It builds ok but I'm not sure if it runs yet because I have issues installing it on my Nexus 4.

lrusak · 2017-04-25T01:29:26Z

Android isn't working as it crashes upon launch but I am having trouble debugging. Maybe someone can help me here.

via logcat all I see is

04-24 17:37:12.593   193   193 F DEBUG   : backtrace:
04-24 17:37:12.593   193   193 F DEBUG   :     #00 pc 024eccc6  /data/app/org.xbmc.kodi-1/lib/arm/libkodi.so
04-24 17:37:12.593   193   193 F DEBUG   :     #01 pc 01262ff8  /data/app/org.xbmc.kodi-1/lib/arm/libkodi.so (CAndroidUtils::CAndroidUtils()+88)
04-24 17:37:12.593   193   193 F DEBUG   :     #02 pc 0125f5ec  /data/app/org.xbmc.kodi-1/lib/arm/libkodi.so (CWinSystemAndroid::CWinSystemAndroid()+128)
04-24 17:37:12.593   193   193 F DEBUG   :     #03 pc 0071e6cc  /data/app/org.xbmc.kodi-1/lib/arm/libkodi.so (xbmcutil::GlobalsSingleton<CWinSystemAndroidGLESContext>::getInstance()+68)
04-24 17:37:12.593   193   193 F DEBUG   :     #04 pc 0071d3a4  /data/app/org.xbmc.kodi-1/lib/arm/libkodi.so

lrusak · 2017-04-25T18:41:38Z

pushed a commit that to get android working. 👍

Now to move on to imx6.

lrusak · 2017-04-26T05:46:55Z

Added imx6. It builds but it won't be working correctly yet as I'm not sure how to handle some stuff.

I also don't have any imx6 hardware to test with. Is it true that the entire display system has to be destroyed in order to change display modes? Can we just drop imx6 and move it to drm/kms? ;)

lrusak · 2017-04-26T20:16:04Z

If I can get an ok by the platform devs for android, amlogic, and rpi, I can drop imx6 and leave that in the windowing/egl directory for now until I'm able to get a device and test the changes.

lrusak · 2017-04-27T20:06:15Z

I've dropped the imx6 commits.

I consider this good to go unless we want to wait until imx6 can be included as well.

Platform devs please looks over:
Android: @koying @peak3d
Amlogic: @peak3d @codesnake
RPi: @popcornmix

lrusak · 2017-04-27T20:26:10Z

note that GLContextEGL.cpp and GLContextEGL.h are the same for all three platforms. This file can be broken out and put into it's own egl folder after this PR.

xbmc/windowing/rpi/WinSystemRpi.cpp

+  m_stereo_mode = RENDER_STEREO_MODE_OFF;
+  m_delayDispReset = false;
+
+  m_rpi = new CRPIUtils();


xbmc/windowing/rpi/WinSystemRpi.cpp

@@ -58,6 +58,9 @@ CWinSystemRpi::~CWinSystemRpi()
  {
    m_nativeWindow = nullptr;
  }
+
+  m_rpi = nullptr;
+  delete m_rpi;


popcornmix · 2017-05-06T16:32:46Z

+1 from me

jenkins build this please

koying · 2017-05-10T17:29:54Z

Works for me.
+1

lrusak · 2017-05-10T17:40:41Z

What is the consensus, is it ok to merge before imx6 cleanup is finished?

Would still be nice to get an ok from an amlogic person @stefansaraev @peak3d @codesnake

stefansaraev · 2017-05-10T17:46:23Z

worked for me last time I tested it. will re-test in 20 min and update this comment

works for me ^^

MartijnKaijser · 2017-05-10T18:39:34Z

jenkins build this please

lrusak force-pushed the egl-cleanup branch 2 times, most recently from b477b98 to 2362647 Compare April 19, 2017 17:12

lrusak force-pushed the egl-cleanup branch 2 times, most recently from 5a6bf63 to 3da5bd4 Compare April 27, 2017 20:06

popcornmix reviewed Apr 30, 2017

View reviewed changes

xbmc/windowing/rpi/WinSystemRpi.cpp

m_stereo_mode = RENDER_STEREO_MODE_OFF;

m_delayDispReset = false;

m_rpi = new CRPIUtils();

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

popcornmix reviewed May 2, 2017

View reviewed changes

xbmc/windowing/rpi/WinSystemRpi.cpp Outdated

@@ -58,6 +58,9 @@ CWinSystemRpi::~CWinSystemRpi()

{

m_nativeWindow = nullptr;

}

m_rpi = nullptr;

delete m_rpi;

This comment was marked as spam.

Sign in to view

popcornmix mentioned this pull request May 5, 2017

GUI: Provide mechanism to disable gui overlay when empty #12054

Merged

9 tasks

lrusak added 3 commits May 10, 2017 15:50

[linux] split out rpi from windowing/egl

12091a2

[linux] split out amlogic from windowing/egl

f9d8ab7

[linux] split out android from windowing/egl

d68397f

lrusak force-pushed the egl-cleanup branch from e383f76 to d68397f Compare May 10, 2017 14:50

MartijnKaijser added this to the L 18.0-alpha1 milestone May 10, 2017

MartijnKaijser added Type: Cleanup non-breaking change which removes non-working or unmaintained functionality v18 Leia labels May 10, 2017

MartijnKaijser changed the title ~~[WIP][Linux] EGL split up and cleanup~~ [Linux] EGL split up and cleanup May 10, 2017

lrusak merged commit a30d4e8 into xbmc:master May 11, 2017

stefansaraev mentioned this pull request May 15, 2017

[fix] amlogic: 4k resolutions after f9d8ab7 #12098

Merged

9 tasks

lrusak mentioned this pull request May 16, 2017

[linux] treat rbpi as linux not a separate platform #12021

Merged

9 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Linux] EGL split up and cleanup #11986

[Linux] EGL split up and cleanup #11986

lrusak commented Apr 18, 2017 •

edited

lrusak commented Apr 19, 2017

hudokkow commented Apr 19, 2017

Mettbrot commented Apr 19, 2017

lrusak commented Apr 19, 2017

stefansaraev commented Apr 19, 2017 •

edited

lrusak commented Apr 19, 2017

FernetMenta commented Apr 19, 2017

lrusak commented Apr 22, 2017

lrusak commented Apr 25, 2017

lrusak commented Apr 25, 2017

lrusak commented Apr 26, 2017

lrusak commented Apr 26, 2017

lrusak commented Apr 27, 2017

lrusak commented Apr 27, 2017

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

popcornmix commented May 6, 2017

koying commented May 10, 2017

lrusak commented May 10, 2017 •

edited

stefansaraev commented May 10, 2017 •

edited

MartijnKaijser commented May 10, 2017

[Linux] EGL split up and cleanup #11986

[Linux] EGL split up and cleanup #11986

Conversation

lrusak commented Apr 18, 2017 • edited

Description

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

Types of change

Checklist:

lrusak commented Apr 19, 2017

hudokkow commented Apr 19, 2017

Mettbrot commented Apr 19, 2017

lrusak commented Apr 19, 2017

stefansaraev commented Apr 19, 2017 • edited

lrusak commented Apr 19, 2017

FernetMenta commented Apr 19, 2017

lrusak commented Apr 22, 2017

lrusak commented Apr 25, 2017

lrusak commented Apr 25, 2017

lrusak commented Apr 26, 2017

lrusak commented Apr 26, 2017

lrusak commented Apr 27, 2017

lrusak commented Apr 27, 2017

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

popcornmix commented May 6, 2017

koying commented May 10, 2017

lrusak commented May 10, 2017 • edited

stefansaraev commented May 10, 2017 • edited

MartijnKaijser commented May 10, 2017

lrusak commented Apr 18, 2017 •

edited

stefansaraev commented Apr 19, 2017 •

edited

lrusak commented May 10, 2017 •

edited

stefansaraev commented May 10, 2017 •

edited