[prev in list] [next in list] [prev in thread] [next in thread]
List: kde-panel-devel
Subject: Re: Review Request 119524: Force OpenGLES if on Qt 5.4
From: "Aleix Pol Gonzalez" <aleixpol () kde ! org>
Date: 2014-08-11 8:54:52
Message-ID: 20140811085452.14668.97936 () probe ! kde ! org
[Download RAW message or body]
--===============9217701940361003651==
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 8bit
> On Aug. 11, 2014, 6:35 a.m., Martin Gräßlin wrote:
> > I highly suggest to revert this change. Enforcing GLES means limiting to GLES \
> > (it's a subset after all) and also not all drivers do support GLES - that's still \
> > the smaller part. We are calling for lots of trouble if we go this route.
> > I suggest to enforce core profile instead.
We discussed it with Marco, apparently this breaks on nvidia.
I'm fine with reverting it (both plasmashell and on krunner) or with if'ing the cases \
where it doesn't work.
- Aleix
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119524/#review64219
-----------------------------------------------------------
On July 29, 2014, 4:15 p.m., Aleix Pol Gonzalez wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119524/
> -----------------------------------------------------------
>
> (Updated July 29, 2014, 4:15 p.m.)
>
>
> Review request for Plasma.
>
>
> Repository: plasma-workspace
>
>
> Description
> -------
>
> When trying to reduce memory usage in plasma shell, we realized that one of the \
> reasons we got such big memory footprint was actually the intel driver (on intel \
> systems, that is). After some investigation, David found out we were going through \
> some memory-consuming path [1]. A way to workaround it is by using only OpenGLES. \
> Everything still works here after this patch, so it seems to be a good change to \
> get in.
> This change needs Qt 5.4, to get the new API that lets us enforce a QSurfaceFormat, \
> hence having it ifdef'd. I'm quite unaware of problems we might find. Knowing we \
> probably want to work in different embedded devices, suggests that OpenGLES on all \
> platforms sounds like a safe bet.
> [1] http://comments.gmane.org/gmane.comp.video.mesa3d.devel/60848
>
>
> Diffs
> -----
>
> shell/main.cpp e34578d
>
> Diff: https://git.reviewboard.kde.org/r/119524/diff/
>
>
> Testing
> -------
>
> I'm using it now, without visible problems. Now we see no trace of i915 in the \
> massif reports.
>
> File Attachments
> ----------------
>
> before
> https://git.reviewboard.kde.org/media/uploaded/files/2014/07/28/5a6beb4f-24c0-4cee-a7a2-038385e35119__plasma-massif-opengl.png
> after
> https://git.reviewboard.kde.org/media/uploaded/files/2014/07/28/fc301ff5-cb1d-4654-a57f-82990220c8e3__plasma-massif-opengles.png
>
>
> Thanks,
>
> Aleix Pol Gonzalez
>
>
--===============9217701940361003651==
MIME-Version: 1.0
Content-Type: text/html; charset="utf-8"
Content-Transfer-Encoding: 8bit
<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="12" style="border: 1px #c9c399 \
solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;"> \
<tr> <td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/119524/">https://git.reviewboard.kde.org/r/119524/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: \
10px;"> <p style="margin-top: 0;">On August 11th, 2014, 6:35 a.m. UTC, <b>Martin \
Gräßlin</b> wrote:</p> <blockquote style="margin-left: 1em; border-left: 2px solid \
#d0d0d0; padding-left: 10px;"> <pre style="white-space: pre-wrap; white-space: \
-moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">I highly suggest to revert this change. Enforcing GLES \
means limiting to GLES (it's a subset after all) and also not all drivers do support \
GLES - that's still the smaller part. We are calling for lots of trouble if we go \
this route.</p> <p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">I suggest to enforce core profile instead.</p></pre> \
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">We \
discussed it with Marco, apparently this breaks on nvidia.</p> <p style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I'm \
fine with reverting it (both plasmashell and on krunner) or with if'ing the cases \
where it doesn't work.</p></pre> <br />
<p>- Aleix</p>
<br />
<p>On July 29th, 2014, 4:15 p.m. UTC, Aleix Pol Gonzalez wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="12" style="border: \
1px #888a85 solid; border-radius: 6px; -moz-border-radius: 6px; \
-webkit-border-radius: 6px;"> <tr>
<td>
<div>Review request for Plasma.</div>
<div>By Aleix Pol Gonzalez.</div>
<p style="color: grey;"><i>Updated July 29, 2014, 4:15 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
plasma-workspace
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" \
style="border: 1px solid #b8b5a0"> <tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: \
-moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">When trying to reduce memory usage in plasma shell, we \
realized that one of the reasons we got such big memory footprint was actually the \
intel driver (on intel systems, that is). After some investigation, David found out \
we were going through some memory-consuming path [1]. A way to workaround it is by \
using only OpenGLES. Everything still works here after this patch, so it seems to be \
a good change to get in.</p> <p style="padding: 0;text-rendering: inherit;margin: \
0;line-height: inherit;white-space: inherit;">This change needs Qt 5.4, to get the \
new API that lets us enforce a QSurfaceFormat, hence having it ifdef'd.<br \
style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: \
normal;" /> I'm quite unaware of problems we might find. Knowing we probably want to \
work in different embedded devices, suggests that OpenGLES on all platforms sounds \
like a safe bet.</p> <p style="padding: 0;text-rendering: inherit;margin: \
0;line-height: inherit;white-space: inherit;">[1] \
http://comments.gmane.org/gmane.comp.video.mesa3d.devel/60848</p></pre> </td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: \
1px solid #b8b5a0"> <tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: \
-moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">I'm using it now, without visible problems. Now we see \
no trace of i915 in the massif reports.</p></pre> </td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>shell/main.cpp <span style="color: grey">(e34578d)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/119524/diff/" style="margin-left: \
3em;">View Diff</a></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">File Attachments \
</h1>
<li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2014/07/28/5a6beb4f-24c0-4cee-a7a2-038385e35119__plasma-massif-opengl.png">before</a></li>
<li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2014/07/28/fc301ff5-cb1d-4654-a57f-82990220c8e3__plasma-massif-opengles.png">after</a></li>
</ul>
</td>
</tr>
</table>
</div>
</body>
</html>
--===============9217701940361003651==--
_______________________________________________
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic