Re: [meta-fsl-demos][PATCH] vivante-gpu-sdk: Contribute recipe for Vivante GPU Samples

[Diego <diego.ml@zoho.com>]

In data martedì 21 maggio 2013 11:34:54, Otavio Salvador ha scritto:
> On Tue, May 21, 2013 at 10:57 AM, Diego Rondini <diego.ml@zoho.com> wrote:
> > Initial contribution of a recipe for Vivate GPU Samples from Vivante GPU
> > Sdk.
> > 
> > Signed-off-by: Diego Rondini <diego.ml@zoho.com>
> 
> Please change the commit log to be more assertive; something like:
> 
> ---
> vivante-gpu-sdk: Add recipe for GPU samples
> 
> Provides the samples provided by Vivante for use with the GPU.
> ---
> 
> Or something like that.
> 

Ack.

> > ---
> > 
> >  ...ation-of-the-GLES1.1-X11-Samples-in-Yocto.patch |  594
> > 
> > ++++++++++++++++++++
> > 
> >  .../0002-Don-t-build-missing-samples.patch         |   28 +
> >  .../vivante-gpu-sdk/vivante-gpu-sdk_1.00.bb        |   35 ++
> >  3 files changed, 657 insertions(+)
> >  create mode 100644
> > 
> > recipes-graphics/vivante-gpu-sdk/vivante-gpu-sdk/0001-Fix-compilation-of-t
> > he-GLES1.1-X11-Samples-in-Yocto.patch> 
> >  create mode 100644
> > 
> > recipes-graphics/vivante-gpu-sdk/vivante-gpu-sdk/0002-Don-t-build-missing-
> > samples.patch> 
> >  create mode 100644 recipes-graphics/vivante-gpu-sdk/
> > 
> > vivante-gpu-sdk_1.00.bb
> > 
> > diff --git
> > a/recipes-graphics/vivante-gpu-sdk/vivante-gpu-sdk/0001-Fix-compilation-of
> > -the-GLES1.1-X11-Samples-in-Yocto.patch
> > b/recipes-graphics/vivante-gpu-sdk/vivante-gpu-sdk/0001-Fix-compilation-o
> > f-the-GLES1.1-X11-Samples-in-Yocto.patch new file mode 100644
> > index 0000000..dacfd36
> > --- /dev/null
> > +++
> > b/recipes-graphics/vivante-gpu-sdk/vivante-gpu-sdk/0001-Fix-compilation-of
> > -the-GLES1.1-X11-Samples-in-Yocto.patch @@ -0,0 +1,594 @@
> > +From 72ba888f8e33a95343be834d5d9f12ce5d8c00f2 Mon Sep 17 00:00:00 2001
> > +From: Diego Rondini <diego.ml@zoho.com>
> > +Date: Tue, 30 Apr 2013 10:52:25 +0200
> > +Subject: [PATCH 1/2] Fix compilation of the GLES1.1 X11 Samples in Yocto
> > +
> > +This commit fixes the following build problems:
> > +- gcc 4.7 compatibility (remove unsupported "-Wl" option);
> > +- wrong compiler binary name (caused by an overwrite of CC and AR
> > variables)
> > +- unknown include and library paths (caused by forcing paths when not
> > needed)
> 
> Missed signed-off-by and upstream-status.
> 

Ok, will add.

> > +---
> > + Samples/GLES1.1/01_SimpleTriangle/Makefile.x11     |    9 +--------
> > + Samples/GLES1.1/02_VertexColors/Makefile.x11       |    7 +------
> > + .../GLES1.1/03_VertexTransformation/Makefile.x11   |    7 +------
[snip]
> > diff --git
> > a/recipes-graphics/vivante-gpu-sdk/vivante-gpu-sdk/0002-Don-t-build-missin
> > g-samples.patch
> > b/recipes-graphics/vivante-gpu-sdk/vivante-gpu-sdk/0002-Don-t-build-missi
> > ng-samples.patch new file mode 100644
> > index 0000000..1503848
> > --- /dev/null
> > +++
> > b/recipes-graphics/vivante-gpu-sdk/vivante-gpu-sdk/0002-Don-t-build-missin
> > g-samples.patch @@ -0,0 +1,28 @@
> > +From 97f3af7a44b41f3e46c54765ba415ba9f511fb81 Mon Sep 17 00:00:00 2001
> > +From: Diego Rondini <diego.ml@zoho.com>
> > +Date: Tue, 30 Apr 2013 12:46:38 +0200
> > +Subject: [PATCH 2/2] Don't build missing samples
> > +
> > +Samples 18_VertexBufferObjects and 19_Beizer are not included in the
> > package, so don't try to compile them.
> 
> Missed signed-off-by and upstream-status.
> 

Ok, will add.

> > +---
> > + Samples/GLES1.1/Makefile.x11 |    4 +---
> > + 1 file changed, 1 insertion(+), 3 deletions(-)
> > +
> > +diff --git a/Samples/GLES1.1/Makefile.x11 b/Samples/GLES1.1/Makefile.x11
> > +index 003763d..1c95158 100755
> > +--- a/Samples/GLES1.1/Makefile.x11
> > ++++ b/Samples/GLES1.1/Makefile.x11
> > +@@ -17,9 +17,7 @@ SUB_PROJECTS= \
> > +       14_ParticlesSpritesAnimation \
> > +       15_ParticleAccelerator \
> > +       16_VertexBufferObjects \
> > +-      17_Beizer \
> > +-      18_VertexBufferObjects \
> > +-      19_Beizer
> > ++      17_Beizer
> > +
> > +
> > + .PHONY: all
> > +--
> > +1.7.9.5
> > +
> > diff --git
> > a/recipes-graphics/vivante-gpu-sdk/vivante-gpu-sdk_1.00.bbb/recipes-graph
> > ics/vivante-gpu-sdk/ vivante-gpu-sdk_1.00.bb
> > new file mode 100644
> > index 0000000..69bcd01
> > --- /dev/null
> > +++ b/recipes-graphics/vivante-gpu-sdk/vivante-gpu-sdk_1.00.bb
> > @@ -0,0 +1,35 @@
> > +SUMMARY = "Vivante GPU SDK Samples"
> > +DESCRIPTION = "Freescale's adaptation of Jeff Molofee demos"
> 
> Is this description accurate?
> 

These are the first lines of every source file:
/*
 * This code was created by Jeff Molofee '99
 * (ported to Linux by Ti Leggett '01)
 * (ported to i.mx51, i.mx31 and x11 by Freescale '10)
 * If you've found this code useful, please let him know.
 *
 * Visit Jeff at http://nehe.gamedev.net/
 *
 */

I know the description is not the best one can produce, but this is not an 
established project, nor a one man/enterprise work. At least the X11 part is 
not Vivante's nor Freescale's specific at all.

> > +LICENSE = "Proprietary"
> > +LIC_FILES_CHKSUM = "file://COPYING;md5=b3ed4253a63ac8555ccab7c4c0aa29a1"
> > +
> > +PR = "r0"
> 
> Drop PR assignment.
> 

Ok, will do.

> > +inherit fsl-eula-unpack
> > +
> > +SRC_URI = "${FSL_MIRROR}/${PN}-${PV}.bin;fsl-eula=true \
> > +
> > file://0001-Fix-compilation-of-the-GLES1.1-X11-Samples-in-Yocto.patch;stri
> > plevel=1 \
> > +       file://0002-Don-t-build-missing-samples.patch;striplevel=1"
> 
> The striplevel 1 is the default so you can remove it.
> 

Ok, will do.

> > +SRC_URI[md5sum] = "344c9a260b46a00f86eb5cb73bf729e0"
> > +SRC_URI[sha256sum] =
> > "0f0a576ce1a08719ee1018832ae808ee723fad27a534c9739b5feec6d7435c57"
> > +
> > +S = "${WORKDIR}/${PN}-${PV}"
> > +GLES11DIR = "${S}/Samples/GLES1.1"
> 
> You can avoid this setting:
> 
> WORKDIR = "${S}/Samples/GLES1.1"
> 

I'll have multiple "workdirs", as this initial version is just for GLES1.1, 
but Samples folder has three subfolders:
GLES1.1
GLES2.0
OpenVG
each one has its own Makefile, and no common Makefile. I'm not interested in 
creating a common Makefile with a patch, as they might even get packaged 
separately in the future according to chipset support.
According to that, would you still use just:
WORKDIR = "${S}/Samples/GLES1.1"

> > +VIVANTE_INSTALL_DIR = "/opt/${PN}"
> 
> > +VIVANTE_INSTALL_DEST_DIR = "${D}${VIVANTE_INSTALL_DIR}"
> 
> 
> I'd use these explicitly as the variable name is bigger than the content of
> it ;-)
> 

Shorter code is not necessarily better or clearer code. However if you want 
I'll follow your suggestion.

> 
> 
> 
> +do_compile () {
> 
> > +       cd "${GLES11DIR}"
> 
> This can be removed.
> 

This is related to the multiple subfolders (GLES1.1, GLES2.0, OpenVG) which 
will come later, so let's discuss the "WORKDIR" issue.

> > +       oe_runmake -f Makefile.x11
> > +}
> > +
> > +do_install () {
> > +       cd "${GLES11DIR}"
> 
> This can be removed.
> 

As above.

> > +       install -d "${VIVANTE_INSTALL_DEST_DIR}"
> > +       oe_runmake -f Makefile.x11 install
> > +       cp -r bin/ "${VIVANTE_INSTALL_DEST_DIR}"
> 
> Uh? make install does not handle this?
> 

This is not in any way an "established project which installs in standard 
paths". It's more a "bunch of samples patched in the years, started quite some 
time ago, and intended just to be compiled and started locally by the learning 
developer".

> > +}
> > +
> > +FILES_${PN} += "${VIVANTE_INSTALL_DIR}"
> > +FILES_${PN}-dbg = "${VIVANTE_INSTALL_DIR}/*/*/.debug"

Thanks for your review. If you can clear out the WORKDIR vs multiple Makefiles 
in subdirs issue I'll post the updated patch.

Bests,
Diego