Ok, no v2. More comments below.
On Wed, Apr 26, 2017 at 06:00:06PM +0530, Karthik Ramanan wrote: > Denys, > > I've looked through the comments, all of them are clarifications (so no v2). > Comments are inlined. > > Regards > Karthik > > On 26-Apr-17 4:58 AM, Denys Dmytriyenko wrote: > >On Tue, Apr 25, 2017 at 02:45:55PM +0530, Karthik Ramanan wrote: > >>Denys, > >> > >>Comments inlined. > >> > >>Regards > >>Karthik > >> > >>On 24-Apr-17 10:34 PM, Denys Dmytriyenko wrote: > >>>On Thu, Apr 20, 2017 at 09:44:22PM +0530, Karthik Ramanan wrote: > >>>>This recipe is primarily adapted from the meta-agl. > >>>>URL reference: https://goo.gl/m0LLmT > >>>> > >>>>The recipe brings in the capability to use the ivi-shell > >>>>feature of the Weston and provides utilities to manage > >>>>the screens, layers and surfaces. > >>>> > >>>>force-type-conversion.patch overlayed in the recipe is required > >>>>to address a compilation issue seen only with TI graphics > >>>>stack. > >>>> > >>>>The other patches are required on top of the 1.11.0 version to > >>>>fully use the multiple screen features on TI platforms. > >>>> > >>>>Signed-off-by: Karthik Ramanan <[email protected]> > >>>>--- > >>>> ...-add-surfaces-Add-screenId-as-an-argument.patch | 101 > >>>> +++++++++++++++++++++ > >>>> ...yer-add-surfaces-surface-layer-management.patch | 69 ++++++++++++++ > >>>> .../force-type-conversion.patch | 13 +++ > >>>> .../wayland/wayland-ivi-extension_1.11.0.bb | 33 +++++++ > >>>> 4 files changed, 216 insertions(+) > >>>> create mode 100644 > >>>> meta-arago-distro/recipes-graphics/wayland/wayland-ivi-extension/0001-layer-add-surfaces-Add-screenId-as-an-argument.patch > >>>> create mode 100644 > >>>> meta-arago-distro/recipes-graphics/wayland/wayland-ivi-extension/0002-layer-add-surfaces-surface-layer-management.patch > >>>> create mode 100644 > >>>> meta-arago-distro/recipes-graphics/wayland/wayland-ivi-extension/force-type-conversion.patch > >>>> create mode 100644 > >>>> meta-arago-distro/recipes-graphics/wayland/wayland-ivi-extension_1.11.0.bb > >>>> > >>>>diff --git > >>>>a/meta-arago-distro/recipes-graphics/wayland/wayland-ivi-extension/0001-layer-add-surfaces-Add-screenId-as-an-argument.patch > >>>> > >>>>b/meta-arago-distro/recipes-graphics/wayland/wayland-ivi-extension/0001-layer-add-surfaces-Add-screenId-as-an-argument.patch > >>>>new file mode 100644 > >>>>index 0000000..b8afc0d > >>>>--- /dev/null > >>>>+++ > >>>>b/meta-arago-distro/recipes-graphics/wayland/wayland-ivi-extension/0001-layer-add-surfaces-Add-screenId-as-an-argument.patch > >>>>@@ -0,0 +1,101 @@ > >>>>+From 0a368e3d65f3f3cbe193e9e4c6b7b7ddf4ec7b54 Mon Sep 17 00:00:00 2001 > >>>>+From: Ramprasad N <[email protected]> > >>>>+Date: Thu, 20 Apr 2017 13:24:20 +0530 > >>>>+Subject: [PATCH 1/2] layer-add-surfaces: Add screenId as an argument > >>>>+ > >>>>+Add Screen_ID as a argument to handle multiple screens. > >>>>+Removed choose_screen function as it is redundant > >>>>+after this change > >>>>+ > >>>>+Signed-off-by: Ramprasad N <[email protected]> > >>>>+Reviewed-by: Karthik Ramanan <[email protected]> > >>>>+--- > >>>>+ .../layer-add-surfaces/src/layer-add-surfaces.c | 47 > >>>>+++++----------------- > >>>>+ 1 file changed, 11 insertions(+), 36 deletions(-) > >>>>+ > >>>>+diff --git > >>>>a/ivi-layermanagement-examples/layer-add-surfaces/src/layer-add-surfaces.c > >>>> > >>>>b/ivi-layermanagement-examples/layer-add-surfaces/src/layer-add-surfaces.c > >>>>+index aaff7de..46d7d15 100644 > >>>>+--- > >>>>a/ivi-layermanagement-examples/layer-add-surfaces/src/layer-add-surfaces.c > >>>>++++ > >>>>b/ivi-layermanagement-examples/layer-add-surfaces/src/layer-add-surfaces.c > >>>>+@@ -32,6 +32,7 @@ > >>>>+ > >>>>+ t_ilm_uint screenWidth; > >>>>+ t_ilm_uint screenHeight; > >>>>++t_ilm_uint screen_ID; > >>>>+ t_ilm_uint layer; > >>>>+ pthread_mutex_t mutex; > >>>>+ static pthread_cond_t waiterVariable = PTHREAD_COND_INITIALIZER; > >>>>+@@ -91,48 +92,20 @@ static void callbackFunction(ilmObjectType object, > >>>>t_ilm_uint id, t_ilm_bool cre > >>>>+ } > >>>>+ } > >>>>+ > >>>>+-/* Choose the display with the largest resolution.*/ > >>>>+-static t_ilm_uint choose_screen(void) > >>>>++int main (int argc, const char * argv[]) > >>>>+ { > >>>>+ struct ilmScreenProperties screenProperties; > >>>>+- t_ilm_uint* screen_IDs = NULL; > >>>>+- t_ilm_uint screen_ID = 0; > >>>>+- t_ilm_uint screen_count = NULL; > >>>>+- t_ilm_uint choosen_width = 0; > >>>>+- t_ilm_uint choosen_height = 0; > >>>>+- int i; > >>>>+- > >>>>+- ilm_getScreenIDs(&screen_count, &screen_IDs); > >>>>+- > >>>>+- for (i = 0; i<screen_count; i++) > >>>>+- { > >>>>+- ilm_getPropertiesOfScreen(screen_IDs[i], &screenProperties); > >>>>+- if (screenProperties.screenWidth > choosen_width) { > >>>>+- choosen_width = screenProperties.screenWidth; > >>>>+- choosen_height = screenProperties.screenHeight; > >>>>+- screen_ID = screen_IDs[i]; > >>>>+- } > >>>>+- } > >>>>+ > >>>>+- screenWidth = choosen_width; > >>>>+- screenHeight = choosen_height; > >>>>+- > >>>>+- free(screen_IDs); > >>>>+- > >>>>+- return screen_ID; > >>>>+-} > >>>>+- > >>>>+-int main (int argc, const char * argv[]) > >>>>+-{ > >>>>+ // Get command-line options > >>>>+- if ( argc != 3) { > >>>>+- printf("Call layer-add-surface <layerID> > >>>><number_of_surfaces>\n"); > >>>>++ if ( argc != 4) { > >>>>++ printf("Call layer-add-surface <screenID> <layerID> > >>>><number_of_surfaces>\n"); > >>>>+ return -1; > >>>>+ } > >>>>+ > >>>>+- layer = strtol(argv[1], NULL, 0); > >>>>++ screen_ID = strtol(argv[1], NULL, 0); > >>>>++ layer = strtol(argv[2], NULL, 0); > >>>>+ > >>>>+- number_of_surfaces = strtol(argv[2], NULL, 0); > >>>>++ number_of_surfaces = strtol(argv[3], NULL, 0); > >>>>+ > >>>>+ pthread_mutexattr_t a; > >>>>+ if (pthread_mutexattr_init(&a) != 0) > >>>>+@@ -156,11 +129,13 @@ int main (int argc, const char * argv[]) > >>>>+ pthread_mutexattr_destroy(&a); > >>>>+ > >>>>+ t_ilm_layer renderOrder[1]; > >>>>+- t_ilm_uint screen_ID; > >>>>+ renderOrder[0] = layer; > >>>>+ ilm_init(); > >>>>+ > >>>>+- screen_ID = choose_screen(); > >>>>++ ilm_getPropertiesOfScreen(screen_ID, &screenProperties); > >>>>++ screenWidth = screenProperties.screenWidth; > >>>>++ screenHeight = screenProperties.screenHeight; > >>>>++ printf("GetPropertiesOfScreen: screen ID (%d), Width (%u), Height > >>>>(%u)\n", screen_ID, screenWidth, screenHeight); > >>>>+ ilm_layerCreateWithDimension(&layer, screenWidth, screenHeight); > >>>>+ printf("CreateWithDimension: layer ID (%d), Width (%u), Height > >>>>(%u)\n", layer, screenWidth, screenHeight); > >>>>+ ilm_layerSetVisibility(layer,ILM_TRUE); > >>>>+-- > >>>>+1.9.1 > >>>>+ > >>>>diff --git > >>>>a/meta-arago-distro/recipes-graphics/wayland/wayland-ivi-extension/0002-layer-add-surfaces-surface-layer-management.patch > >>>> > >>>>b/meta-arago-distro/recipes-graphics/wayland/wayland-ivi-extension/0002-layer-add-surfaces-surface-layer-management.patch > >>>>new file mode 100644 > >>>>index 0000000..eff4a79 > >>>>--- /dev/null > >>>>+++ > >>>>b/meta-arago-distro/recipes-graphics/wayland/wayland-ivi-extension/0002-layer-add-surfaces-surface-layer-management.patch > >>>>@@ -0,0 +1,69 @@ > >>>>+From 0d164d686147d2d3b08cd79ef9930f62ff8d528b Mon Sep 17 00:00:00 2001 > >>>>+From: Ramprasad N <[email protected]> > >>>>+Date: Thu, 20 Apr 2017 13:30:31 +0530 > >>>>+Subject: [PATCH 2/2] layer-add-surfaces: surface-layer management > >>>>+ > >>>>+This patch prevents surfaces from getting added to a new layer > >>>>+if they are already registered to an existing layer. > >>>>+ > >>>>+Signed-off-by: Ramprasad N <[email protected]> > >>>>+Reviewed-by: Karthik Ramanan <[email protected]> > >>>>+--- > >>>>+ .../layer-add-surfaces/src/layer-add-surfaces.c | 35 > >>>>++++++++++++++++++++-- > >>>>+ 1 file changed, 33 insertions(+), 2 deletions(-) > >>>>+ > >>>>+diff --git > >>>>a/ivi-layermanagement-examples/layer-add-surfaces/src/layer-add-surfaces.c > >>>> > >>>>b/ivi-layermanagement-examples/layer-add-surfaces/src/layer-add-surfaces.c > >>>>+index 46d7d15..cc76117 100644 > >>>>+--- > >>>>a/ivi-layermanagement-examples/layer-add-surfaces/src/layer-add-surfaces.c > >>>>++++ > >>>>b/ivi-layermanagement-examples/layer-add-surfaces/src/layer-add-surfaces.c > >>>>+@@ -59,14 +59,45 @@ static void surfaceCallbackFunction(t_ilm_uint id, > >>>>struct ilmSurfaceProperties* > >>>>+ configure_ilm_surface(id, sp->origSourceWidth, > >>>>sp->origSourceHeight); > >>>>+ } > >>>>+ } > >>>>+- > >>>>+ static void callbackFunction(ilmObjectType object, t_ilm_uint id, > >>>>t_ilm_bool created, void *user_data) > >>>>+ { > >>>>+ (void)user_data; > >>>>+ struct ilmSurfaceProperties sp; > >>>>++ t_ilm_int llength, slength, i, j; > >>>>++ t_ilm_uint* layerIDs = NULL; > >>>>++ t_ilm_uint* surfaceIDs = NULL; > >>>>++ t_ilm_bool addRequired = ILM_TRUE; > >>>>+ > >>>>+ if (object == ILM_SURFACE) { > >>>>+- if (created) { > >>>>++ ilm_getLayerIDs(&llength, &layerIDs); > >>>>++ > >>>>++ if(llength > 1) { > >>>>++ for(i=0; i < llength; i++) { > >>>>++ if(layerIDs[i] != layer) { > >>>>++ ilm_getSurfaceIDsOnLayer(layerIDs[i], &slength, > >>>>&surfaceIDs); > >>>>++ for(j=0; j < slength; j++){ > >>>>++ if(surfaceIDs[j] == id){ > >>>>++ addRequired = ILM_FALSE; > >>>>++ printf("surface %d is already attached to > >>>>layer %d\n", id, layerIDs[i]); > >>>>++ } > >>>>++ > >>>>++ } > >>>>++ > >>>>++ if(surfaceIDs) { > >>>>++ free(surfaceIDs); > >>>>++ } > >>>>++ > >>>>++ } > >>>>++ > >>>>++ } > >>>>++ > >>>>++ } > >>>>++ > >>>>++ if(layerIDs) { > >>>>++ free(layerIDs); > >>>>++ } > >>>>++ > >>>>++ if (created && addRequired) { > >>>>+ if (number_of_surfaces > 0) { > >>>>+ number_of_surfaces--; > >>>>+ printf("surface : %d created\n",id); > >>>>+-- > >>>>+1.9.1 > >>>>+ > >>>>diff --git > >>>>a/meta-arago-distro/recipes-graphics/wayland/wayland-ivi-extension/force-type-conversion.patch > >>>> > >>>>b/meta-arago-distro/recipes-graphics/wayland/wayland-ivi-extension/force-type-conversion.patch > >>>>new file mode 100644 > >>>>index 0000000..a72aeed > >>>>--- /dev/null > >>>>+++ > >>>>b/meta-arago-distro/recipes-graphics/wayland/wayland-ivi-extension/force-type-conversion.patch > >>>>@@ -0,0 +1,13 @@ > >>>>+diff --git > >>>>a/ivi-layermanagement-examples/EGLWLMockNavigation/src/OpenGLES2App.cpp > >>>>b/ivi-layermanagement-examples/EGLWLMockNavigation/src/OpenGLES2App.cpp > >>>>+index 2e65864..e14dbc0 100644 > >>>>+--- > >>>>a/ivi-layermanagement-examples/EGLWLMockNavigation/src/OpenGLES2App.cpp > >>>>++++ > >>>>b/ivi-layermanagement-examples/EGLWLMockNavigation/src/OpenGLES2App.cpp > >>>>+@@ -244,7 +247,7 @@ bool OpenGLES2App::createEGLContext() > >>>>+ m_eglContextStruct.eglSurface = NULL; > >>>>+ m_eglContextStruct.eglContext = NULL; > >>>>+ > >>>>+- m_eglContextStruct.eglDisplay = > >>>>eglGetDisplay(m_wlContextStruct.wlDisplay); > >>>>++ m_eglContextStruct.eglDisplay = > >>>>eglGetDisplay((EGLNativeDisplayType)m_wlContextStruct.wlDisplay); > >>>>+ eglstatus = eglGetError(); > >>>>+ if (!m_eglContextStruct.eglDisplay) > >>>>+ { > >>>>diff --git > >>>>a/meta-arago-distro/recipes-graphics/wayland/wayland-ivi-extension_1.11.0.bb > >>>> > >>>>b/meta-arago-distro/recipes-graphics/wayland/wayland-ivi-extension_1.11.0.bb > >>>>new file mode 100644 > >>>>index 0000000..157e1ee > >>>>--- /dev/null > >>>>+++ > >>>>b/meta-arago-distro/recipes-graphics/wayland/wayland-ivi-extension_1.11.0.bb > >>>>@@ -0,0 +1,33 @@ > >>>>+SUMMARY = "Wayland IVI Extension" > >>>>+DESCRIPTION = "GENIVI Layer Management API based on Wayland IVI > >>>>Extension" > >>>>+HOMEPAGE = "http://projects.genivi.org/wayland-ivi-extension"; > >>>>+BUGTRACKER = > >>>>"http://bugs.genivi.org/enter_bug.cgi?product=Wayland%20IVI%20Extension"; > >>>>+LICENSE = "Apache-2.0" > >>>>+LIC_FILES_CHKSUM = "file://LICENSE;md5=1f1a56bb2dadf5f2be8eb342acf4ed79" > >>>>+ > >>>>+SRCREV = "c9001582b10ce209c37b42dd560947c5aa8928b3" > >>>>+ > >>>>+SRC_URI = "git://github.com/GENIVI/${BPN}.git;protocol=http \ > >>>>+ file://force-type-conversion.patch \ > >>>>+ > >>>>file://0001-layer-add-surfaces-Add-screenId-as-an-argument.patch \ > >>>>+ file://0002-layer-add-surfaces-surface-layer-management.patch > >>>>\ > >>>>+ " > >>>>+ > >>>>+S = "${WORKDIR}/git" > >>>>+ > >>>>+DEPENDS = "weston virtual/libgles2 pixman" > >>>>+ > >>>>+inherit cmake > >>>>+ > >>>>+EXTRA_OECMAKE := "-DWITH_ILM_INPUT=1" > >>>>+EXTRA_OECMAKE += "-DLIB_SUFFIX=${@d.getVar('baselib', > >>>>True).replace('lib', '')}" > >>>What these 2 lines are doing? There's no need to use := in the first line. > >>>And > >>>in the second line, why so convoluted way - all it does is pass an empty > >>>string. > >>>Can these 2 lines be simplified and merged into one? > >>Few points: > >>a) This recipe section with is taken directly from the meta-agl > >>reference that I provided. > >>b) The immediate variable expansion ":=", can be avoided but its > >>fine to have right. Is there a recommended best practice? > >Well, it's not clear why it's needed. It would force any expansion early on, > >but there are none. > > Yes, I can submit a patch upstream and then bring it here. > > > > > > >>c) You are right that the expansion results in an empty string, but > >>I looked further into this and I see that such a usage is quite > >>prevalent. > >>I don't quite understand the intended purpose. Can you please let me > >>know what the intended purpose of this could be? > >>Again, I can change this but like I mentioned this is coming from > >>the upstream recipe. Let me know what you think? > >Ok, since the recipe is coming from another layer, it explains few things. > >The code there that extracts LIB_SUFFIX is made to be platform-agnostic - > >most > >of the architectures have baselib defined as "lib", but PowerPC64 defines it > >as "lib64", hence LIB_SUFFIX will be "64" in that case. It wouldn't make much > >sense for meta-arago, since we only care/support ARM, but other layers may > >want to be more inclusive and platform-agnostic... > Thanks for the details, that helps in further understanding of the > usage. Looking through the supported platforms on AGL, there are > some intel and 64 bit variants of ARM. > Would line make a difference for 64-bit ARM platforms? We should > retain it in the current form in that case. I don't see any other platform besides powerpc64 using lib64 in OE/Yocto, not even x86-64 or arm64... > >Now, it would be nice to mention that the recipe comes from meta-agl with > >minimal changes (only extra patches?) > Yes, I already captured that as part of the commit message and body. > > > >And Eric mentioned that it required some extra config steps to make it work - > >do you want to incorporate that into v2 patchset? > By default, we will be in desktop shell. Eric's request is only for > some documentation update. > So we can follow it up separately. Ok. > > > > > >>>>+ > >>>>+PR = "r3" > >>>>+ > >>>>+# Need these temporarily to prevent a non-fatal do_package_qa issue > >>>>+INSANE_SKIP_${PN} += "dev-deps" > >>>>+INSANE_SKIP_${PN}-dev += "dev-elf dev-so" > >>>>-- > >>>>1.9.1 > >>>> > >>>>_______________________________________________ > >>>>meta-arago mailing list > >>>>[email protected] > >>>>http://arago-project.org/cgi-bin/mailman/listinfo/meta-arago > _______________________________________________ meta-arago mailing list [email protected] http://arago-project.org/cgi-bin/mailman/listinfo/meta-arago
