<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/128201/">https://git.reviewboard.kde.org/r/128201/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On June 19th, 2016, 12:20 p.m. CEST, <b>David Faure</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="https://git.reviewboard.kde.org/r/128201/diff/2/?file=469229#file469229line90" style="color: black; font-weight: bold; text-decoration: underline;">shell/progresswidget/statusbarprogresswidget.cpp</a>
<span style="font-weight: normal;">
(Diff revision 2)
</span>
</th>
</tr>
</thead>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">84</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="kt">bool</span> <span class="n">macWidgetStyle</span> <span class="o">=</span> <span class="n">QApplication</span><span class="o">::</span><span class="n">style</span><span class="p">()</span><span class="o">-></span><span class="n">objectName</span><span class="p">()</span> <span class="o">==</span> <span class="n">QLatin1String</span><span class="p">(</span><span class="s">"macintosh"</span><span class="p">);</span></pre></td>
</tr>
</tbody>
</table>
<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;">Yes, see QWidget::setStyle().</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Not sure what the problem is with alignment, but at least it seems clear to me that if CT_ProgressBar returns a value smaller than the contents, we need to resize down the contents. Isn't that the issue here?</p></pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; 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;">That's what I'm doing - and I'm also sizing up the contents when the progress bar height is taller. As to replacing <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">QApplication::style()</code> with <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">m_pProgressBar->style()</code>:</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;"><div class="codehilite" style="background: #f8f8f8"><pre style="line-height: 125%"> <span style="color: #008000; font-weight: bold">qWarning</span><span style="color: #666666">()</span> <span style="color: #666666"><<</span> <span style="color: #BA2121">"progbar style:"</span> <span style="color: #666666"><<</span> <span style="color: #008000; font-weight: bold">m_pProgressBar-</span><span style="color: #666666">></span><span style="color: #008000; font-weight: bold">style</span><span style="color: #666666">()</span> <span style="color: #666666"><<</span> <span style="color: #008000; font-weight: bold">m_pProgressBar-</span><span style="color: #666666">></span><span style="color: #008000; font-weight: bold">style</span><span style="color: #666666">()</span><span style="color: #008000; font-weight: bold">-</span><span style="color: #666666">></span><span style="color: #008000; font-weight: bold">objectName</span><span style="color: #666666">()</span>
<span style="color: #666666"><<</span> <span style="color: #BA2121">"app style:"</span> <span style="color: #666666"><<</span> <span style="color: #008000; font-weight: bold">QApplication</span><span style="color: #666666">:</span><span style="color: #AA22FF">:style</span><span style="color: #666666">();</span>
</pre></div>
</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">gives</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;"><div class="codehilite" style="background: #f8f8f8"><pre style="line-height: 125%">progbar style: QStyleSheetStyle(0x7fdbf2656c00) "" app style: QMacStyle(0x7fdbf248e8d0, name = "macintosh")
progbar style: QStyleSheetStyle(0x7fc515d44190) "" app style: QtCurve::Style(0x7fc513876600, name = "qtcurve")
progbar style: QStyleSheetStyle(0x7fcb7c505e10) "" app style: Breeze::Style(0x7fcb79cbfd30, name = "breeze")
</pre></div>
</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">In other words, <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">QWidget::style()</code> returns something that is really a <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">QStyleSheetStyle</code> most (?) of the time. QStyleSheetStyle does have a <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">QStyle *baseStyle()</code> method, but you need to include a private header to gain access to that class. Do you know of any other way to get the current style name? I've tried <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">QProxyStyle pbStyle(m_pProgressBar->style())</code> but that doesn't help (and crashes when pbStyle goes out of scope). I'm tempted to conclude that using <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">m_pProgressBar->style()</code> may well be a theoretically more correct, but in practice not workable approach.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">As to the alignment issue: I also couldn't fathom what that is about, in any case nothing I tried allowed me to get rid of it completely. I only managed to make it "acceptably small" with the current code (without flat buttons). See the 1st screenshot of the Mac/native widget without the patch; the alignment offset is visible there, and is maybe slightly worse than what I'm getting now but still acceptable as long as you don't study the layout too closely. It becomes much less noticeable when using a flat button, IMHO good enough not to complexify the code even more in an attempt to improve things.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">BTW, QProgressBar does accept <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">WA_MacMiniSize</code> but 1) that affects all styles on OS X and 2) it ascerbates the alignment issue to an extreme point: the icon vcentre is aligned more or less with the progressbar bottom. Not usable AFAIAC even if the lower progress bar does look better.</p></pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On June 19th, 2016, 12:20 p.m. CEST, <b>David Faure</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="https://git.reviewboard.kde.org/r/128201/diff/2/?file=469229#file469229line96" style="color: black; font-weight: bold; text-decoration: underline;">shell/progresswidget/statusbarprogresswidget.cpp</a>
<span style="font-weight: normal;">
(Diff revision 2)
</span>
</th>
</tr>
</thead>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">90</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="kt">int</span> <span class="n">pbHeight</span> <span class="o">=</span> <span class="n">m_pProgressBar</span><span class="o">-></span><span class="n">sizeHint</span><span class="p">().</span><span class="n">height</span><span class="p">();</span></pre></td>
</tr>
</tbody>
</table>
<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;">Oops I meant to remove that comment once I wrote the previous one (i.e. after understanding the difference between styles where CT_ProgressBar adapts to the contents, and styles where it doesn't)</p></pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; 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;">What's the issue to fix here? NB: after re-reading the comment I remember that the choice of height estimate also depends on the kind of button that's used. With a non-flat button I find the overal result better using <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">minimumSizeHint()</code>.</p></pre>
<br />
<p>- René J.V.</p>
<br />
<p>On June 19th, 2016, 12:29 p.m. CEST, René J.V. Bertin 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 KDevelop and David Faure.</div>
<div>By René J.V. Bertin.</div>
<p style="color: grey;"><i>Updated June 19, 2016, 12:29 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kdevplatform
</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;">I came across this old bit of code, judging from the use of the deprecated <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">Q_OS_MAC</code> token.
Some quick testing suggests that using <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">WA_LayoutUsesWidgetRect</code> achieves what I think is the intended effect without platform-specific code. (That platform test ought really check for using the <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">macintosh</code> widget style btw, rather than "are we running on OS X").</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;">For now only on OS X, with the native theme as well as QtCurve, Breeze and Oxygen.</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/progresswidget/statusbarprogresswidget.cpp <span style="color: grey">(1a32ea8)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/128201/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/2016/06/16/1498e8dc-4541-473b-b6db-2c2ae138cbc3__Screen_Shot_2016-06-17_at_00.17.17.png">OS X/Mac native *without* the patch</a></li>
<li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2016/06/17/dd352994-1753-4b33-a0d2-cd2a3132df48__progress-stock.png">stock widget under Linux (same layout using Breeze)</a></li>
<li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2016/06/18/8e6a0761-b760-4f41-be67-7f9ba7318349__Screen_Shot_2016-06-18_at_19.21.52.png">OS X/native with the new patch</a></li>
<li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2016/06/18/3535be4c-3034-493d-88c0-61ab9a13844d__Screen_Shot_2016-06-18_at_19.24.46.png">OS X using QtCurve with the new patch</a></li>
<li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2016/06/18/359d5f57-bdaa-45a7-a0b0-6acb5609459f__Screen_Shot_2016-06-18_at_19.16.19.png">Linux, Breeze, new patch</a></li>
<li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2016/06/18/f5eb0902-884a-47e5-bf84-a3581a21bece__Screen_Shot_2016-06-18_at_19.14.17.png">Linux, QtCurve, new patch</a></li>
<li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2016/06/18/536d4f09-8409-4eb4-80ca-27d3f744d6a0__Screen_Shot_2016-06-18_at_19.16.56.png">Linux, Oxygen, new patch</a></li>
<li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2016/06/19/ee2d687e-69ea-493f-9ae4-87d90be614e2__progress-patched-breeze.png">Linux, Breeze + Breeze icons, new patch</a></li>
<li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2016/06/19/7e3cfc5c-cb80-491c-b4d2-4d6f9c839a48__progress-patched-qtcurve.png">Linux, QtCurve + Breeze icons, new patch</a></li>
<li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2016/06/19/0b10d97d-3ba6-4d05-a663-3a03e29bc1c9__progress-patched-oxygen.png">Linux,Oxygen+Breeze icons, new patch</a></li>
<li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2016/06/19/2a5b48bf-38f1-4ba9-a291-5de65537f8e1__progress-patched-breeze2.png">Linux, Breeze, new patch with maximumHeight from progressBar->sizeHint().height()</a></li>
</ul>
</td>
</tr>
</table>
</div>
</body>
</html>