DevHeads.net

Review Request: Add support for building libplasma with GLES2

Review request for kdelibs.

Summary
After build kwin with GLES2 code path, the kwin binary still has dependency on libGL.so, which is introduced by libplasma.so. Then we also need to add GLES2 support to libplasma, so that kwin/plasma only has dependency on libGLESv2.so in this case.

The new option "BUILD_PLASMA_WITH_OPENGLES" added in attached patch is disabled by default, and distributions can turn it on when do packaging for OpenGL ES2.0 support.

Diffs
CMakeLists.txt 2dcc5ec
cmake/modules/FindOpenGLES.cmake PRE-CREATION
includes/CMakeLists.txt 9954c37
plasma/CMakeLists.txt 8e6b0d7

Diff: <a href="http://git.reviewboard.kde.org/r/100705/diff" title="http://git.reviewboard.kde.org/r/100705/diff">http://git.reviewboard.kde.org/r/100705/diff</a>

Testing
After turn the "BUILD_PLASMA_WITH_OPENGLES" option on, kwin-gles has no dependency on libGL.so now. And by default, libplasma still depends on libGL.so when run ldd.

Thanks,

Jammy

Comments

Re: Review Request: Add support for building libplasma with GLES

By Jammy Zhou at 02/23/2011 - 22:07

(Updated Feb. 24, 2011, 2:07 a.m.)

Review request for kdelibs and Plasma.

Changes
add plasma-devs for inputs

Summary
After build kwin with GLES2 code path, the kwin binary still has dependency on libGL.so, which is introduced by libplasma.so. Then we also need to add GLES2 support to libplasma, so that kwin/plasma only has dependency on libGLESv2.so in this case.

The new option "BUILD_PLASMA_WITH_OPENGLES" added in attached patch is disabled by default, and distributions can turn it on when do packaging for OpenGL ES2.0 support.

Diffs
CMakeLists.txt 2dcc5ec
cmake/modules/FindOpenGLES.cmake PRE-CREATION
includes/CMakeLists.txt 9954c37
plasma/CMakeLists.txt 8e6b0d7

Diff: <a href="http://git.reviewboard.kde.org/r/100705/diff" title="http://git.reviewboard.kde.org/r/100705/diff">http://git.reviewboard.kde.org/r/100705/diff</a>

Testing
After turn the "BUILD_PLASMA_WITH_OPENGLES" option on, kwin-gles has no dependency on libGL.so now. And by default, libplasma still depends on libGL.so when run ldd.

Thanks,

Jammy

Re: Review Request: Add support for building libplasma with GLES

By Jammy Zhou at 02/24/2011 - 23:29

(Updated Feb. 25, 2011, 3:29 a.m.)

Review request for kdelibs and Plasma.

Changes
Update the patch to remove direct OpenGL dependency for libplasma. The OpenGL usage in libplasma should better go through libQtOpenGL,which can be enabled for desktop OpenGL or OpenGL ES2.0

Summary
After build kwin with GLES2 code path, the kwin binary still has dependency on libGL.so, which is introduced by libplasma.so. Then we also need to add GLES2 support to libplasma, so that kwin/plasma only has dependency on libGLESv2.so in this case.

The new option "BUILD_PLASMA_WITH_OPENGLES" added in attached patch is disabled by default, and distributions can turn it on when do packaging for OpenGL ES2.0 support.

Diffs (updated)
CMakeLists.txt b8d53de
includes/CMakeLists.txt 9954c37
plasma/CMakeLists.txt 8e6b0d7
plasma/glapplet.cpp 00d7caf

Diff: <a href="http://git.reviewboard.kde.org/r/100705/diff" title="http://git.reviewboard.kde.org/r/100705/diff">http://git.reviewboard.kde.org/r/100705/diff</a>

Testing
After turn the "BUILD_PLASMA_WITH_OPENGLES" option on, kwin-gles has no dependency on libGL.so now. And by default, libplasma still depends on libGL.so when run ldd.

Thanks,

Jammy

Re: Review Request: Add support for building libplasma with GLES

By Sebastian =?utf... at 02/28/2011 - 09:03

On Friday, February 25, 2011 04:29:36 Jammy Zhou wrote:
Shouldn't that option cover both, kwin and plasma? Or are there serious cases
where you'd want one, but not the other on opengl-es? If not, let's merge them
to prevent building and packaging headaches.

Cheers,

Re: Review Request: Add support for building libplasma with GLES

By Marco Martin at 02/28/2011 - 07:50

Ship it!

to me seems good.
filling the background of black is something that eventually should do a reimplementation.

- Marco

On Feb. 25, 2011, 3:29 a.m., Jammy Zhou wrote:

Re: Review Request: Add support for building libplasma with GLES

By Martin =?iso-88... at 02/23/2011 - 13:29

The GLApplet seems to be unmaintained and is nowhere used in KDE (see <a href="http://lxr.kde.org/ident?i=GLApplet" title="http://lxr.kde.org/ident?i=GLApplet">http://lxr.kde.org/ident?i=GLApplet</a> ). There was once one demo applet using it, but it has vanished.

GLApplet uses OpenGL only for glClear before passing the QPainter to the Plasmoid. I think we could just remove the background clearing and require it from the applet and drop the link to OpenGL completely.

Another thing is that given that it is just not used adding the complete GLES detection and building to Plasma does not make sense. It's probably better just to add a cmake option to disable building of the plasma applet (maybe using the mobile profile for that?).

The CMake stuff to find GLES is unfortunately not in a state to be included in kdelibs. I contacted build system some time ago and they gave me a nice list of things which need to be improved and it's still on my TODO stack :-(

It would be nice to have some input from Plasma devs here. Could you please add the Plasma group to the review?

- Martin

On Feb. 21, 2011, 4:06 p.m., Jammy Zhou wrote:

Re: Review Request: Add support for building libplasma with GLES

By Jammy Zhou at 02/24/2011 - 22:16

Thanks for the detailed answers, Martin. I will prepare a new patch to remove the direct OpenGL dependency of libplasma as you suggested. I think if OpenGL rendering is really needed in libplasma, it can be done through QtOpenGL module, which can handle desktop OpenGL and OpenGL ES2.0 in Qt.

- Jammy

On Feb. 24, 2011, 2:07 a.m., Jammy Zhou wrote:

Re: Review Request: Add support for building libplasma with GLES

By Martin =?iso-88... at 02/24/2011 - 12:45

Technical background for the non OpenGL devs in Plasma: the GLApplet has two lines of OpenGL code: it sets the clear color and clears the background. In normal speak, it repaints the background completely black. Afterwards the paint implementation of the applet is called which does the actual OpenGL rendering. Now the normal way in doing a paint process, is to first clear the background. If I would write an applet that's what I would have done in my code as I would not have expected the framework to clear already. It is also not documented that it does that and you do not always want to clear the background but want to continue to render on the previous frame.

So the chances are that even if there exists an applet, which we do not know about, it would do the clearing itself.

- Martin

On Feb. 24, 2011, 2:07 a.m., Jammy Zhou wrote:

Re: Review Request: Add support for building libplasma with GLES

By Marco Martin at 02/25/2011 - 08:33

On Thursday 24 February 2011, Martin Gräßlin wrote:
opengl on qgraphicsview is so horribly slow that i don't see it going
anywhere, yeah

if it's marked as deprecated, the mobile profile would have it already removed
and that's good ;)

Cheers,
Marco Martin

Re: Review Request: Add support for building libplasma with GLES

By Jammy Zhou at 02/28/2011 - 07:40

Hi Macro,

I have updated the patch to remove direct OpenGL dependency for libplasma.
Could you please review the patch? (It seems that my mails to kde-core-devel
list were moderated by the server, so I send mail directly to you here)

Regards,
Jammy

Re: Review Request: Add support for building libplasma with GLES

By Marco Martin at 02/28/2011 - 07:51

On Monday 28 February 2011, Jammy Zhou wrote:
Hi Jammy,
to me the patch seems good.

Cheers,
Marco Martin

Re: Review Request: Add support for building libplasma with GLES

By Jammy Zhou at 02/23/2011 - 22:25

I'm a newbie for KDE development, what's GLApplet originally used for? And if GLApplet is really obsolete, maybe we'd better retire it to remove the dependency of libplasma on libGL. :)

I also considered to remove glClear in GLApplet, but I'm not sure if it will cause some regressions, so I prepared this change.

Anyway, waiting for inputs from plasma-devs to make final decision.

- Jammy

On Feb. 24, 2011, 2:07 a.m., Jammy Zhou wrote: