aacid added a comment.
You really need to remove all the mentions of page you're adding, this is
geometry that has nothing to do with pages.
Yes in Okular in 99.99% of the cases it's used in conjunction with a page,
but it is not about pages, it just so happen that most of the needs okular has
regarding geometry is about pages and its contents.
INLINE COMMENTS
> area.h:63
> + *
> + * NormalizedRect provides additional geometric operations recarding
> rectangles.
> + *
recarding typeo
> davidhurka wrote in area.h:98
> I think in this case it is strictly page //size// related, because it unifies
> vertical and horizontal distance using Phytagoras.
>
> Without the page size, the result would be meaningless in many cases.
>
> 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?
>
> If you wish to keep the documemtation compatible to future use cases, I could
> just clarify the scaling. Examle:
>
> * @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.
> *
It is not page size related *at all*
It's just two points, one of them has a scaling. Nothing here is necessarily
about pages.
> davidhurka wrote in area.h:108
> Is there a reason to make this one function `static`?
i don't know, you may want to ask Peter, but it was 6 years ago, i doubt he
remembers.
What's your problem with it being static?
> davidhurka wrote in area.h:192
> This is implemented with double == 0. Won’t this always return true?
>
> bool NormalizedRect::isNull() const
> {
> return left == 0 && top== 0 && right == 0 && bottom == 0;
> }
Why would it always return true?
When left is 0.5 it will obviously return false, no?
> davidhurka wrote in area.h:229
> What’s the purpose of rounding? The application code looks like it doesn’t
> matter.
I don't understand the question. The purpose it is "it's rounded and thus
potentially more accurate than without rounding because 0.9 will be 1 instead
of 0"? i mean it's like you're asking "what is rounding"?
> davidhurka wrote in area.h:308
> Adjectives like ‘left’ or ‘top’ aren’t very precise. Is the rectangle left of
> the point or is the point left of the rectangle?
>
> Additionally, this is inconsistent to ‘isTop’ <-> ‘isTopOrLevel’. I suggest
> to rename the functions to names like:
>
> - isAboveOrLevel
> - isLeftOrAmong
> - isLeftOfOrAlong
Breaking API is a big no no just because you don't like the name.
The documentation clearly says what left means.
> davidhurka wrote in area.h:379
> “A reference to an ‘object’” is very broad.
>
> This is an area on a page (like RegularAreaRect), and links to an Annotation
> or SourceReference, or something “not owned”.
>
> Some bad alternative ideas to ‘ObjectRect’: ;)
>
> - InterestingArea
> - AreaToClick
> - DetailArea
> - ActiveArea
> - IntelligentArea
> - SpeciaArea
> - ReferencingArea
We're not renaming ObjectRect.
> davidhurka wrote in area.h:386
> What does this mean?
No idea, you're the first peson ever that probably reads this line :D
I used the power of git history and i think tab means table and the table tries
to explain which kind of object the object() function returns.
> davidhurka wrote in area.h:412
> What’s the purpose of ‘ellipse’? Currently it’s not used.
>
> Are any of these parameters used at all?
What do you mean ellipse is not used?
It is used here, isn't it?
https://cgit.kde.org/okular.git/tree/core/area.cpp#n314
> davidhurka wrote in area.h:456
> The base class implementation just ignores the page size and takes the point
> as normalized.
>
> This method is not used anywhere. Maybe it should be fixed, ore be pure
> virtual?
What do you mean it's not used anywhere? Have you tried marking it as pure
virtual?
REPOSITORY
R223 Okular
REVISION DETAIL
https://phabricator.kde.org/D21266
To: davidhurka, #okular
Cc: aacid, okular-devel, joaonetto, tfella, ngraham, darcyshen
