[prev in list] [next in list] [prev in thread] [next in thread]
List: kstars-devel
Subject: [Kstars-devel] Re: Review Request: Legend feature (currently used
From: "Akarsh Simha" <akarshsimha () gmail ! com>
Date: 2011-07-11 2:15:02
Message-ID: 20110711021502.8639.66692 () vidsolbach ! de
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/101912/#review4584
-----------------------------------------------------------
kstars/legend.cpp
<http://git.reviewboard.kde.org/r/101912/#comment3991>
KStars is now a single-instance class, so you can just use KStars::Inst=
ance(). Saves passing one parameter.
kstars/legend.cpp
<http://git.reviewboard.kde.org/r/101912/#comment3992>
You could inline all of these right? Probably, the compiler will be sma=
rt enough to do that anyway, though. But it also removes some clutter if yo=
u put them as one-liners in the header file.
kstars/legend.cpp
<http://git.reviewboard.kde.org/r/101912/#comment3993>
Maybe you should instead do a Q_ASSERT(false) so that we crash (I usual=
ly think it's better to crash and make sure the programmer fixes the proble=
m than returning a non-result that might be harder to detect :D)
kstars/skymap.h
<http://git.reviewboard.kde.org/r/101912/#comment3994>
Is this still used? Or can it be done away with?
kstars/skyqpainter.cpp
<http://git.reviewboard.kde.org/r/101912/#comment3995>
Could you explain this change? I'm not sure about the details, but do n=
ote that there are stars in the NGC and IC catalogs that are sometimes not =
part of the other star catalogs we use.
=
Edit: Sorry, I think I notice that the star's dia is set to 2.0 if it i=
s less than 2.0 in drawDeepSkySymbol(...), whereby it will get drawn anyway=
. Am I right here?
kstars/skyqpainter.cpp
<http://git.reviewboard.kde.org/r/101912/#comment3996>
Makes sense!
- Akarsh
On July 10, 2011, 10:38 p.m., Rafal Kulaga wrote:
> =
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/101912/
> -----------------------------------------------------------
> =
> (Updated July 10, 2011, 10:38 p.m.)
> =
> =
> Review request for KStars, Victor Carbune and Akarsh Simha.
> =
> =
> Summary
> -------
> =
> Attached diff adds the option to include legends in exported sky images. =
Both vertical and horizontal orientations are supported; there are two type=
s of legend: full-blown (symbol descriptions, star magnitudes and scale) an=
d scale-only. Some changes will inevitably be made to this code in a few da=
ys - please note that there are some hard-coded values in legend.cpp which =
will be gone after integration with the functionality I am developing now (=
FOV representation exporting).
> =
> Any comments are welcome, be they look&feel or code-related.
> =
> =
> Diffs
> -----
> =
> kstars/CMakeLists.txt 0c335b6 =
> kstars/dialogs/exportimagedialog.h PRE-CREATION =
> kstars/dialogs/exportimagedialog.cpp PRE-CREATION =
> kstars/dialogs/exportimagedialog.ui PRE-CREATION =
> kstars/kstarsactions.cpp e917cac =
> kstars/kstarsdcop.cpp 42dcb0f =
> kstars/legend.h PRE-CREATION =
> kstars/legend.cpp PRE-CREATION =
> kstars/skymap.h e7a7f56 =
> kstars/skymapdrawabstract.h c94a745 =
> kstars/skymapdrawabstract.cpp cf44fc5 =
> kstars/skypainter.h 1340568 =
> kstars/skyqpainter.h df7cc9b =
> kstars/skyqpainter.cpp 87719b2 =
> =
> Diff: http://git.reviewboard.kde.org/r/101912/diff
> =
> =
> Testing
> -------
> =
> Done some testing, everything worked (and looked) fine for me.
> =
> =
> Thanks,
> =
> Rafal
> =
>
[Attachment #5 (text/html)]
<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 \
solid;"> <tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="http://git.reviewboard.kde.org/r/101912/">http://git.reviewboard.kde.org/r/101912/</a>
</td>
</tr>
</table>
<br />
<div>
<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="http://git.reviewboard.kde.org/r/101912/diff/1/?file=26665#file26665line35" \
style="color: black; font-weight: bold; text-decoration: \
underline;">kstars/legend.cpp</a> <span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; \
"></pre></td> <td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: \
0; ">const int ySymbolSpacing = 70;</pre></td>
</tr>
</tbody>
<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">35</font></th> <td bgcolor="#c5ffc4" \
width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span \
class="n">Legend</span><span class="o">::</span><span class="n">Legend</span><span \
class="p">(</span><span class="n">KStars</span> <span class="o">*</span><span \
class="n">kstars</span><span class="p">,</span> <span \
class="n">LEGEND_ORIENTATION</span> <span class="n">orientation</span><span \
class="p">)</span></pre></td> </tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; \
white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">KStars is \
now a single-instance class, so you can just use KStars::Instance(). Saves passing \
one parameter.</pre> </div>
<br />
<div>
<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="http://git.reviewboard.kde.org/r/101912/diff/1/?file=26665#file26665line54" \
style="color: black; font-weight: bold; text-decoration: \
underline;">kstars/legend.cpp</a> <span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; \
"></pre></td> <td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: \
0; ">const int ySymbolSpacing = 70;</pre></td>
</tr>
</tbody>
<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">54</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">Legend</span><span class="o">::</span><span \
class="n">getSymbolSize</span><span class="p">()</span></pre></td> </tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; \
white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">You could \
inline all of these right? Probably, the compiler will be smart enough to do that \
anyway, though. But it also removes some clutter if you put them as one-liners in the \
header file.</pre> </div>
<br />
<div>
<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="http://git.reviewboard.kde.org/r/101912/diff/1/?file=26665#file26665line470" \
style="color: black; font-weight: bold; text-decoration: \
underline;">kstars/legend.cpp</a> <span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; \
"></pre></td> <td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: \
0; ">const int ySymbolSpacing = 70;</pre></td>
</tr>
</tbody>
<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">470</font></th> <td bgcolor="#c5ffc4" \
width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span \
class="k">default</span><span class="o">:</span> <span class="k">return</span><span \
class="p">;</span> <span class="c1">// should never happen</span></pre></td> </tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; \
white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Maybe you \
should instead do a Q_ASSERT(false) so that we crash (I usually think it's better \
to crash and make sure the programmer fixes the problem than returning a non-result \
that might be harder to detect :D)</pre> </div>
<br />
<div>
<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="http://git.reviewboard.kde.org/r/101912/diff/1/?file=26666#file26666line267" \
style="color: black; font-weight: bold; text-decoration: \
underline;">kstars/skymap.h</a> <span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">class \
SkyMap : public QGraphicsView {</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" \
align="right"><font size="2">267</font></th> <td bgcolor="#ffffff" width="50%"><pre \
style="font-size: 8pt; line-height: 140%; margin: 0; "> <span \
class="kr">inline</span> <span class="kt">void</span> <span \
class="nf">exportSkyImage</span><span class="p">(</span> <span \
class="n">QPaintDevice</span> <span class="o">*</span><span class="n">pd</span> <span \
class="p">)</span> <span class="p">{</span> <span class="n">dynamic_cast</span><span \
class="o"><</span><span class="n">SkyMapDrawAbstract</span> <span \
class="o">*></span><span class="p">(</span><span \
class="n">m_SkyMapDraw</span><span class="p">)</span><span \
class="o">-></span><span class="n">exportSkyImage</span><span class="p">(</span> \
<span class="n">pd</span> <span class="p">);</span> <span \
class="p">}</span></pre></td> <th bgcolor="#f0f0f0" style="border-left: 1px solid \
#C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font \
size="2">267</font></th> <td bgcolor="#ffffff" width="50%"><pre style="font-size: \
8pt; line-height: 140%; margin: 0; "> <span class="kr">inline</span> <span \
class="kt">void</span> <span class="nf">exportSkyImage</span><span class="p">(</span> \
<span class="n">QPaintDevice</span> <span class="o">*</span><span class="n">pd</span> \
<span class="p">)</span> <span class="p">{</span> <span \
class="n">dynamic_cast</span><span class="o"><</span><span \
class="n">SkyMapDrawAbstract</span> <span class="o">*></span><span \
class="p">(</span><span class="n">m_SkyMapDraw</span><span class="p">)</span><span \
class="o">-></span><span class="n">exportSkyImage</span><span class="p">(</span> \
<span class="n">pd</span> <span class="p">);</span> <span \
class="p">}</span></pre></td> </tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; \
white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Is this \
still used? Or can it be done away with?</pre> </div>
<br />
<div>
<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="http://git.reviewboard.kde.org/r/101912/diff/1/?file=26671#file26671line375" \
style="color: black; font-weight: bold; text-decoration: \
underline;">kstars/skyqpainter.cpp</a> <span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">bool \
SkyQPainter::drawDeepSkyObject(DeepSkyObject* obj, bool drawImage)</pre></td>
</tr>
</tbody>
<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">375</font></th> <td bgcolor="#c5ffc4" \
width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span \
class="c1">// if size is 0.0 set it to 1.0, this are normally stars (type 0 and \
1)</span></pre></td> </tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; \
white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Could you \
explain this change? I'm not sure about the details, but do note that there are \
stars in the NGC and IC catalogs that are sometimes not part of the other star \
catalogs we use.
Edit: Sorry, I think I notice that the star's dia is set to 2.0 if it is less \
than 2.0 in drawDeepSkySymbol(...), whereby it will get drawn anyway. Am I right \
here?</pre> </div>
<br />
<div>
<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="http://git.reviewboard.kde.org/r/101912/diff/1/?file=26671#file26671line411" \
style="color: black; font-weight: bold; text-decoration: \
underline;">kstars/skyqpainter.cpp</a> <span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">void \
SkyQPainter::drawDeepSkySymbol(const QPointF& pos, DeepSkyObject* obj, float \
positionAngle)</pre></td> <td colspan="2"><pre style="font-size: 8pt; line-height: \
140%; margin: 0; "></pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" \
align="right"><font size="2">403</font></th> <td bgcolor="#fdfebc" width="50%"><pre \
style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="kt">void</span> \
<span class="n">SkyQPainter</span><span class="o">::</span><span \
class="n">drawDeepSkySymbol</span><span class="p">(</span><span \
class="k">const</span> <span class="n">QPointF</span><span class="o"><span \
class="hl">&</span></span><span class="hl"> </span><span \
class="n">pos</span><span class="p">,</span> <span class="n"><span \
class="hl">DeepSkyObject</span></span><span class="o"><span \
class="hl">*</span></span><span class="hl"> </span><span class="n"><span \
class="hl">obj</span></span><span class="p">,</span> <span class="kt">float</span> \
<span class="n">positionAngle</span><span class="p">)</span></pre></td> <th \
bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid \
#C0C0C0;" align="right"><font size="2">411</font></th> <td bgcolor="#fdfebc" \
width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span \
class="kt">void</span> <span class="n">SkyQPainter</span><span \
class="o">::</span><span class="n">drawDeepSkySymbol</span><span \
class="p">(</span><span class="k">const</span> <span class="n">QPointF</span> <span \
class="o"><span class="hl">&</span></span><span class="n">pos</span><span \
class="p">,</span> <span class="kt"><span class="hl">int</span></span><span \
class="hl"> </span><span class="n"><span class="hl">type</span></span><span \
class="p"><span class="hl">,</span></span><span class="hl"> </span><span \
class="kt"><span class="hl">float</span></span><span class="hl"> </span><span \
class="n"><span class="hl">size</span></span><span class="p"><span \
class="hl">,</span></span><span class="hl"> </span><span class="kt"><span \
class="hl">float</span></span><span class="hl"> </span><span class="n"><span \
class="hl">e</span></span><span class="p">,</span> <span class="kt">float</span> \
<span class="n">positionAngle</span><span class="p">)</span></pre></td> </tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; \
white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Makes \
sense!</pre> </div>
<br />
<p>- Akarsh</p>
<br />
<p>On July 10th, 2011, 10:38 p.m., Rafal Kulaga wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" \
style="background-image: \
url('http://git.reviewboard.kde.org/media/rb/images/review_request_box_top_bg.png'); \
background-position: left top; background-repeat: repeat-x; border: 1px black \
solid;"> <tr>
<td>
<div>Review request for KStars, Victor Carbune and Akarsh Simha.</div>
<div>By Rafal Kulaga.</div>
<p style="color: grey;"><i>Updated July 10, 2011, 10:38 p.m.</i></p>
<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;">Attached diff adds the option to include legends in exported sky images. \
Both vertical and horizontal orientations are supported; there are two types of \
legend: full-blown (symbol descriptions, star magnitudes and scale) and scale-only. \
Some changes will inevitably be made to this code in a few days - please note that \
there are some hard-coded values in legend.cpp which will be gone after integration \
with the functionality I am developing now (FOV representation exporting).
Any comments are welcome, be they look&feel or code-related.
</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;">Done some testing, everything worked (and looked) fine for me.</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>kstars/CMakeLists.txt <span style="color: grey">(0c335b6)</span></li>
<li>kstars/dialogs/exportimagedialog.h <span style="color: \
grey">(PRE-CREATION)</span></li>
<li>kstars/dialogs/exportimagedialog.cpp <span style="color: \
grey">(PRE-CREATION)</span></li>
<li>kstars/dialogs/exportimagedialog.ui <span style="color: \
grey">(PRE-CREATION)</span></li>
<li>kstars/kstarsactions.cpp <span style="color: grey">(e917cac)</span></li>
<li>kstars/kstarsdcop.cpp <span style="color: grey">(42dcb0f)</span></li>
<li>kstars/legend.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>kstars/legend.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>kstars/skymap.h <span style="color: grey">(e7a7f56)</span></li>
<li>kstars/skymapdrawabstract.h <span style="color: grey">(c94a745)</span></li>
<li>kstars/skymapdrawabstract.cpp <span style="color: grey">(cf44fc5)</span></li>
<li>kstars/skypainter.h <span style="color: grey">(1340568)</span></li>
<li>kstars/skyqpainter.h <span style="color: grey">(df7cc9b)</span></li>
<li>kstars/skyqpainter.cpp <span style="color: grey">(87719b2)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/101912/diff/" style="margin-left: \
3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>
_______________________________________________
Kstars-devel mailing list
Kstars-devel@kde.org
https://mail.kde.org/mailman/listinfo/kstars-devel
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic