[Okular-devel] [okular] [Bug 322310] Konqueror crashed trying to open pdf from sukl.cz
https://bugs.kde.org/show_bug.cgi?id=322310 --- Comment #5 from kavol ka...@seznam.cz --- (In reply to comment #3) Fixed in 4.10.5 cool, thanks a lot -- You are receiving this mail because: You are the assignee for the bug. ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
[Okular-devel] [okular] [Bug 323639] New: Can't click on items in the contents list
https://bugs.kde.org/show_bug.cgi?id=323639 Bug ID: 323639 Summary: Can't click on items in the contents list Classification: Unclassified Product: okular Version: 0.17.0 Platform: Ubuntu Packages URL: http://www.canon.co.uk/Support/Consumer_Products/produ cts/cameras/Digital_Compact/Powershot_SX_series/PowerS hot_SX500_IS.aspx?DLtcmuri=tcm:14-974016page=1type=d ownload OS: Linux Status: UNCONFIRMED Severity: normal Priority: NOR Component: general Assignee: okular-devel@kde.org Reporter: mfraz74+...@gmail.com If I view the PDF in the URL there is a list of contents, but I am unable to click on any of them to go to the selected page. Reproducible: Always Steps to Reproduce: 1.Load PDF 2.Try to select an item from the contents 3.Find that the page doesn't change. Expected Results: Okular should go to the page I've selected from the contents. -- You are receiving this mail because: You are the assignee for the bug. ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
[Okular-devel] [okular] [Bug 323639] Can't click on items in the contents list
https://bugs.kde.org/show_bug.cgi?id=323639 Albert Astals Cid aa...@kde.org changed: What|Removed |Added Priority|NOR |HI Status|UNCONFIRMED |CONFIRMED CC||aa...@kde.org Ever confirmed|0 |1 --- Comment #1 from Albert Astals Cid aa...@kde.org --- Probably a poppler issue. Someone should investigate and see if we can reassign to them (probably yes as evince has the same problem). -- You are receiving this mail because: You are the assignee for the bug. ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
Re: [Okular-devel] Review Request 110914: Tabbed interface
On Aug. 14, 2013, 10:29 p.m., Albert Astals Cid wrote: shell/shell.h, line 49 http://git.reviewboard.kde.org/r/110914/diff/2/?file=176095#file176095line49 Why are you changing the dbus names? This will break whatever scripts people where using. Jonathan Doman wrote: It seems my experiences were caused by a bug/feature in qdbusviewer (https://bugreports.qt-project.org/browse/QTBUG-6943). So, while technically it's okay to keep the interface names how they are, it makes more sense to me that they be different. Please bring it back, changing existing interface names should be avoided unless totally necessary. - Albert --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110914/#review37814 --- On Aug. 3, 2013, 10:03 p.m., Jonathan Doman wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110914/ --- (Updated Aug. 3, 2013, 10:03 p.m.) Review request for Okular. Description --- This patch adds support for a tabbed interface (multiple documents in one window). The core work just adds a tab bar that switches between multiple embedded okularparts, but there are many other considerations: - Tab context menu allows for duplicating or detaching (detached tabs start in new okular process) - `okular file.pdf` will open file in existing window if possible, unless --new flag is used. It also selects the most recently raised/activated window to use. This mirrors behavior I expect from browsers and other tabbed interfaces. - Warns when closing window with multiple tabs - No warning is given when opening an already open file. This is the behavior I strongly prefer (and observe in other programs), but will change if there is consensus otherwise. When selecting different tools in one part, the tool selection propagates to all parts, but the GUI does not reflect that. This bug is present in other programs (e.g. multiple okularparts in Konqueror), so I made no attempt to diagnose or fix. One menu item was added for the multiple tab warning option. When testing this, I noticed that items in the Settings menu seem to move around when switching tabs, and I cannot diagnose or fix this. It seems to be related to XMLGUI bug #64754. My development branch is also hosted at https://github.com/jrmrjnck/okular-tabbed This addresses bug 155515. http://bugs.kde.org/show_bug.cgi?id=155515 Diffs - part.cpp 71c3d0f5d908969ffcf18aba327297ccd2e1c8e1 part.h 4b3aafdb637080ae81eb0e45742f53a34738984d shell/main.cpp e0ca587ba167c4020d5af5bd33a2dc1b4923cac4 shell/shell.h c065c560fb4ddfcf181601cf35e9ca14581731f6 shell/shell.cpp 1708501daaef817a1ce35fa5d96701a66ab66983 shell/shell.rc 93fbc417588312792bab39b693c65e5d414c87c6 Diff: http://git.reviewboard.kde.org/r/110914/diff/ Testing --- Thanks, Jonathan Doman ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
Re: [Okular-devel] Review Request 111829: Use DPI of current screen for PDF rendering
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/111829/#review38036 --- Yeah, i'd drop the multiplication probably. After that i'll give a final review, but i think it's good to go. - Albert Astals Cid On Aug. 17, 2013, 12:02 a.m., Eugene Shalygin wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/111829/ --- (Updated Aug. 17, 2013, 12:02 a.m.) Review request for Okular and Albert Astals Cid. Description --- This patch relies on master branch of LibKScreen. This patch does not solve the problem in bug completely, but makes Okular behaviour more correct (see below). The problem (in the bug) is that Okular uses fixed DPI for PDF rendering (72.0 dots per inch) and therefore scale of rendered document becomes incorrect. With current mainline laptop screens having DPI easily twice larger than this constant (72), the problem shows itself quiet strongly. Additional problems araise with multiscreen configuration, when 1) DPI of each individual screen may be different, and 2) there is no tools in Qt to detect DPI of individual screens in virtual desktop mode. Therefore XRandr has to be used for DPI detection. Raw XRandr is bad dependency for Okular and libkscreen is proposed instead. This patch approach to the solution in the following way: 1. libkscreen detection staff is added to CMakeLists.txt and config.h 2. It changes Utils::realDpi() function to use libkscreen if present. With libkscreen the function looks for output that contains maximal part of given widget (suppose to be used for document rendering) and returns DPI of that screen. 3. Genenerator interface is extended by adding UtilizeDPI feature and setDPI() method, that is called by Document class right before calling loadXXX() if the generator supports this feature 4. Poppler generator is changed to support UtilizeDPI feature. 5. PageSizeMetric enum is extended with Pixels value to explicitly say that page size is defined in screen pixels (see my posts in the bug); Poppler generator uses this case. To completetly fix the bug, Document must invalidate generated pixmaps after the widget movements into another screen. I do not know how to track this without subclassing the main window class. Therefore I decided to publish this part of work to get your responce, especially regarding item 3 (Generator class changes). In the current state, manual reloading of a document after moving Okular to another screen fixes the scale, that is, in my eyes, is quiet helpful already. Even if we subclass the Okular main window, I do not know what to do when Okular is used as KPart. This addresses bug 268757. http://bugs.kde.org/show_bug.cgi?id=268757 Diffs - CMakeLists.txt 217337f config-okular.h.cmake 7217f8d core/document.cpp 3af92c8 core/document_p.h 3a257de core/generator.h a5a971b core/generator.cpp 41beb92 core/generator_p.h b59293a core/utils.h 8d5d5fc core/utils.cpp 5dd8448 generators/poppler/generator_pdf.h 5d5853a generators/poppler/generator_pdf.cpp 1a44523 Diff: http://git.reviewboard.kde.org/r/111829/diff/ Testing --- Manual. In all screens, that report correct physical size to XRandr, size of documents is correct Thanks, Eugene Shalygin ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
[Okular-devel] [okular] [Bug 323267] Adding and removing bookmarks in the thumbnail panel acts on the current page instead
https://bugs.kde.org/show_bug.cgi?id=323267 Albert Astals Cid aa...@kde.org changed: What|Removed |Added CC||mail...@gmail.com --- Comment #1 from Albert Astals Cid aa...@kde.org --- Mailson you broke this. -- You are receiving this mail because: You are the assignee for the bug. ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
[Okular-devel] [okular] [Bug 323262] Find previous fails at end of line
https://bugs.kde.org/show_bug.cgi?id=323262 --- Comment #2 from Albert Astals Cid aa...@kde.org --- For completion: Jaan says he is interested in fixing this in bug 323263 -- You are receiving this mail because: You are the assignee for the bug. ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
Re: [Okular-devel] Review Request 110914: Tabbed interface
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110914/ --- (Updated Aug. 17, 2013, 3:06 p.m.) Review request for Okular. Changes --- Reverted changes to DBus names Description --- This patch adds support for a tabbed interface (multiple documents in one window). The core work just adds a tab bar that switches between multiple embedded okularparts, but there are many other considerations: - Tab context menu allows for duplicating or detaching (detached tabs start in new okular process) - `okular file.pdf` will open file in existing window if possible, unless --new flag is used. It also selects the most recently raised/activated window to use. This mirrors behavior I expect from browsers and other tabbed interfaces. - Warns when closing window with multiple tabs - No warning is given when opening an already open file. This is the behavior I strongly prefer (and observe in other programs), but will change if there is consensus otherwise. When selecting different tools in one part, the tool selection propagates to all parts, but the GUI does not reflect that. This bug is present in other programs (e.g. multiple okularparts in Konqueror), so I made no attempt to diagnose or fix. One menu item was added for the multiple tab warning option. When testing this, I noticed that items in the Settings menu seem to move around when switching tabs, and I cannot diagnose or fix this. It seems to be related to XMLGUI bug #64754. My development branch is also hosted at https://github.com/jrmrjnck/okular-tabbed This addresses bug 155515. http://bugs.kde.org/show_bug.cgi?id=155515 Diffs (updated) - part.h 4b3aafdb637080ae81eb0e45742f53a34738984d part.cpp 71c3d0f5d908969ffcf18aba327297ccd2e1c8e1 shell/main.cpp e0ca587ba167c4020d5af5bd33a2dc1b4923cac4 shell/shell.h c065c560fb4ddfcf181601cf35e9ca14581731f6 shell/shell.cpp 1708501daaef817a1ce35fa5d96701a66ab66983 shell/shell.rc 93fbc417588312792bab39b693c65e5d414c87c6 Diff: http://git.reviewboard.kde.org/r/110914/diff/ Testing --- Thanks, Jonathan Doman ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
[Okular-devel] [okular] [Bug 322919] Crash on exit while PDF file is updated
https://bugs.kde.org/show_bug.cgi?id=322919 Albert Astals Cid aa...@kde.org changed: What|Removed |Added Status|NEEDSINFO |CONFIRMED Resolution|WAITINGFORINFO |--- Ever confirmed|0 |1 --- Comment #7 from Albert Astals Cid aa...@kde.org --- Valgrind says ==23131== Invalid read of size 8 ==23131==at 0x17D258BB: TOCModelPrivate::~TOCModelPrivate() (tocmodel.cpp:108) ==23131==by 0x17D2591B: TOCModel::~TOCModel() (tocmodel.cpp:169) ==23131==by 0x17D25938: TOCModel::~TOCModel() (tocmodel.cpp:170) ==23131==by 0x7095307: QObjectPrivate::deleteChildren() (in /usr/lib/x86_64-linux-gnu/libQtCore.so.4.8.4) ==23131==by 0x5FA6B36: QWidget::~QWidget() (in /usr/lib/x86_64-linux-gnu/libQtGui.so.4.8.4) ==23131==by 0x648B008: QTreeView::~QTreeView() (in /usr/lib/x86_64-linux-gnu/libQtGui.so.4.8.4) ==23131==by 0x7095307: QObjectPrivate::deleteChildren() (in /usr/lib/x86_64-linux-gnu/libQtCore.so.4.8.4) ==23131==by 0x5FA6B36: QWidget::~QWidget() (in /usr/lib/x86_64-linux-gnu/libQtGui.so.4.8.4) ==23131==by 0x17D23838: TOC::~TOC() (toc.cpp:58) ==23131==by 0x17CA09E1: Okular::Part::~Part() (part.cpp:838) ==23131==by 0x17CA0CB8: Okular::Part::~Part() (part.cpp:863) ==23131==by 0x7095307: QObjectPrivate::deleteChildren() (in /usr/lib/x86_64-linux-gnu/libQtCore.so.4.8.4) ==23131== Address 0xedc2830 is 0 bytes inside a block of size 24 free'd ==23131==at 0x4C2BADC: operator delete(void*) (vg_replace_malloc.c:480) ==23131==by 0x7095307: QObjectPrivate::deleteChildren() (in /usr/lib/x86_64-linux-gnu/libQtCore.so.4.8.4) ==23131==by 0x5FA6B36: QWidget::~QWidget() (in /usr/lib/x86_64-linux-gnu/libQtGui.so.4.8.4) ==23131==by 0x648B008: QTreeView::~QTreeView() (in /usr/lib/x86_64-linux-gnu/libQtGui.so.4.8.4) ==23131==by 0x7095307: QObjectPrivate::deleteChildren() (in /usr/lib/x86_64-linux-gnu/libQtCore.so.4.8.4) ==23131==by 0x5FA6B36: QWidget::~QWidget() (in /usr/lib/x86_64-linux-gnu/libQtGui.so.4.8.4) ==23131==by 0x17D23838: TOC::~TOC() (toc.cpp:58) ==23131==by 0x17CA09E1: Okular::Part::~Part() (part.cpp:838) ==23131==by 0x17CA0CB8: Okular::Part::~Part() (part.cpp:863) ==23131==by 0x7095307: QObjectPrivate::deleteChildren() (in /usr/lib/x86_64-linux-gnu/libQtCore.so.4.8.4) ==23131==by 0x5FA6B36: QWidget::~QWidget() (in /usr/lib/x86_64-linux-gnu/libQtGui.so.4.8.4) ==23131==by 0x5A1FEE4: KMainWindow::~KMainWindow() (kmainwindow.cpp:467) -- You are receiving this mail because: You are the assignee for the bug. ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
[Okular-devel] [okular] [Bug 322919] Crash on exit while PDF file is updated
https://bugs.kde.org/show_bug.cgi?id=322919 --- Comment #8 from Albert Astals Cid aa...@kde.org --- Possible solutions, make TOCModel *m_oldModel; be a QPointer so it gets automatically nulled when it's deleted or change parentship of m_model in toc.cpp when calling setOldModelData/clearOldModelData Both look not nice. Comments on what you guys think its less worse? -- You are receiving this mail because: You are the assignee for the bug. ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
Re: [Okular-devel] Review Request 111829: Use DPI of current screen for PDF rendering
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/111829/ --- (Updated Aug. 17, 2013, 8:29 p.m.) Review request for Okular and Albert Astals Cid. Changes --- Drop multiplication operators for Resolution class Description --- This patch relies on master branch of LibKScreen. This patch does not solve the problem in bug completely, but makes Okular behaviour more correct (see below). The problem (in the bug) is that Okular uses fixed DPI for PDF rendering (72.0 dots per inch) and therefore scale of rendered document becomes incorrect. With current mainline laptop screens having DPI easily twice larger than this constant (72), the problem shows itself quiet strongly. Additional problems araise with multiscreen configuration, when 1) DPI of each individual screen may be different, and 2) there is no tools in Qt to detect DPI of individual screens in virtual desktop mode. Therefore XRandr has to be used for DPI detection. Raw XRandr is bad dependency for Okular and libkscreen is proposed instead. This patch approach to the solution in the following way: 1. libkscreen detection staff is added to CMakeLists.txt and config.h 2. It changes Utils::realDpi() function to use libkscreen if present. With libkscreen the function looks for output that contains maximal part of given widget (suppose to be used for document rendering) and returns DPI of that screen. 3. Genenerator interface is extended by adding UtilizeDPI feature and setDPI() method, that is called by Document class right before calling loadXXX() if the generator supports this feature 4. Poppler generator is changed to support UtilizeDPI feature. 5. PageSizeMetric enum is extended with Pixels value to explicitly say that page size is defined in screen pixels (see my posts in the bug); Poppler generator uses this case. To completetly fix the bug, Document must invalidate generated pixmaps after the widget movements into another screen. I do not know how to track this without subclassing the main window class. Therefore I decided to publish this part of work to get your responce, especially regarding item 3 (Generator class changes). In the current state, manual reloading of a document after moving Okular to another screen fixes the scale, that is, in my eyes, is quiet helpful already. Even if we subclass the Okular main window, I do not know what to do when Okular is used as KPart. This addresses bug 268757. http://bugs.kde.org/show_bug.cgi?id=268757 Diffs (updated) - generators/poppler/generator_pdf.h 5d5853a generators/poppler/generator_pdf.cpp 1a44523 core/utils.h 8d5d5fc core/utils.cpp 5dd8448 core/generator.cpp 41beb92 core/generator_p.h b59293a core/generator.h a5a971b core/document.cpp 3af92c8 core/document_p.h 3a257de CMakeLists.txt 217337f config-okular.h.cmake 7217f8d Diff: http://git.reviewboard.kde.org/r/111829/diff/ Testing --- Manual. In all screens, that report correct physical size to XRandr, size of documents is correct Thanks, Eugene Shalygin ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
[Okular-devel] Review Request 112135: Fix for Bug 323262 and Bug 323263
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112135/ --- Review request for Okular. Description --- This patch solves Bug 323262 and Bug 323263. I also refactored and simplified the code a little. By removing unnecessary calls to toLower in TextPagePrivate::findTextInternalForward and TextPagePrivate::findTextInternalBackward I also fixed a small bug: the letter capital I with dot above (U+0130) did not match itself in case-insensitive mode on Qt 4.8.4 (U+0130 still does not match lowercase i (U+0069), which can be considered another bug, that I didn't fix (although this behavior conforms to the Unicode case folding rules)). (I did not implement the Knuth-Morris-Pratt algorithm that I promised in a comment of Bug 323263 because on second thought I find that the win, if any, would probably be negligible except for some very special documents and special query strings.) This addresses bugs 323262 and 323263. http://bugs.kde.org/show_bug.cgi?id=323262 http://bugs.kde.org/show_bug.cgi?id=323263 Diffs - core/textpage.cpp 855942d core/textpage_p.h 8ecf0c9 Diff: http://git.reviewboard.kde.org/r/112135/diff/ Testing --- Thanks, Jaan Vajakas ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
Re: [Okular-devel] Review Request 110003: Best-fit zoom
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110003/ --- (Updated Aug. 17, 2013, 8:24 p.m.) Review request for Okular. Changes --- Addressing Albert Astals Cid's recent comments on constness, version numbers, and documentation (handbook). Description --- Attached patch implements best-fit zoom for Okular. It is a refined version of the patch submitted in bug report 249364, attachment 51069. The refinement addresses the scrollbar issues as observed in continuous view mode. This addresses bug 249364. http://bugs.kde.org/show_bug.cgi?id=249364 Diffs (updated) - conf/dlggeneralbase.ui f2c9efd conf/okular.kcfg 1e23d61 doc/index.docbook 432aee2 part.rc 64aeffb ui/pageview.h 5484cc5 ui/pageview.cpp 5944072 Diff: http://git.reviewboard.kde.org/r/110003/diff/ Testing --- Thanks, Thomas Fischer ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
Re: [Okular-devel] Dt. 9th August - status
On Sat, Aug 17, 2013 at 2:06 AM, Albert Astals Cid aa...@kde.org wrote: El Divendres, 16 d'agost de 2013, a les 22:29:38, Jaydeep Solanki va escriure: How it occurs : 1) Open an ePub 2) As soon as it opens, hold Right arrow key, till it reaches the end of document or crashes another way would be 2) Hold PgUp/ PgDn key, till it reaches the end of document or crashes. *Note:* it doesn't always crash. In fact the probability of it crashing is very low, thus it gets difficult to debug. Have you tried with the helgrind tool of valgrind? yes The only information related to us, that I got is http://paste.kde.org/p14eda246/ It says nothing about QTextDocument::drawContents(..). Cheers, Albert On Tue, Aug 13, 2013 at 3:17 AM, Albert Astals Cid aa...@kde.org wrote: El Dissabte, 10 d'agost de 2013, a les 01:19:14, Jaydeep Solanki va escriure: Hi, I have tested yesterday's patch (without lock) with about 15 different epubs, and it crashed only once. I tried the same epub that made it crash once a few more times, but it didn't crash. Thus, crash doesn't depend on the epub. I observed a few things, it crashes only when I'm scrolling the pages at super fast speed (like pressing PgDn or right arrow key), this tells that generating pixmaps fast makes it break, so I set the performance to Greedy, which makes Okular generate all the pixmaps as soon as possible, but it never crashes there! The error that it gave me when crash occurred was memory corrupted, so I tried valgrind, but it didn't detect any invalid memory. May be whenever you get time give it a try. Let's see if it crashes with you. Can you please exactly tell me what you are doing and describe the crash you are getting? Cheers, Albert Cheers, Jaydeep ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
Re: [Okular-devel] Review Request 111410: Selection tool: copy/extract as vector graphic by calling pdftocairo
On Aug. 16, 2013, 10:18 p.m., Albert Astals Cid wrote: core/generator.h, line 395 http://git.reviewboard.kde.org/r/111410/diff/6/?file=181596#file181596line395 Why are these not const anymore? Constructing an instance of QProcess to run pdftocairo requires passing a non-const QObject pointer and this is used. I changed it to creating the instance without a parent. - Thomas --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/111410/#review38002 --- On Aug. 16, 2013, 9:17 p.m., Thomas Fischer wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/111410/ --- (Updated Aug. 16, 2013, 9:17 p.m.) Review request for Okular. Description --- This patch implements the feature request of bug 321350: if a PDF file is viewed, the selection tool offers the new extraction method vector which allows to save to a file (PDF, SVG, EPS, PostScript). The crop operation is performed by calling pdftocairo with matching arguments. The resulting file contains the original PDF file's content without rendering it to a pixmap. I am not sure if calling an external program is an acceptable solution for this problem. However, it is tested if the program is available before showing the new option. Alternatively, the code of pdftocairo (as part of poppler) would had to be copied and integrated into Okular increasing the solution's complexity. I am not aware of a similar solution available using poppler-qt4 only. Maybe using a QPrinter printing to PDF would have been an alternative, but again this seemed to be too complex. Diffs - core/document.h fe296e0 core/document.cpp 3af92c8 core/generator.h a5a971b core/generator.cpp 41beb92 generators/poppler/generator_pdf.h 5d5853a generators/poppler/generator_pdf.cpp 1a44523 ui/pageview.cpp 5944072 Diff: http://git.reviewboard.kde.org/r/111410/diff/ Testing --- Thanks, Thomas Fischer ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
Re: [Okular-devel] Review Request 111410: Selection tool: copy/extract as vector graphic by calling pdftocairo
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/111410/ --- (Updated Aug. 17, 2013, 9:11 p.m.) Review request for Okular. Changes --- Addressing recent comments: re-introducing const-ness, making search for pdftocairo binary lazy, ... Description --- This patch implements the feature request of bug 321350: if a PDF file is viewed, the selection tool offers the new extraction method vector which allows to save to a file (PDF, SVG, EPS, PostScript). The crop operation is performed by calling pdftocairo with matching arguments. The resulting file contains the original PDF file's content without rendering it to a pixmap. I am not sure if calling an external program is an acceptable solution for this problem. However, it is tested if the program is available before showing the new option. Alternatively, the code of pdftocairo (as part of poppler) would had to be copied and integrated into Okular increasing the solution's complexity. I am not aware of a similar solution available using poppler-qt4 only. Maybe using a QPrinter printing to PDF would have been an alternative, but again this seemed to be too complex. Diffs (updated) - core/document.h fe296e0 core/document.cpp 3af92c8 core/generator.h a5a971b core/generator.cpp 41beb92 generators/poppler/generator_pdf.h 5d5853a generators/poppler/generator_pdf.cpp 1a44523 ui/pageview.cpp 5944072 Diff: http://git.reviewboard.kde.org/r/111410/diff/ Testing --- Thanks, Thomas Fischer ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
Re: [Okular-devel] Review Request 111681: TextDocumentGenerator: Use black as default text color
On Aug. 17, 2013, 2:28 a.m., Albert Astals Cid wrote: To be honest i'm a bit confused by all the different patches trying to fix the same thing, there's this one, the other one that tries to use kcolorscheme, the other one that tries to let the user choose. And what I don't understand is why we need these patches. I would understand that if an epub sets the background color it will also set the text color (so all is fine, none of our colors is used) and if no color is set, i would undertand that the color scheme by the user provides acceptable colors. So where is the problem? Christoph Feck wrote: The problem is not documents that set colors, but those that set no colors (for example, plain text files). In the rendering code, the pixmap is pre-filled with hard-coded white color, but the text is actually rendered using the system's foreground color. This leads to white on white with dark color schemes (which usually use white text). There are, as you see, several ways to fix it. Why I believe this one makes more sense that using the system's background color: See my rationale at Description above. Albert Astals Cid wrote: Ok, can you try Jaydeep's branch epub-qtextdoc according to https://www.dropbox.com/s/mgf778ug6yjie2l/GSoC%20patches.odt the color issues are fixed there. Can you confirm? Is the solution acceptable for you? Christoph Feck wrote: From quickly checking commit 1823cf9998555e22c6f3d8701728882dc34b244b (which is documented to fix the color issue), I see that Jaydeep injects CSS code to change QTextDocument's default color to Qt::black. While this might have the same result, my approach is simpler and uses less resources. Additionally, it looks like his patch is limited to epub, while my patch works for all generators that use textdocumentgenerator.cpp? Jaydeep might clarify, if I am wrong. Christoph Feck wrote: (Let me add that I wasn't aware about his work while I wrote the patch. I have no intention to disturb his work.) For sure, you patch addresses a bigger issue by fixing this for all generators that use TextDocumentGenerator. I'd be happy to remove my patch for this patch, but I'm still not finding the links in blue color. Albert can you please confirm, if the links show up in blue. - Jaydeep --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/111681/#review37994 --- On Aug. 17, 2013, 2:28 a.m., Christoph Feck wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/111681/ --- (Updated Aug. 17, 2013, 2:28 a.m.) Review request for Okular. Description --- As indicated in bug 322547, some documents do not specify a text color, and probably assume the default text color to be black. QTextDocument, however, defaults to using the system text color. This patch changes the default text color to Qt::black. It should affect epub, fb2, odt, and plain text generators. I think it is better to use this approach instead of changing the paper color to use the system background color (see bug 253583), because 1) the document might specify a text color in some places, 2) the user is able to change the fg/bg colors anyway using Okular's Accessibility options, and those probably expect black on white. This addresses bugs 253583 and 322547. http://bugs.kde.org/show_bug.cgi?id=253583 http://bugs.kde.org/show_bug.cgi?id=322547 Diffs - core/textdocumentgenerator.cpp b260b3f Diff: http://git.reviewboard.kde.org/r/111681/diff/ Testing --- I tested the document from bug 322547 comment #3. Thanks, Christoph Feck ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
Re: [Okular-devel] Review Request 111681: TextDocumentGenerator: Use black as default text color
On Aug. 17, 2013, 2:28 a.m., Albert Astals Cid wrote: To be honest i'm a bit confused by all the different patches trying to fix the same thing, there's this one, the other one that tries to use kcolorscheme, the other one that tries to let the user choose. And what I don't understand is why we need these patches. I would understand that if an epub sets the background color it will also set the text color (so all is fine, none of our colors is used) and if no color is set, i would undertand that the color scheme by the user provides acceptable colors. So where is the problem? Christoph Feck wrote: The problem is not documents that set colors, but those that set no colors (for example, plain text files). In the rendering code, the pixmap is pre-filled with hard-coded white color, but the text is actually rendered using the system's foreground color. This leads to white on white with dark color schemes (which usually use white text). There are, as you see, several ways to fix it. Why I believe this one makes more sense that using the system's background color: See my rationale at Description above. Albert Astals Cid wrote: Ok, can you try Jaydeep's branch epub-qtextdoc according to https://www.dropbox.com/s/mgf778ug6yjie2l/GSoC%20patches.odt the color issues are fixed there. Can you confirm? Is the solution acceptable for you? Christoph Feck wrote: From quickly checking commit 1823cf9998555e22c6f3d8701728882dc34b244b (which is documented to fix the color issue), I see that Jaydeep injects CSS code to change QTextDocument's default color to Qt::black. While this might have the same result, my approach is simpler and uses less resources. Additionally, it looks like his patch is limited to epub, while my patch works for all generators that use textdocumentgenerator.cpp? Jaydeep might clarify, if I am wrong. Christoph Feck wrote: (Let me add that I wasn't aware about his work while I wrote the patch. I have no intention to disturb his work.) Jaydeep Solanki wrote: For sure, you patch addresses a bigger issue by fixing this for all generators that use TextDocumentGenerator. I'd be happy to remove my patch for this patch, but I'm still not finding the links in blue color. Albert can you please confirm, if the links show up in blue. Okay, it seems that my color scheme was the culprit for not showing links in blue. I changed color scheme and now it works. Albert, IMHO this patch seems more appropriate. The other one with KColorScheme doesn't seem so good, as it changes the background color of the document which is not necessary. - Jaydeep --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/111681/#review37994 --- On Aug. 17, 2013, 2:28 a.m., Christoph Feck wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/111681/ --- (Updated Aug. 17, 2013, 2:28 a.m.) Review request for Okular. Description --- As indicated in bug 322547, some documents do not specify a text color, and probably assume the default text color to be black. QTextDocument, however, defaults to using the system text color. This patch changes the default text color to Qt::black. It should affect epub, fb2, odt, and plain text generators. I think it is better to use this approach instead of changing the paper color to use the system background color (see bug 253583), because 1) the document might specify a text color in some places, 2) the user is able to change the fg/bg colors anyway using Okular's Accessibility options, and those probably expect black on white. This addresses bugs 253583 and 322547. http://bugs.kde.org/show_bug.cgi?id=253583 http://bugs.kde.org/show_bug.cgi?id=322547 Diffs - core/textdocumentgenerator.cpp b260b3f Diff: http://git.reviewboard.kde.org/r/111681/diff/ Testing --- I tested the document from bug 322547 comment #3. Thanks, Christoph Feck ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
Re: [Okular-devel] Review Request 111681: TextDocumentGenerator: Use black as default text color
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/111681/#review38048 --- core/textdocumentgenerator.cpp http://git.reviewboard.kde.org/r/111681/#comment28138 This isn't necessary, after clicking a link, the images aren't going to update. So, if it is never going to be displayed, it's better not to write it. - Jaydeep Solanki On Aug. 17, 2013, 2:28 a.m., Christoph Feck wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/111681/ --- (Updated Aug. 17, 2013, 2:28 a.m.) Review request for Okular. Description --- As indicated in bug 322547, some documents do not specify a text color, and probably assume the default text color to be black. QTextDocument, however, defaults to using the system text color. This patch changes the default text color to Qt::black. It should affect epub, fb2, odt, and plain text generators. I think it is better to use this approach instead of changing the paper color to use the system background color (see bug 253583), because 1) the document might specify a text color in some places, 2) the user is able to change the fg/bg colors anyway using Okular's Accessibility options, and those probably expect black on white. This addresses bugs 253583 and 322547. http://bugs.kde.org/show_bug.cgi?id=253583 http://bugs.kde.org/show_bug.cgi?id=322547 Diffs - core/textdocumentgenerator.cpp b260b3f Diff: http://git.reviewboard.kde.org/r/111681/diff/ Testing --- I tested the document from bug 322547 comment #3. Thanks, Christoph Feck ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
