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.

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.


+
+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

Reply via email to