[digikam] [Bug 337231] New: use high resolution pixmaps on retina displays [patch]

[Axel]

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

            Bug ID: 337231
           Summary: use high resolution pixmaps on retina displays [patch]
    Classification: Unclassified
           Product: digikam
           Version: 4.2.0
          Platform: MacPorts Packages
                OS: OS X
            Status: UNCONFIRMED
          Severity: wishlist
          Priority: NOR
         Component: Preview
          Assignee: [hidden email]
          Reporter: [hidden email]

On Mac OS X retina displays, photos in digikam look pretty pixelized, because
each logical pixel actually consists of 2x2 physical pixels, with
correspondingly sharp borders. This is due to the fact that Qt reports only
half of the actual resolution to the application, so that pixel-based layouts
do not get unreadable.  Qt then handles native resolution for UI elements,
rendering e.g. fonts at native resolution. Also, if pixmaps have a higher
resolution, Qt renders them at native resolution. However, at present, the
pixmaps that digikam provides to Qt are constructed at logical resolution.

I have patched digikam to provide higher resolution pixmaps and would like to
contribute this patch. It makes use of the fact full resolution images are
loaded if "Embedded preview loads full-sized images" is set. The displayed
pixmaps in preview and slide show are then made large enough to contain the
full resolution preview image (or a part of it), rather then just the (logical)
screen pixels covered. By this, Qt scales the pixmap to match the physical
screen resolution, giving a pretty brilliant picture on a Macbook Retina. The
fix also changes the Info.plist to make Mac OS X aware that digikam can handle
the high resolution.

Reproducible: Always

Steps to Reproduce:
1. Start digikam with a retina display (e.g. Macbook Retina)
2. Preview any 5+ MPixels photo
Actual Results:  
The preview or slideshow are displayed pixelized, with logical pixels
consisting of 2x2 physical pixels each.

Expected Results:  
The preview should be displayed in native resolution, i.e. without blocking
pixels.

--
You are receiving this mail because:
You are the assignee for the bug.
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

[Axel]

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

--- Comment #1 from Axel <[hidden email]> ---
Created attachment 87637
  --> https://bugs.kde.org/attachment.cgi?id=87637&action=edit
use high res pixmaps on retina displays

--
You are receiving this mail because:
You are the assignee for the bug.
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

[Gilles Caulier-4]

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

Gilles Caulier <[hidden email]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[hidden email]

--- Comment #2 from Gilles Caulier <[hidden email]> ---
Thanks for the patch.

I appreciate all OSX patches, since i use Macbook pro here.

In your patch, you have introduced a new Info.plist.in patched automatically by
cmake at configuration time.

What's this kind of type of file ?

All file patched by cmake must use .in_cmake extension and must be located to
core/cmake/templates subdir.

Gilles Caulier

--
You are receiving this mail because:
You are the assignee for the bug.
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

[Axel]

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

--- Comment #3 from Axel <[hidden email]> ---
Created attachment 87641
  --> https://bugs.kde.org/attachment.cgi?id=87641&action=edit
updated patch with correct path for Info.plist template

--
You are receiving this mail because:
You are the assignee for the bug.
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

[Axel]

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

--- Comment #4 from Axel <[hidden email]> ---
Sorry, I wasn't aware of this policy. The Info.plist template is by the cmake
Mac OS X target to create the resource info for the bundle. This Info.plist
needs to mark the application to be high resolution capable, which is the
"NSPrincipalClass->NSApplication". There is a template included with cmake,
which was used by digikam before, but that lacks this pair, so I had to provide
a fixed one. In addition, I filled some of the fields like "License", which are
also empty in the standard cmake template.

An updated patch with the relocated Info.plist.in_cmake is attached.

--
You are receiving this mail because:
You are the assignee for the bug.
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

[Gilles Caulier-4]

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

--- Comment #5 from Gilles Caulier <[hidden email]> ---
Excellent...

I will review and apply patch to git/master.

Q: Do you know well OSX envirronnement. There are few topic to fix with digiKam
as, for ex, the capability to load digiKam when a camear device or a memory
card is plug to computer.
This kind of feature work with iPhoto for ex, but i never found a way to
implement it under OSX.

Gilles Caulier

--
You are receiving this mail because:
You are the assignee for the bug.
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

[Gilles Caulier-4]

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

--- Comment #6 from Gilles Caulier <[hidden email]> ---
In Info.plist file, "signature" field is patched with "????". This string must
not be empty instead ?

Gilles Caulier

--
You are receiving this mail because:
You are the assignee for the bug.
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

[Gilles Caulier-4]

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

Gilles Caulier <[hidden email]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
   Version Fixed In|                            |4.2.0
         Resolution|---                         |FIXED
      Latest Commit|                            |http://commits.kde.org/digi
                   |                            |kam/54eeb3c49f3dba2d8a29ecc
                   |                            |f8c9150b1f42fcc7a

--- Comment #7 from Gilles Caulier <[hidden email]> ---
Git commit 54eeb3c49f3dba2d8a29eccf8c9150b1f42fcc7a by Gilles Caulier.
Committed on 08/07/2014 at 15:36.
Pushed by cgilles into branch 'master'.

apply patch #87641 to handle Retina display properly under OSX with preview
image mode.
FIXED-IN: 4.2.0

A  +38   -0    cmake/templates/Info.plist.in_cmake
M  +9    -0    digikam/CMakeLists.txt
M  +19   -11   libs/widgets/graphicsview/graphicsdimgitem.cpp
M  +53   -15   utilities/slideshow/slideshow.cpp

http://commits.kde.org/digikam/54eeb3c49f3dba2d8a29eccf8c9150b1f42fcc7a

--
You are receiving this mail because:
You are the assignee for the bug.
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

[Axel]

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

--- Comment #8 from Axel <[hidden email]> ---
Thanks for pushing so fast!

As for the "????" signature, that I actually kept from the standard template
included in cmake-3.0 (MacOSXBundleInfo.plist.in), also I was also wondering
why they put that.

Regarding the device plugging, I will take a look. I haven't worked on that
part in Mac OS X (yet), but it is actually an interesting problem :-).

--
You are receiving this mail because:
You are the assignee for the bug.
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

[Marcel Wiesweg]

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

--- Comment #9 from Marcel Wiesweg <[hidden email]> ---
Comment on attachment 87637
  --> https://bugs.kde.org/attachment.cgi?id=87637
use high res pixmaps on retina displays

Some minor comments:
- is the "cut-off" approach of float->int conversion by using int(float-value)
preferable to using QRectF::toRect() which provides proper rounding?
- maybe you can expand the comment a bit by adding some of the explanation from
your mail so that the problem can be understood from the code and  the comment

--
You are receiving this mail because:
You are the assignee for the bug.
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

[Gilles Caulier-4]

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

--- Comment #10 from Gilles Caulier <[hidden email]> ---
Axel,

Do you see comment #9 from Marcel ? Can you make a new patch against git/master
fixing reported points ?

Gilles Caulier

--
You are receiving this mail because:
You are the assignee for the bug.
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

[Axel]

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

--- Comment #11 from Axel <[hidden email]> ---
Yes, sorry for the delay. I will switch to QRectF and also expand the comment,
but I won't have time before Friday evening.

Axel

--
You are receiving this mail because:
You are the assignee for the bug.
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

[Axel]

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

--- Comment #12 from Axel <[hidden email]> ---
Created attachment 87715
  --> https://bugs.kde.org/attachment.cgi?id=87715&action=edit
Expanded comments and Qt rounding

Please find attached a patch that fixes the issues raised by Marcel. The code
now uses QRectF/QSizeF for rounding everywhere.

--
You are receiving this mail because:
You are the assignee for the bug.
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

[Gilles Caulier-4]

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

--- Comment #13 from Gilles Caulier <[hidden email]> ---
Git commit 6a7a0d543fdfdc7a8cbd379a641e67d20f808094 by Gilles Caulier.
Committed on 13/07/2014 at 07:31.
Pushed by cgilles into branch 'master'.

apply patch #87715 from Axel Arnolda

M  +25   -7    libs/widgets/graphicsview/graphicsdimgitem.cpp
M  +20   -4    utilities/slideshow/slideshow.cpp

http://commits.kde.org/digikam/6a7a0d543fdfdc7a8cbd379a641e67d20f808094

--
You are receiving this mail because:
You are the assignee for the bug.
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

[Axel]

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

--- Comment #14 from Axel <[hidden email]> ---
An unpleasant follow-up: I just noticed that the June 24 patch for the MacPorts
kdelibs4 jeopardizes my patch. It makes rasterization the default rendering
method rather than using the native rendering which is retina-capable.

There are two ways to get the native rendering back, which are really
MacPorts-specific:
- either, one edits once more the Info.plist, and adds an environment variable
"RasterOff".
- or, one calls a static function of KApplication called doNotRaster before
creating the KApplication.

The latter approach definitely can't go into the main digikam source, since it
is MacPorts-specific, and only for versions of kdelibs4 >= 4.12.5_1. Setting
the environment variable won't harm, but looks a bit ad-hoc for those that
compile kde themselves. It also can't be fixed in MacPorts at the moment, since
MacPorts installs still digikam 3.5.0.

--
You are receiving this mail because:
You are the assignee for the bug.
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

[Gilles Caulier-4]

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

--- Comment #15 from Gilles Caulier <[hidden email]> ---
Using pre processor rules cannot fix the problem with your last patch ?

Do you suggest to revert your patch ?

Gilles Caulier

--
You are receiving this mail because:
You are the assignee for the bug.
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

[Axel]

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

--- Comment #16 from Axel <[hidden email]> ---
Created attachment 87795
  --> https://bugs.kde.org/attachment.cgi?id=87795&action=edit
fix telling Macports' kdelibs not use rastering

The retina patch itself is fine, therefore there is no need to revert it. But
since Macports is the recommended installation route on Mac for digikam, the
patch should also work with MacPorts, probably.
To that aim the attached patch sets the environment variable "RasterOff" in the
Info.plist. This tells the Macports kdelibs to not use raster rendering and
thus allows retina resolution, and won't have any effect on the stock kdelibs,
with which retina resolution should work anyways.

--
You are receiving this mail because:
You are the assignee for the bug.
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

[Axel]

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

--- Comment #17 from Axel <[hidden email]> ---
Created attachment 87796
  --> https://bugs.kde.org/attachment.cgi?id=87796&action=edit
also make imageeditor and showfoto retina-capable

This patch also enables retina rendering in the digikam image editor and the
showfoto application.

--
You are receiving this mail because:
You are the assignee for the bug.
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

[Gilles Caulier-4]

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

--- Comment #18 from Gilles Caulier <[hidden email]> ---
Git commit cda1f5a7d81a2bd01803e5bd284e136a29234319 by Gilles Caulier.
Committed on 18/07/2014 at 09:31.
Pushed by cgilles into branch 'master'.

apply patch #87795 to indicate to not use Raster rendering under OSX

M  +5    -0    cmake/templates/Info.plist.cmake.in

http://commits.kde.org/digikam/cda1f5a7d81a2bd01803e5bd284e136a29234319

--
You are receiving this mail because:
You are the assignee for the bug.
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

[Gilles Caulier-4]

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

--- Comment #19 from Gilles Caulier <[hidden email]> ---
Git commit d8ab531034a6d25736676f47b177b9d43ba0bd6d by Gilles Caulier.
Committed on 18/07/2014 at 09:33.
Pushed by cgilles into branch 'master'.

appply patch #87796 to support retina display under OSX with Image editor and
Showfoto

M  +9    -0    showfoto/CMakeLists.txt
M  +35   -8    utilities/imageeditor/widgets/imagepreviewitem.cpp

http://commits.kde.org/digikam/d8ab531034a6d25736676f47b177b9d43ba0bd6d

--
You are receiving this mail because:
You are the assignee for the bug.
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel