Re: [Okular-devel] Review Request 111829: Use DPI of current screen for PDF rendering

[Christoph Feck]

> On Jan. 13, 2014, 12:40 a.m., Albert Astals Cid wrote:
> > I did some small changes on my own so we did not have to go back and forth 
> > more which would have just made it slow for all.
> > Thanks for the patch :-)
> 
> Eugene Shalygin wrote:
> Thanks! Very happy that Okular now shows PDF in correct scale!

Very nice, thanks for the work! I love it when software adapts :)


- Christoph


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/111829/#review47283
---


On Jan. 13, 2014, 12:39 a.m., Eugene Shalygin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/111829/
> ---
> 
> (Updated Jan. 13, 2014, 12:39 a.m.)
> 
> 
> Review request for Okular and Albert Astals Cid.
> 
> 
> Bugs: 268757
> http://bugs.kde.org/show_bug.cgi?id=268757
> 
> 
> Repository: okular
> 
> 
> 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. 
> 
> 
> 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: https://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

Re: [Okular-devel] Review Request 111829: Use DPI of current screen for PDF rendering

[Eugene Shalygin]

> On Jan. 13, 2014, 1:40 a.m., Albert Astals Cid wrote:
> > I did some small changes on my own so we did not have to go back and forth 
> > more which would have just made it slow for all.
> > Thanks for the patch :-)

Thanks! Very happy that Okular now shows PDF in correct scale!


- Eugene


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/111829/#review47283
---


On Jan. 13, 2014, 1:39 a.m., Eugene Shalygin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/111829/
> ---
> 
> (Updated Jan. 13, 2014, 1:39 a.m.)
> 
> 
> Review request for Okular and Albert Astals Cid.
> 
> 
> Bugs: 268757
> http://bugs.kde.org/show_bug.cgi?id=268757
> 
> 
> Repository: okular
> 
> 
> 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. 
> 
> 
> 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: https://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

Re: [Okular-devel] Review Request 111829: Use DPI of current screen for PDF rendering

[Albert Astals Cid]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/111829/#review47283
---


I did some small changes on my own so we did not have to go back and forth more 
which would have just made it slow for all.
Thanks for the patch :-)

- Albert Astals Cid


On Jan. 13, 2014, 12:39 a.m., Eugene Shalygin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/111829/
> ---
> 
> (Updated Jan. 13, 2014, 12:39 a.m.)
> 
> 
> Review request for Okular and Albert Astals Cid.
> 
> 
> Bugs: 268757
> http://bugs.kde.org/show_bug.cgi?id=268757
> 
> 
> Repository: okular
> 
> 
> 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. 
> 
> 
> 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: https://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

Re: [Okular-devel] Review Request 111829: Use DPI of current screen for PDF rendering

[Commit Hook]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/111829/#review47282
---


This review has been submitted with commit 
ed3559462759bf9b94d7b3e8f1ee289423fa9d17 by Albert Astals Cid on behalf of 
Eugene Shalygin to branch master.

- Commit Hook


On Aug. 21, 2013, 12:56 a.m., Eugene Shalygin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/111829/
> ---
> 
> (Updated Aug. 21, 2013, 12:56 a.m.)
> 
> 
> Review request for Okular and Albert Astals Cid.
> 
> 
> Bugs: 268757
> http://bugs.kde.org/show_bug.cgi?id=268757
> 
> 
> Repository: okular
> 
> 
> 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. 
> 
> 
> 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: https://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

Re: [Okular-devel] Review Request 111829: Use DPI of current screen for PDF rendering

[Eugene Shalygin]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/111829/
---

(Updated Jan. 13, 2014, 12:39 a.m.)


Status
--

This change has been marked as submitted.


Review request for Okular and Albert Astals Cid.


Bugs: 268757
http://bugs.kde.org/show_bug.cgi?id=268757


Repository: okular


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. 


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: https://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

Re: [Okular-devel] Review Request 111829: Use DPI of current screen for PDF rendering

[Albert Astals Cid]

> On Oct. 1, 2013, 9:37 p.m., Albert Astals Cid wrote:
> > core/utils.cpp, line 116
> > 
> >
> > Hmmm, my libkscreen does not have sizeMm, what libkscreen version are 
> > you using?
> 
> Eugene Shalygin wrote:
> The changes were accepted into master branch of libkscreen some time ago, 
> and I do not know when they will be released
> 
> Albert Astals Cid wrote:
> Ok, we'll have to wait until it gets released since otherwise the code is 
> not compiling at the moment for people running released versions of stuff. 
> It'd be cool if you can warn us once there's a released version of libkscreen 
> we can use.
> 
> Eugene Shalygin wrote:
> The change was commited on the next day after 1.0.1 tag. So, no problem, 
> I'll ping here. Does it mean that you do not have any other objections? ;)
> 
> Albert Astals Cid wrote:
> Can't tell if i have more objections until i can compile and run the 
> stuff, i defenitely want to try how hard is to get away from using pixels and 
> back to points, but i'll try that myself once i get this thing compiling 
> properly.
> 
> Eugene Shalygin wrote:
> Version 1.0.2 of libkscreen was released --- 
> http://download.kde.org/stable/libkscreen/1.0.2/
> 
> Eugene Shalygin wrote:
> ping?
> 
> Albert Astals Cid wrote:
> on it sorry, been busy
> 
> Eugene Shalygin wrote:
> OK. Do we have a chance to make it done for 4.13?

Yes, see http://techbase.kde.org/Schedules/KDE4/4.13_Release_Schedule


- Albert


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/111829/#review41080
---


On Aug. 21, 2013, 12:56 a.m., Eugene Shalygin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/111829/
> ---
> 
> (Updated Aug. 21, 2013, 12:56 a.m.)
> 
> 
> Review request for Okular and Albert Astals Cid.
> 
> 
> Bugs: 268757
> http://bugs.kde.org/show_bug.cgi?id=268757
> 
> 
> Repository: okular
> 
> 
> 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. 
> 
> 
> 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: https://git.reviewboard.kde.org/r/111829/diff/
>

Re: [Okular-devel] Review Request 111829: Use DPI of current screen for PDF rendering

[Eugene Shalygin]

> On Oct. 1, 2013, 11:37 p.m., Albert Astals Cid wrote:
> > core/utils.cpp, line 116
> > 
> >
> > Hmmm, my libkscreen does not have sizeMm, what libkscreen version are 
> > you using?
> 
> Eugene Shalygin wrote:
> The changes were accepted into master branch of libkscreen some time ago, 
> and I do not know when they will be released
> 
> Albert Astals Cid wrote:
> Ok, we'll have to wait until it gets released since otherwise the code is 
> not compiling at the moment for people running released versions of stuff. 
> It'd be cool if you can warn us once there's a released version of libkscreen 
> we can use.
> 
> Eugene Shalygin wrote:
> The change was commited on the next day after 1.0.1 tag. So, no problem, 
> I'll ping here. Does it mean that you do not have any other objections? ;)
> 
> Albert Astals Cid wrote:
> Can't tell if i have more objections until i can compile and run the 
> stuff, i defenitely want to try how hard is to get away from using pixels and 
> back to points, but i'll try that myself once i get this thing compiling 
> properly.
> 
> Eugene Shalygin wrote:
> Version 1.0.2 of libkscreen was released --- 
> http://download.kde.org/stable/libkscreen/1.0.2/
> 
> Eugene Shalygin wrote:
> ping?
> 
> Albert Astals Cid wrote:
> on it sorry, been busy

OK. Do we have a chance to make it done for 4.13?


- Eugene


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/111829/#review41080
---


On Aug. 21, 2013, 2:56 a.m., Eugene Shalygin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/111829/
> ---
> 
> (Updated Aug. 21, 2013, 2:56 a.m.)
> 
> 
> Review request for Okular and Albert Astals Cid.
> 
> 
> Bugs: 268757
> http://bugs.kde.org/show_bug.cgi?id=268757
> 
> 
> Repository: okular
> 
> 
> 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. 
> 
> 
> 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: https://git.reviewboard.kde.org/r/111829/diff/
> 
> 
> Testing
> ---
> 
> Manual. In all screens, that report correct physical size to XRandr, size o

Re: [Okular-devel] Review Request 111829: Use DPI of current screen for PDF rendering

[Albert Astals Cid]

> On Oct. 1, 2013, 9:37 p.m., Albert Astals Cid wrote:
> > core/utils.cpp, line 116
> > 
> >
> > Hmmm, my libkscreen does not have sizeMm, what libkscreen version are 
> > you using?
> 
> Eugene Shalygin wrote:
> The changes were accepted into master branch of libkscreen some time ago, 
> and I do not know when they will be released
> 
> Albert Astals Cid wrote:
> Ok, we'll have to wait until it gets released since otherwise the code is 
> not compiling at the moment for people running released versions of stuff. 
> It'd be cool if you can warn us once there's a released version of libkscreen 
> we can use.
> 
> Eugene Shalygin wrote:
> The change was commited on the next day after 1.0.1 tag. So, no problem, 
> I'll ping here. Does it mean that you do not have any other objections? ;)
> 
> Albert Astals Cid wrote:
> Can't tell if i have more objections until i can compile and run the 
> stuff, i defenitely want to try how hard is to get away from using pixels and 
> back to points, but i'll try that myself once i get this thing compiling 
> properly.
> 
> Eugene Shalygin wrote:
> Version 1.0.2 of libkscreen was released --- 
> http://download.kde.org/stable/libkscreen/1.0.2/
> 
> Eugene Shalygin wrote:
> ping?

on it sorry, been busy


- Albert


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/111829/#review41080
---


On Aug. 21, 2013, 12:56 a.m., Eugene Shalygin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/111829/
> ---
> 
> (Updated Aug. 21, 2013, 12:56 a.m.)
> 
> 
> Review request for Okular and Albert Astals Cid.
> 
> 
> Bugs: 268757
> http://bugs.kde.org/show_bug.cgi?id=268757
> 
> 
> Repository: okular
> 
> 
> 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. 
> 
> 
> 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: https://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
> 
>

Re: [Okular-devel] Review Request 111829: Use DPI of current screen for PDF rendering

[Eugene Shalygin]

> On Oct. 1, 2013, 11:37 p.m., Albert Astals Cid wrote:
> > core/utils.cpp, line 116
> > 
> >
> > Hmmm, my libkscreen does not have sizeMm, what libkscreen version are 
> > you using?
> 
> Eugene Shalygin wrote:
> The changes were accepted into master branch of libkscreen some time ago, 
> and I do not know when they will be released
> 
> Albert Astals Cid wrote:
> Ok, we'll have to wait until it gets released since otherwise the code is 
> not compiling at the moment for people running released versions of stuff. 
> It'd be cool if you can warn us once there's a released version of libkscreen 
> we can use.
> 
> Eugene Shalygin wrote:
> The change was commited on the next day after 1.0.1 tag. So, no problem, 
> I'll ping here. Does it mean that you do not have any other objections? ;)
> 
> Albert Astals Cid wrote:
> Can't tell if i have more objections until i can compile and run the 
> stuff, i defenitely want to try how hard is to get away from using pixels and 
> back to points, but i'll try that myself once i get this thing compiling 
> properly.
> 
> Eugene Shalygin wrote:
> Version 1.0.2 of libkscreen was released --- 
> http://download.kde.org/stable/libkscreen/1.0.2/

ping?


- Eugene


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/111829/#review41080
---


On Aug. 21, 2013, 2:56 a.m., Eugene Shalygin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/111829/
> ---
> 
> (Updated Aug. 21, 2013, 2:56 a.m.)
> 
> 
> Review request for Okular and Albert Astals Cid.
> 
> 
> Bugs: 268757
> http://bugs.kde.org/show_bug.cgi?id=268757
> 
> 
> Repository: okular
> 
> 
> 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. 
> 
> 
> 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: https://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

Re: [Okular-devel] Review Request 111829: Use DPI of current screen for PDF rendering

[Eugene Shalygin]

> On Oct. 1, 2013, 11:37 p.m., Albert Astals Cid wrote:
> > core/utils.cpp, line 116
> > 
> >
> > Hmmm, my libkscreen does not have sizeMm, what libkscreen version are 
> > you using?
> 
> Eugene Shalygin wrote:
> The changes were accepted into master branch of libkscreen some time ago, 
> and I do not know when they will be released
> 
> Albert Astals Cid wrote:
> Ok, we'll have to wait until it gets released since otherwise the code is 
> not compiling at the moment for people running released versions of stuff. 
> It'd be cool if you can warn us once there's a released version of libkscreen 
> we can use.
> 
> Eugene Shalygin wrote:
> The change was commited on the next day after 1.0.1 tag. So, no problem, 
> I'll ping here. Does it mean that you do not have any other objections? ;)
> 
> Albert Astals Cid wrote:
> Can't tell if i have more objections until i can compile and run the 
> stuff, i defenitely want to try how hard is to get away from using pixels and 
> back to points, but i'll try that myself once i get this thing compiling 
> properly.

Version 1.0.2 of libkscreen was released --- 
http://download.kde.org/stable/libkscreen/1.0.2/


- Eugene


---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/111829/#review41080
---


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.
> 
> 
> Bugs: 268757
> http://bugs.kde.org/show_bug.cgi?id=268757
> 
> 
> Repository: okular
> 
> 
> 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. 
> 
> 
> 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/mail

Re: [Okular-devel] Review Request 111829: Use DPI of current screen for PDF rendering

[Albert Astals Cid]

> On Oct. 1, 2013, 9:37 p.m., Albert Astals Cid wrote:
> > core/utils.cpp, line 116
> > 
> >
> > Hmmm, my libkscreen does not have sizeMm, what libkscreen version are 
> > you using?
> 
> Eugene Shalygin wrote:
> The changes were accepted into master branch of libkscreen some time ago, 
> and I do not know when they will be released
> 
> Albert Astals Cid wrote:
> Ok, we'll have to wait until it gets released since otherwise the code is 
> not compiling at the moment for people running released versions of stuff. 
> It'd be cool if you can warn us once there's a released version of libkscreen 
> we can use.
> 
> Eugene Shalygin wrote:
> The change was commited on the next day after 1.0.1 tag. So, no problem, 
> I'll ping here. Does it mean that you do not have any other objections? ;)

Can't tell if i have more objections until i can compile and run the stuff, i 
defenitely want to try how hard is to get away from using pixels and back to 
points, but i'll try that myself once i get this thing compiling properly.


- Albert


---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/111829/#review41080
---


On Aug. 21, 2013, 12: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, 12:56 a.m.)
> 
> 
> Review request for Okular and Albert Astals Cid.
> 
> 
> Bugs: 268757
> http://bugs.kde.org/show_bug.cgi?id=268757
> 
> 
> Repository: okular
> 
> 
> 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. 
> 
> 
> 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

Re: [Okular-devel] Review Request 111829: Use DPI of current screen for PDF rendering

[Eugene Shalygin]

> On Oct. 1, 2013, 11:37 p.m., Albert Astals Cid wrote:
> > core/utils.cpp, line 116
> > 
> >
> > Hmmm, my libkscreen does not have sizeMm, what libkscreen version are 
> > you using?
> 
> Eugene Shalygin wrote:
> The changes were accepted into master branch of libkscreen some time ago, 
> and I do not know when they will be released
> 
> Albert Astals Cid wrote:
> Ok, we'll have to wait until it gets released since otherwise the code is 
> not compiling at the moment for people running released versions of stuff. 
> It'd be cool if you can warn us once there's a released version of libkscreen 
> we can use.

The change was commited on the next day after 1.0.1 tag. So, no problem, I'll 
ping here. Does it mean that you do not have any other objections? ;)


- Eugene


---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/111829/#review41080
---


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.
> 
> 
> Bugs: 268757
> http://bugs.kde.org/show_bug.cgi?id=268757
> 
> 
> Repository: okular
> 
> 
> 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. 
> 
> 
> 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

Re: [Okular-devel] Review Request 111829: Use DPI of current screen for PDF rendering

[Albert Astals Cid]

> On Oct. 1, 2013, 9:37 p.m., Albert Astals Cid wrote:
> > core/utils.cpp, line 116
> > 
> >
> > Hmmm, my libkscreen does not have sizeMm, what libkscreen version are 
> > you using?
> 
> Eugene Shalygin wrote:
> The changes were accepted into master branch of libkscreen some time ago, 
> and I do not know when they will be released

Ok, we'll have to wait until it gets released since otherwise the code is not 
compiling at the moment for people running released versions of stuff. It'd be 
cool if you can warn us once there's a released version of libkscreen we can 
use.


- Albert


---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/111829/#review41080
---


On Aug. 21, 2013, 12: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, 12:56 a.m.)
> 
> 
> Review request for Okular and Albert Astals Cid.
> 
> 
> Bugs: 268757
> http://bugs.kde.org/show_bug.cgi?id=268757
> 
> 
> Repository: okular
> 
> 
> 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. 
> 
> 
> 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

Re: [Okular-devel] Review Request 111829: Use DPI of current screen for PDF rendering

[Eugene Shalygin]

> On Oct. 1, 2013, 11:37 p.m., Albert Astals Cid wrote:
> > core/utils.cpp, line 116
> > 
> >
> > Hmmm, my libkscreen does not have sizeMm, what libkscreen version are 
> > you using?

The changes were accepted into master branch of libkscreen some time ago, and I 
do not know when they will be released


- Eugene


---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/111829/#review41080
---


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.
> 
> 
> Bugs: 268757
> http://bugs.kde.org/show_bug.cgi?id=268757
> 
> 
> Repository: okular
> 
> 
> 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. 
> 
> 
> 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

Re: [Okular-devel] Review Request 111829: Use DPI of current screen for PDF rendering

[Albert Astals Cid]

---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/111829/#review41080
---



core/utils.cpp


Hmmm, my libkscreen does not have sizeMm, what libkscreen version are you 
using?


- Albert Astals Cid


On Aug. 21, 2013, 12: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, 12:56 a.m.)
> 
> 
> Review request for Okular and Albert Astals Cid.
> 
> 
> Bugs: 268757
> http://bugs.kde.org/show_bug.cgi?id=268757
> 
> 
> Repository: okular
> 
> 
> 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. 
> 
> 
> 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

Re: [Okular-devel] Review Request 111829: Use DPI of current screen for PDF rendering

[Eugene Shalygin]

> 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.
> 
> Eugene Shalygin wrote:
> > * 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).
> 
>
> 
> Albert Astals Cid wrote:
> I'm going to be on holiday most of september, so unless someone else 
> takes care of this review you'll have to wait until I come back.
> 
> Eugene Shalygin wrote:
> Well, then just commit it right now ;)
> Could you please then try to answer my question before going on holiday?
> 
> Albert Astals Cid wrote:
> Should be learning some Japanese, but oh well, i'm already here.
> 
> > > * 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? 
> Well, i'm undeccided on this one, i'd like to share as much dpi<->pixel 
> conversion code, and putting it outside the generator helps this, but of 
> course it means we have to change more *core* code that's also not so great, 
> so well think if you could make it so some of the code that does dpi<->pixel 
> was shared a bit more up the chain. About the other generators, they don't 
> return "Points" so we wouldn't need to change them back and forth
> 
> 
> > > * 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). 
> It is, but as far as I've been told X only has one DPI setting so...
> > 2. We use widget object to get the monitor in which the display the 
> pages (and its current DPI).
> ... are you sure about this? Everyone I asked as told me that if I have a 
> two screen desktop shared across two monitors with different DPI, I'll get 
> the same DPI (one of the two monitors) when querying on both, bsically 
> because if this did not happen how would you render something that is shown 
> on both screens because it's "in the middle"?

First of all, thanks for your time!

> Well, i'm undeccided on this one, i'd like to share as much dpi<->pixel 
> conversion code,... so well think if you could make it so some of the code 

Re: [Okular-devel] Review Request 111829: Use DPI of current screen for PDF rendering

[Albert Astals Cid]

> On Aug. 25, 2013, 4: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.
> 
> Eugene Shalygin wrote:
> > * 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).
> 
>
> 
> Albert Astals Cid wrote:
> I'm going to be on holiday most of september, so unless someone else 
> takes care of this review you'll have to wait until I come back.
> 
> Eugene Shalygin wrote:
> Well, then just commit it right now ;)
> Could you please then try to answer my question before going on holiday?

Should be learning some Japanese, but oh well, i'm already here.

> > * 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? 
Well, i'm undeccided on this one, i'd like to share as much dpi<->pixel 
conversion code, and putting it outside the generator helps this, but of course 
it means we have to change more *core* code that's also not so great, so well 
think if you could make it so some of the code that does dpi<->pixel was shared 
a bit more up the chain. About the other generators, they don't return "Points" 
so we wouldn't need to change them back and forth


> > * 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). 
It is, but as far as I've been told X only has one DPI setting so...
> 2. We use widget object to get the monitor in which the display the pages 
> (and its current DPI).
... are you sure about this? Everyone I asked as told me that if I have a two 
screen desktop shared across two monitors with different DPI, I'll get the same 
DPI (one of the two monitors) when querying on both, bsically because if this 
did not happen how would you render something that is shown on both screens 
because it's "in the middle"?


- Albert


---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/111829/#review38556
---


On Aug. 21, 2013, 12:56 a.m., Eugene Shalygin wrote:
> 
> 

Re: [Okular-devel] Review Request 111829: Use DPI of current screen for PDF rendering

[Eugene Shalygin]

> 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.
> 
> Eugene Shalygin wrote:
> > * 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).
> 
>
> 
> Albert Astals Cid wrote:
> I'm going to be on holiday most of september, so unless someone else 
> takes care of this review you'll have to wait until I come back.

Well, then just commit it right now ;)
Could you please then try to answer my question before going on holiday?


- 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 bu

Re: [Okular-devel] Review Request 111829: Use DPI of current screen for PDF rendering

[Albert Astals Cid]

> On Aug. 25, 2013, 4: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.
> 
> Eugene Shalygin wrote:
> > * 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).
> 
>

I'm going to be on holiday most of september, so unless someone else takes care 
of this review you'll have to wait until I come back.


- Albert


---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/111829/#review38556
---


On Aug. 21, 2013, 12: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, 12: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 movement

Re: [Okular-devel] Review Request 111829: Use DPI of current screen for PDF rendering

[Eugene Shalygin]

> 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 t

Re: [Okular-devel] Review Request 111829: Use DPI of current screen for PDF rendering

[Albert Astals Cid]

---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/111829/#review38556
---


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.

- Albert Astals Cid


On Aug. 21, 2013, 12: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, 12: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-de

Re: [Okular-devel] Review Request 111829: Use DPI of current screen for PDF rendering

[Eugene Shalygin]

> On Aug. 21, 2013, 12:57 a.m., Albert Astals Cid wrote:
> > core/document_p.h, line 278
> > 
> >
> > Do we need this one?

Thanks! This is a leftover from time when Generator was not having dpi() 
function


- Eugene


---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/111829/#review38246
---


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

Re: [Okular-devel] Review Request 111829: Use DPI of current screen for PDF rendering

[Eugene Shalygin]

---
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.


Changes
---

Remove DocumentPrivate::m_DPI


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)
-

  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

Re: [Okular-devel] Review Request 111829: Use DPI of current screen for PDF rendering

[Albert Astals Cid]

---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/111829/#review38246
---



core/document_p.h


Do we need this one?


- Albert Astals Cid


On Aug. 17, 2013, 6:29 p.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, 6:29 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
> -
> 
>   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

Re: [Okular-devel] Review Request 111829: Use DPI of current screen for PDF rendering

[Eugene Shalygin]

---
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

Re: [Okular-devel] Review Request 111829: Use DPI of current screen for PDF rendering

[Albert Astals Cid]

---
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

Re: [Okular-devel] Review Request 111829: Use DPI of current screen for PDF rendering

[Eugene Shalygin]

---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/111829/
---

(Updated Aug. 17, 2013, 2:02 a.m.)


Review request for Okular and Albert Astals Cid.


Changes
---

Let's try with dedicated DPI class (named Resolution). Perhaps, multiplication 
opeartors are not strictly required, since they simplifies life not that much 
ass one would want


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)
-

  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

Re: [Okular-devel] Review Request 111829: Use DPI of current screen for PDF rendering

[Albert Astals Cid]

> 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?
> 
> Albert Astals Cid wrote:
> 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)
> 
> Eugene Shalygin wrote:
> I would not use two qreals not only because of esthetical reason (keeping 
> two related values together is certainly good), but there is also at least 
> one practical at the moment: we can have only one function realDpi() instead 
> of two.
> Therefore, I would use a DPI class even in the Utils class. But if you 
> say that that would complicate support, we can live with qreals.
> 
> But, since DPI  class is just two numbers, without functions, and I can 
> not see how its structure can change in the future, I would think that inline 
> class is fine (comparing to private implementation, i.e. exported symbols).

Ok, bring back the DPI class back, don't put it in generator though, makes not 
much sense there, maybe utils.h or somewhere like that. Also don't really like 
the DPI name much, maybe DPIValues? DPISettings? (that one is probably bad too).


- 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.c

Re: [Okular-devel] Review Request 111829: Use DPI of current screen for PDF rendering

[Eugene Shalygin]

> On Aug. 15, 2013, 11: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?
> 
> Albert Astals Cid wrote:
> 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)

I would not use two qreals not only because of esthetical reason (keeping two 
related values together is certainly good), but there is also at least one 
practical at the moment: we can have only one function realDpi() instead of two.
Therefore, I would use a DPI class even in the Utils class. But if you say that 
that would complicate support, we can live with qreals.

But, since DPI  class is just two numbers, without functions, and I can not see 
how its structure can change in the future, I would think that inline class is 
fine (comparing to private implementation, i.e. exported symbols).


- Eugene


---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/111829/#review37877
---


On Aug. 16, 2013, 12:46 a.m., Eugene Shalygin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/111829/
> ---
> 
> (Updated Aug. 16, 2013, 12:46 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
> -
> 
>   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
> 
>

Re: [Okular-devel] Review Request 111829: Use DPI of current screen for PDF rendering

[Albert Astals Cid]

> 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

Re: [Okular-devel] Review Request 111829: Use DPI of current screen for PDF rendering

[Albert Astals Cid]

---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/111829/#review37905
---


Agreed with you, a QSizeF is not good either, why you don't want to just use 
two qreals?


core/generator.h


Why do we still need this?



core/generator.h


const QSizeF& is not a good idea as returning value, just return QSizeF



core/utils.h


something's up with indentation


- Albert Astals Cid


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

Re: [Okular-devel] Review Request 111829: Use DPI of current screen for PDF rendering

[Eugene Shalygin]

> On Aug. 15, 2013, 11:03 p.m., Albert Astals Cid 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?


- Eugene


---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/111829/#review37877
---


On Aug. 16, 2013, 12:46 a.m., Eugene Shalygin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/111829/
> ---
> 
> (Updated Aug. 16, 2013, 12:46 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
> -
> 
>   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

Re: [Okular-devel] Review Request 111829: Use DPI of current screen for PDF rendering

[Eugene Shalygin]

---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/111829/
---

(Updated Aug. 16, 2013, 12:46 a.m.)


Review request for Okular and Albert Astals Cid.


Changes
---

Fix issues 


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)
-

  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

Re: [Okular-devel] Review Request 111829: Use DPI of current screen for PDF rendering

[Eugene Shalygin]

> On Aug. 15, 2013, 11:03 p.m., Albert Astals Cid wrote:
> > core/generator.h, line 214
> > 
> >
> > Do we really need this class? I think I'd do with just
> > 
> > setDPI(qreal dpiX, qreal dpiY)
> > 
> > Less classes means less worries about having to maintain the api in it

Maybe setDPI(const QSizeF&) ? Just to consolidate information. QSizeF is not 
the best class for this purpose, therefore I've added Geneartor::DPI


- Eugene


---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/111829/#review37877
---


On Aug. 2, 2013, 2:19 p.m., Eugene Shalygin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/111829/
> ---
> 
> (Updated Aug. 2, 2013, 2:19 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
> -
> 
>   CMakeLists.txt 217337f 
>   config-okular.h.cmake 7217f8d 
>   core/document.cpp 6d462f6 
>   core/document_p.h 3a257de 
>   core/generator.h a5a971b 
>   core/generator.cpp 41beb92 
>   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

Re: [Okular-devel] Review Request 111829: Use DPI of current screen for PDF rendering

[Albert Astals Cid]

---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/111829/#review37877
---



CMakeLists.txt


https://projects.kde.org/projects/playground/libs/libkscreen gives me a 404



core/generator.h


Do we really need this class? I think I'd do with just

setDPI(qreal dpiX, qreal dpiY)

Less classes means less worries about having to maintain the api in it



core/generator.h


Add the @since



core/utils.h


This breaks the API, please let the old variants still exist (and 
potentially mark them as deprecated) and add the new ones with the correct 
@since marker



generators/poppler/generator_pdf.cpp


Don't use this-> makes me unhappy to see it in C++ code :D


- Albert Astals Cid


On Aug. 2, 2013, 12:19 p.m., Eugene Shalygin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/111829/
> ---
> 
> (Updated Aug. 2, 2013, 12:19 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
> -
> 
>   CMakeLists.txt 217337f 
>   config-okular.h.cmake 7217f8d 
>   core/document.cpp 6d462f6 
>   core/document_p.h 3a257de 
>   core/generator.h a5a971b 
>   core/generator.cpp 41beb92 
>   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

Re: [Okular-devel] Review Request 111829: Use DPI of current screen for PDF rendering

[Albert Astals Cid]

> On Aug. 15, 2013, 9:03 p.m., Albert Astals Cid wrote:
> > generators/poppler/generator_pdf.cpp, line 683
> > 
> >
> > Don't use this-> makes me unhappy to see it in C++ code :D

Forgot to add:

Do we really need the UtilizeDPI feature and setDPi being virtual? What is your 
thought about various generators needing to override that? From what i can see 
you are just precalculating the dpi and storing it in the generator, you may as 
well do it in the Generator base class, no? And we save ourselves from adding 
the extra api


- Albert


---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/111829/#review37877
---


On Aug. 2, 2013, 12:19 p.m., Eugene Shalygin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/111829/
> ---
> 
> (Updated Aug. 2, 2013, 12:19 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
> -
> 
>   CMakeLists.txt 217337f 
>   config-okular.h.cmake 7217f8d 
>   core/document.cpp 6d462f6 
>   core/document_p.h 3a257de 
>   core/generator.h a5a971b 
>   core/generator.cpp 41beb92 
>   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

Re: [Okular-devel] Review Request 111829: Use DPI of current screen for PDF rendering

[Eugene Shalygin]

---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/111829/
---

(Updated Aug. 2, 2013, 2:19 p.m.)


Review request for Okular and Albert Astals Cid.


Changes
---

changes to libkscreen were commited


Description (updated)
---

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 6d462f6 
  core/document_p.h 3a257de 
  core/generator.h a5a971b 
  core/generator.cpp 41beb92 
  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

Re: [Okular-devel] Review Request 111829: Use DPI of current screen for PDF rendering

[Eugene Shalygin]

---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/111829/
---

(Updated Aug. 1, 2013, 9:11 p.m.)


Review request for Okular and Albert Astals Cid.


Changes
---

qDebug() -> kDebug()


Description
---

This patch relies on changes to LibKScreen, submitted in 
https://git.reviewboard.kde.org/r/111827/
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)
-

  CMakeLists.txt 217337f 
  config-okular.h.cmake 7217f8d 
  core/document.cpp 6d462f6 
  core/document_p.h 3a257de 
  core/generator.h a5a971b 
  core/generator.cpp 41beb92 
  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] Review Request 111829: Use DPI of current screen for PDF rendering

[Eugene Shalygin]

---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/111829/
---

Review request for Okular and Albert Astals Cid.


Description
---

This patch relies on changes to LibKScreen, submitted in 
https://git.reviewboard.kde.org/r/111827/
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
-

  generators/poppler/generator_pdf.cpp 1a44523 
  core/utils.cpp 5dd8448 
  generators/poppler/generator_pdf.h 5d5853a 
  core/generator.cpp 41beb92 
  core/utils.h 8d5d5fc 
  core/document_p.h 3a257de 
  core/generator.h a5a971b 
  config-okular.h.cmake 7217f8d 
  core/document.cpp 6d462f6 
  CMakeLists.txt 217337f 

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