<table><tr><td style="">davidhurka added a comment.
</td><a style="text-decoration: none; padding: 4px 8px; margin: 0 8px 8px; float: right; color: #464C5C; font-weight: bold; border-radius: 3px; background-color: #F7F7F9; background-image: linear-gradient(to bottom,#fff,#f1f0f1); display: inline-block; border: 1px solid rgba(71,87,120,.2);" href="https://phabricator.kde.org/D21266">View Revision</a></tr></table><br /><div><div><p>Should the section “Coordinate System” of NormalizedPoint mention the “Trimmed Margins” feature? To prevent that someone sloppily codes a feature, which draws something on the page and fails if Trim Margins is active.</p></div></div><br /><div><strong>INLINE COMMENTS</strong><div><div style="margin: 6px 0 12px 0;"><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D21266#inline-119502">View Inline</a><span style="color: #4b4d51; font-weight: bold;">aacid</span> wrote in <span style="color: #4b4d51; font-weight: bold;">area.h:36</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">i don't see why your version of the text is better.</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Not sure. Shall I change it back?</p>

<p style="padding: 0; margin: 8px;">The paragraph “Coordinate System” should make it clear anyway.</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D21266#inline-119503">View Inline</a><span style="color: #4b4d51; font-weight: bold;">aacid</span> wrote in <span style="color: #4b4d51; font-weight: bold;">area.h:47</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">zoom doesn't seem the appropiate word here, there's no zooming involved</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Hmm, agreed. Will change it back.</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D21266#inline-119504">View Inline</a><span style="color: #4b4d51; font-weight: bold;">aacid</span> wrote in <span style="color: #4b4d51; font-weight: bold;">area.h:69</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">why "the" ?</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">I thought that all instances will be equal points, so “the” would be appropiate.</p>

<p style="padding: 0; margin: 8px;">Usually it goes “creates a null ...”, so “a” is probaby better. BTW: There is no NormalizedPoint::isNull(), so why this constructor?</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D21266#inline-119505">View Inline</a><span style="color: #4b4d51; font-weight: bold;">aacid</span> wrote in <span style="color: #4b4d51; font-weight: bold;">area.h:80</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">I don't like that you mention a page here.</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">One can scale with a factor or with a divisor, and will probaby call both scaling <em>factor</em>. So what should xScale or yScale be? I think saying “page size” makes it clear. Using a PageSize or ScalingFactor struct would make it clear as well.</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D21266#inline-119506">View Inline</a><span style="color: #4b4d51; font-weight: bold;">aacid</span> wrote in <span style="color: #4b4d51; font-weight: bold;">area.h:98</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">same, this is not striclty page related</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">I think in this case it is strictly page <em>size</em> related, because it unifies vertical and horizontal distance using Phytagoras.</p>

<p style="padding: 0; margin: 8px;">Without the page size, the result would be meaningless in many cases.</p>

<p style="padding: 0; margin: 8px;">Besides that, NormalizedPoint classes could be used as abstract coordinate system convenience classes, for anything else with varying absolute sizes. Can we expect anything else than pages?</p>

<p style="padding: 0; margin: 8px;">If you wish to keep the documemtation compatible to future use cases, I could just clarify the scaling. Examle:</p>

<div class="remarkup-code-block" style="margin: 12px 0;" data-code-lang="text" data-sigil="remarkup-code-block"><pre class="remarkup-code" style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; padding: 12px; margin: 0; background: rgba(71, 87, 120, 0.08);">* @par Scaling
* To convert a NormalizedPoint to a point with absolute coordinates,
* the normalized coordinate system needs to be mapped to the reference area.
* This can be done with two scaling factors, xScale and yScale.
* These will just be equal to the width and height of the reference area.
*</pre></div></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D21266#inline-119508">View Inline</a><span style="color: #4b4d51; font-weight: bold;">aacid</span> wrote in <span style="color: #4b4d51; font-weight: bold;">area.h:128</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">This information makes no sense here.</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">TextEntity provides example usage, so someone who wonders why this class exists will be happy.</p>

<p style="padding: 0; margin: 8px;">Throwing in this link, because it mentions examples.<br />
<a href="https://community.kde.org/Guidelines_and_HOWTOs/API_Documentation#Writing_APIDOX_in_New_Code" class="remarkup-link" target="_blank" rel="noreferrer">https://community.kde.org/Guidelines_and_HOWTOs/API_Documentation#Writing_APIDOX_in_New_Code</a></p>

<p style="padding: 0; margin: 8px;">I will remove the “glyphs, words, line...”, that’s too specific.</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D21266#inline-119509">View Inline</a><span style="color: #4b4d51; font-weight: bold;">aacid</span> wrote in <span style="color: #4b4d51; font-weight: bold;">area.h:131</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">What a regularAreaRect is doesn't belong to the description of NormalizedRect (imho, the <span class="phabricator-remarkup-mention-unknown">@see</span> is fine)</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">It provides an alternative to NormalizedRect. NormalizedRect is already pretty good for describing shapes on a page, but in some cases RegularAreaRect will be better.</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D21266#inline-119501">View Inline</a><span style="color: #4b4d51; font-weight: bold;">aacid</span> wrote in <span style="color: #4b4d51; font-weight: bold;">area.h:872</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">This comment is weird, it seems to imply such functionality exists in this class, when it doesn't.</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Doesn’t RegularAreaRect::geometry() do that?</p></div></div></div></div></div><br /><div><strong>REPOSITORY</strong><div><div>R223 Okular</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D21266">https://phabricator.kde.org/D21266</a></div></div><br /><div><strong>To: </strong>davidhurka, Okular<br /><strong>Cc: </strong>aacid, okular-devel, joaonetto, tfella, ngraham, darcyshen<br /></div>