> On Aug. 15, 2013, 9:03 p.m., Albert Astals Cid wrote: > > > > Eugene Shalygin wrote: > Thanks for the review! I've tried to fix the issues. The only one left is > question with Generator::DPI. I've replaced it with QSizeF (not the best > choice, I know). Somehow keeping both X and Y dpis together makes me happy :) > Is it good to keep it as is, or replace with a pair of qreals?
Seems our comments crossed themselves in the internet, I don't actually like QSize since it's not a size, I am not sure i like DPI because it's a new class just two store two numbers, but i don't have a strong opinion. If you really want to keep them together, bring the DPI class, but please make the implementation private (i.e. not all over the header) - Albert ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/111829/#review37877 ----------------------------------------------------------- On Aug. 15, 2013, 10:46 p.m., Eugene Shalygin wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/111829/ > ----------------------------------------------------------- > > (Updated Aug. 15, 2013, 10:46 p.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 > ----- > > core/utils.cpp 5dd8448 > core/generator_p.h b59293a > core/utils.h 8d5d5fc > core/generator.cpp 41beb92 > core/document_p.h 3a257de > core/generator.h a5a971b > core/document.cpp 3af92c8 > CMakeLists.txt 217337f > config-okular.h.cmake 7217f8d > 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