davidhurka added a comment.
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. INLINE COMMENTS > aacid wrote in area.h:36 > i don't see why your version of the text is better. Not sure. Shall I change it back? The paragraph “Coordinate System” should make it clear anyway. > aacid wrote in area.h:47 > zoom doesn't seem the appropiate word here, there's no zooming involved Hmm, agreed. Will change it back. > aacid wrote in area.h:69 > why "the" ? I thought that all instances will be equal points, so “the” would be appropiate. Usually it goes “creates a null ...”, so “a” is probaby better. BTW: There is no NormalizedPoint::isNull(), so why this constructor? > aacid wrote in area.h:80 > I don't like that you mention a page here. One can scale with a factor or with a divisor, and will probaby call both scaling //factor//. 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. > aacid wrote in area.h:98 > same, this is not striclty page related 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. * > aacid wrote in area.h:128 > This information makes no sense here. TextEntity provides example usage, so someone who wonders why this class exists will be happy. Throwing in this link, because it mentions examples. https://community.kde.org/Guidelines_and_HOWTOs/API_Documentation#Writing_APIDOX_in_New_Code I will remove the “glyphs, words, line...”, that’s too specific. > aacid wrote in area.h:131 > What a regularAreaRect is doesn't belong to the description of NormalizedRect > (imho, the @see is fine) 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. > aacid wrote in area.h:872 > This comment is weird, it seems to imply such functionality exists in this > class, when it doesn't. Doesn’t RegularAreaRect::geometry() do that? REPOSITORY R223 Okular REVISION DETAIL https://phabricator.kde.org/D21266 To: davidhurka, #okular Cc: aacid, okular-devel, joaonetto, tfella, ngraham, darcyshen
