D21281: [RFC] Write a bit Documentation for Part and PageView

[David Hurka]

davidhurka added a comment.


  Didn’t get far yet. Much information is visible in the source code, but what 
should I put into the class/member documentation, and what is clear just 
because it’s a Part?
  
  Is there some more KParts literature? I could find KParts on api.kde.org and 
the tutorials on community.kde.org, but couldn’t really transfer that to Okular.

REPOSITORY
  R223 Okular

REVISION DETAIL
  https://phabricator.kde.org/D21281

To: davidhurka, #okular
Cc: okular-devel, joaonetto, tfella, ngraham, darcyshen, aacid

D21281: [RFC] Write a bit Documentation for Part and PageView

[David Hurka]

davidhurka created this revision.
davidhurka added a reviewer: Okular.
Herald added a project: Okular.
Herald added a subscriber: okular-devel.
davidhurka requested review of this revision.

REVISION SUMMARY
  This shall add some documentation to the classes Part and PageView, 
  to describe their purposes.
  
  The intention comes from D21195 , where I 
am thinking whether I should
  mess up Okular::Part instead of Okular::PageView.

TEST PLAN
  Run doxygen

REPOSITORY
  R223 Okular

BRANCH
  create-part-and-pageview-class-documentation

REVISION DETAIL
  https://phabricator.kde.org/D21281

AFFECTED FILES
  part.h

To: davidhurka, #okular
Cc: okular-devel, joaonetto, tfella, ngraham, darcyshen, aacid

[okular] [Bug 407463] Improve straight line annotation style

[David Hurka]

https://bugs.kde.org/show_bug.cgi?id=407463

--- Comment #4 from David Hurka  ---
(In reply to Tobias Deiminger from comment #3)
> - LL+Slash: No conflict. No idea what this is good for.

Sometimes, / is used instead of > for dimensions. But the slash should be
centered on the leader line, and not shorten the line like in attachment
120012.

-- 
You are receiving this mail because:
You are the assignee for the bug.

D21271: [WIP] Improve documentation of TextEntity stuff

[David Hurka]

davidhurka added a comment.


  TextPagePrivate::correctTextOrder() calls some complex functions, which are 
yet undocumented. Interesting stuff is happening in them, so they should get 
some documentation. I added their prototypes to core/textpage_p.h, so I can add 
documentation to them.
  
  In D21271#466691 , @yurchor wrote:
  
  > Thanks for fixing these typos.
  
  
  Thanks for spotting these typos. :)

INLINE COMMENTS

> textpage.cpp:1880
>  {
> -//m_page->m_page->width() and m_page->m_page->height() are in pixels at
> -//100% zoom level, and thus depend on display DPI. We scale pageWidth and
> -//pageHeight to remove the dependence. Otherwise bugs would be more 
> difficult
> -//to reproduce and Okular could fail in extreme cases like a large TV 
> with low DPI.
> +// m_page->width() and m_page->height() are in pixels at
> +// 100% zoom level, and thus depend on display DPI.

This is some interesting information, which should be documented more visible. 
Is the information still true?

> textpage.cpp:1883
> +// To avoid Okular failing on lowDPI displays,
> +// we scale pageWidth and pageHeight so their sum equals 2000.
>  const double scalingFactor = 2000.0 / (m_page->width() + 
> m_page->height());

Is this enough for documents with high text density? At least, this is a good 
reason for NormalizedRect::roundedGeometry().

> textpage.h:52
> + *
> + * @par Vertical Text
> + * Currently, the reordering mixes up TextEntitys which represent glyphs or 
> words of vertical text.

Documentation can’t fix https://bugs.kde.org/show_bug.cgi?id=407133. As soon as 
TextPagePrivate::correctTextOrder handles vertical text, this paragraph can be 
removed.

REPOSITORY
  R223 Okular

REVISION DETAIL
  https://phabricator.kde.org/D21271

To: davidhurka, #okular
Cc: yurchor, okular-devel, joaonetto, tfella, ngraham, darcyshen, aacid

D21271: [WIP] Improve documentation of TextEntity stuff

[Yuri Chornoivan]

yurchor added a comment.


  Thanks for fixing these typos.

INLINE COMMENTS

> textpage.h:179
>  /**
> - * Text extraction function.
> + * Text extraction function. Looks for text in the given @param 
> area, and concatenates it to a string.
>   *

Should be "@p area".

http://www.doxygen.nl/manual/commands.html#cmdp

> textpage_p.h:22
> +/**
> + * Memory-optimized storage of a TextEntity. Stores a string and it’s 
> bounding box.
> + *

Typo: it's bounding box -> its bounding box

REPOSITORY
  R223 Okular

REVISION DETAIL
  https://phabricator.kde.org/D21271

To: davidhurka, #okular
Cc: yurchor, okular-devel, joaonetto, tfella, ngraham, darcyshen, aacid

D21271: [WIP] Improve documentation of TextEntity stuff

[David Hurka]

davidhurka created this revision.
davidhurka added a reviewer: Okular.
davidhurka added a project: Okular.
Herald added a subscriber: okular-devel.
davidhurka requested review of this revision.

REVISION SUMMARY
  This adds some important documentation on TextEntity and other classes, and 
improves some of the existing documentation.
  
  This includes changing parameter names from ‘rect’ to ‘area’, because I found 
‘rect’ misleading.

TEST PLAN
  Run doxygen

REPOSITORY
  R223 Okular

BRANCH
  improve-documentation-of-textentity-handling

REVISION DETAIL
  https://phabricator.kde.org/D21271

AFFECTED FILES
  core/page.cpp
  core/textpage.cpp
  core/textpage.h
  core/textpage_p.h

To: davidhurka, #okular
Cc: okular-devel, joaonetto, tfella, ngraham, darcyshen, aacid

[okular] [Bug 407463] Improve straight line annotation style

[Tobias Deiminger]

https://bugs.kde.org/show_bug.cgi?id=407463

--- Comment #3 from Tobias Deiminger  ---
(In reply to Simone Gaiarin from comment #2)
> You are right. I have not thought about that use case.

But you're also right in that not all combinations make sense.

Let's check, how could leader line + arrow X be used?
- LL+Square: Partially overlapping. No idea what this is good for.
- LL+Circle: No visual conflict. No idea what this is good for.
- LL+Diamond: No visual conflict. No idea what this is good for.
- LL+OpenArrow: Best for dimensioning of longer distances.
- LL+ClosedArrow: Similar to OpenArrow.
- LL+None: No conflict.
- LL+Butt: Complete conflict.
- LL+ROpenArrow: Best for dimensioning of shorter distances. But we must arrows
"outside" leader for this use case (currently bug?).
- LL+RClosedArrow: Similar to ROpenArrow.
- LL+Slash: No conflict. No idea what this is good for.

In attachment 120022 I faked the text "20 mm" by using typewriter. That's
clumsy, we should better use the native caption feature of line annotations.
It's implemented in poppler as LineAnnotation::lineShowCaption, but Okular
does't expose this setting => additional item for improvement.

Unrelated: Square, circle, diamond, and closed arrows can be filled with a
color. Fill color is not yet exposed in Okular GUI => additional item for
improvement.

So there are multiple things to improve, and some of them need more
consideration. Would you mind turning this bug into a Phabricator task?

-- 
You are receiving this mail because:
You are the assignee for the bug.

[okular] [Bug 407338] Okular segfaults when opening a file digitally signed with portablesigner and Acrobat Reader

[Albert Astals Cid]

https://bugs.kde.org/show_bug.cgi?id=407338

Albert Astals Cid  changed:

   What|Removed |Added

 CC||saskat...@net-c.ca

--- Comment #4 from Albert Astals Cid  ---
*** Bug 407670 has been marked as a duplicate of this bug. ***

-- 
You are receiving this mail because:
You are the assignee for the bug.

[okular] [Bug 407670] Okular crashes when opening a PDF file with several signatures in it.

[Albert Astals Cid]

https://bugs.kde.org/show_bug.cgi?id=407670

Albert Astals Cid  changed:

   What|Removed |Added

 Resolution|--- |DUPLICATE
 Status|REPORTED|RESOLVED
 CC||aa...@kde.org

--- Comment #1 from Albert Astals Cid  ---
This bug report is not very good.

You're neither including the pdf file nor the segmentation fault backtrace.

anyhow, duplicate

*** This bug has been marked as a duplicate of bug 407338 ***

-- 
You are receiving this mail because:
You are the assignee for the bug.

[okular] [Bug 407463] Improve straight line annotation style

[Simone Gaiarin]

https://bugs.kde.org/show_bug.cgi?id=407463

--- Comment #2 from Simone Gaiarin  ---
You are right. I have not thought about that use case.

-- 
You are receiving this mail because:
You are the assignee for the bug.

D21248: Add line annotation ending arrows for non PDF documents

[Tobias Deiminger]

tobiasdeiminger retitled this revision from "WIP: Add line annotation ending 
arrows for non PDF documents" to "Add line annotation ending arrows for non PDF 
documents".
tobiasdeiminger edited the summary of this revision.
tobiasdeiminger edited the test plan for this revision.

REPOSITORY
  R223 Okular

REVISION DETAIL
  https://phabricator.kde.org/D21248

To: tobiasdeiminger
Cc: sander, knambiar, okular-devel, joaonetto, tfella, ngraham, darcyshen, aacid

D21248: WIP: Add line annotation ending arrows for non PDF documents

[Tobias Deiminger]

tobiasdeiminger updated this revision to Diff 58238.
tobiasdeiminger added a comment.


  Added circle, expose LineAnnotPainer.

REPOSITORY
  R223 Okular

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D21248?vs=58184&id=58238

BRANCH
  feature/drawlineendings

REVISION DETAIL
  https://phabricator.kde.org/D21248

AFFECTED FILES
  ui/pagepainter.cpp
  ui/pagepainter.h

To: tobiasdeiminger
Cc: sander, knambiar, okular-devel, joaonetto, tfella, ngraham, darcyshen, aacid

[okular] [Bug 406237] PDFs get an added margin when printing

[Michael Weghorn]

https://bugs.kde.org/show_bug.cgi?id=406237

--- Comment #10 from Michael Weghorn  ---
(In reply to Matthew Trescott from comment #7)
> [...]
> I just double-checked this to make certain. I turned off WiFi temporarily so
> CUPS would hold my print job (it's a network printer), then printed my
> Letter-size PDF. I double-checked the printer properties in Okular to make
> sure that it was set to Letter (Letter is the default for the print queue
> so the paper size was already correctly set). However, when I opened the
> corresponding PostScript document in /var/spool/cups, it had the distinctive
> sqrt(2) side ratio of A-size paper. This confirms the results I had when I
> permitted the print jobs to go through previously and discovered this bug---I
> just didn't want to waste paper this time.
> 
> Summary: the bug here is that Okular always prints in A4 to physical
> printers,
> even when a different paper size has been selected.

This didn't happen in my case. Can you attach the generated PostScript file as
well as the PPD of your printer here (which is located at
'/etc/cups/ppd/.ppd)?


> But mightn't it be the responsibility of the application using the Qt print
> dialog (Okular in this case) to tell Qt what paper size to default to for
> "Print to PDF"? My wild guess at the internals of how this works is just that
> the application provides a PDF which Qt forwards unmodified to the OS's print
> server, along with the metadata needed to adjust the printer settings. I
> doubt
> the Qt print dialog would be smart enough to parse the PDF it gets (if indeed
> my theory of how it works is correct) to figure out what paper size to use.
> 
> Summary: the bug here is that the Printer Properties dialog for Print to PDF
> defaults to A4 for the paper size, rather than the size of the document
> being printed.

I guess the application *could* set the paper size. I'd say whether or not this
is desirable depends on the use case. As mentioned in my previous comment for
the real physical printer, I'm happy with the A4 default. In my opinion, this
is not a bug at a quick glance, but would rather be an enhancement request to
change it if you think using the current document's size should be preselected.
(Choosing the "correct" default is hard and you'll probably never find any that
everybody is happy with...)


> Ah, yes. It does work, and it makes sense since PDF files don't seem to have
> any semantic concept of margins. But in that case, "Fit to printable area"
> should not be the default scaling setting, since it adds margins where most
> PDFs will probably not need them.
> 
> Summary: the bug here is that that the "Fit to printable area" scaling
> setting is the default, rather than "Fit to full page"

Whatever should be the "correct" default can for sure be discussed. Basically
the same as above: It's not really a bug in my opinion. If you think it should
be changed, I'd say: Feel free to file a request to change this and let's see
what decision will be taken.
Note that there's already bug 404510 that requests the possibilty to store the
preferred scaling setting, so maybe this would "fix" your case as well?


> > > - "Print original size" shifts the entire document down and to the right.
> > 
> > I can confirm this with default margins set. Again: Does this still happen
> > when you set all margins to 0? (no longer happens then in my case)
> > This might need a closer look, not sure if it's supposed to work like this.
> 
> Indeed, setting margins to zero does cause it to be positioned correctly.
> However, I would guess this setting should be called "crop to fit printable
> area" and make the horizontal cropping equal between the left and right
> sides,
> and the vertical cropping equal between top and bottom. Currently it's not
> very useful.
> 
> Summary: the bug here is that "Print original size" shifts the document
> content to fit the top and left margins, rather than cropping the document
> on all sides to fit the margins.

At a quick glance, there's mainly two options to handle the current options:

A) what happens currently: print margins are honoured; the document is not
scaled, so the painting starts at the top-left of the printable area with
original size. Whatever doesn't fit, just get's lost.
B) alternative: print margins are ignored when the "Print original size" is
selected. If the document has sufficient margins, this is fine, otherwise parts
might get cropped.

If the document is smaller than the paper size (e.g. printing A5 to A4) and the
document doesn't have proper margins, A) might be desirable, but I see that
e.g. when printing A4 to A4, B) may be preferrable.
I'm personally not sure how to best deal with this.
(The original suggestion in https://phabricator.kde.org/D7962 had two separate
options to select what kind of scaling to apply and whether or not to take
print margins into account, so the user would have been able to choose what
option to use here. However, a single combobox was consi