> On Aug. 25, 2013, 6:13 p.m., Albert Astals Cid wrote: > > Ok, the code looks "sane" but there are two things that still make me > > unsure about this whole thing: > > > > * Why you need Pixels? > > In the bug you say "Points, but actually it is pixels, DIVIDED by 72." > > That is not true, the page size of a PDF is defined in points (well it's > > actually defined in UserUnits but that defaults to the Point value) > > Why do you say it's pixels DIVIDED by 72? > > I'd be happier if we could still have the PDF backend return stuff in > > points and have all the dpi conversion magic in the layers above it > > > > * I'm still undecided as of why we need the widget realDpi. Reasons: > > As far as I know (though my knowledge may not be very good :D) all the > > systems force you to have the same DPI in all the monitors, so having to > > pass the widget seems "nice it's going to do magic" when I move it to > > another monitor, but that's really not happening no? Or at least if there > > was a system in which you could have different screens with different > > monitors, we miss something so that when you move it from one monitor to > > another the dpi gets recalculated no? > > > > I know this may sound like I'm stalling the patch, but it is really not, I > > just want to make sure we end up with a patch that we're all happy and > > convinced with.
> * Why you need Pixels? Regarding the "pixels divided..." I'm not sure now :). The story had been started with aim to get correct scale on screen, as you certainly remember. To my understanding, Page::width() and Page::height() are in screen pixels (at least it seems to me like this from reading of e.g. PageView::updateItemSize()). If this is correct, PageView class should handle screen DPI, and all generators should set page size in points (or in something, that has a dimension). Will we go this way? > * I'm still undecided as of why we need the widget realDpi. I'm afraid I do not understand your comment completetly. I'll answwer on question that I understood. 1. The system can not "force" us to have the same DPI in all monitors, since the system can not change physical DPI of the screen (at least in the Universe #3 :D). 2. We use widget object to get the monitor in which the display the pages (and its current DPI). - Eugene ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/111829/#review38556 ----------------------------------------------------------- On Aug. 21, 2013, 2:56 a.m., Eugene Shalygin wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/111829/ > ----------------------------------------------------------- > > (Updated Aug. 21, 2013, 2:56 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/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
