Review for 191719

[Cyrille Berger]

Here are two patches to fix DPI in krita for jpeg and png. I would especially
need someone to check units. Currently krita is consistant with itself...
Would be nice also, if we had the unit of resolution in the UI. (and why do I
have to divide by 72 ?)

--
Cyrille Berger

[fix-191719-png.diff]

---

Index: krita/ui/kis_png_converter.cpp
===================================================================
--- krita/ui/kis_png_converter.cpp (revision 1045026)
+++ krita/ui/kis_png_converter.cpp (working copy)
@@ -541,6 +541,16 @@
             m_img -> addAnnotation(annotation);
         }
     }
+    
+    // Read resolution
+    int unit_type;
+    png_uint_32 x_resolution, y_resolution;
+    
+    png_get_pHYs(png_ptr, info_ptr,&x_resolution,&y_resolution, &unit_type);
+    if (unit_type == PNG_RESOLUTION_METER)
+    {
+        m_img->setResolution((double) x_resolution/100.0 / 72 * 2.54, (double) y_resolution/100.0 / 72 * 2.54 );
+    }
 
     double coeff = quint8_MAX / (double)(pow(2, color_nb_bits) - 1);
     KisPaintLayerSP layer = new KisPaintLayer(m_img.data(), m_img -> nextLayerName(), UCHAR_MAX);
@@ -985,6 +995,12 @@
 #endif
     }
 
+    // Save resolution
+    int unit_type;
+    png_uint_32 x_resolution, y_resolution;
+    
+    png_set_pHYs(png_ptr, info_ptr, img->xRes() * 100.0 * 72 / 2.54, img->yRes() * 100.0 * 72 / 2.54, PNG_RESOLUTION_METER);
+    
     // Save the information to the file
     png_write_info(png_ptr, info_ptr);
     png_write_flush(png_ptr);

---

[fix-191719-jpeg.diff]

---

Index: plugins/formats/jpeg/kis_jpeg_converter.cc
===================================================================
--- plugins/formats/jpeg/kis_jpeg_converter.cc (revision 1045026)
+++ plugins/formats/jpeg/kis_jpeg_converter.cc (working copy)
@@ -204,6 +204,26 @@
         }
     }
 
+    // Set resolution
+    double xres, yres;
+    if (cinfo.density_unit == 0)
+    {
+        xres = 72;
+        yres = 72;
+    }
+    else if ( cinfo.density_unit == 1 )
+    {
+        xres = cinfo.X_density;
+        yres = cinfo.Y_density;
+    }
+    else if ( cinfo.density_unit == 2 )
+    {
+        xres = cinfo.X_density * 2.54;
+        yres = cinfo.Y_density * 2.54;
+    }
+    m_img->setResolution(xres / 72, yres / 72);
+    
+    // Create layer
     KisPaintLayerSP layer = KisPaintLayerSP(new KisPaintLayer(m_img.data(), m_img -> nextLayerName(), quint8_MAX));
 
     // Read data
@@ -454,7 +474,6 @@
     }
     cinfo.in_color_space = color_type;   // colorspace of input image
 
-
     // Set default compression parameters
     jpeg_set_defaults(&cinfo);
     // Customize them
@@ -511,6 +530,12 @@
     break;
     }
 
+    // Save resolution
+    cinfo.X_density = img->xRes() * 72;
+    cinfo.Y_density = img->yRes() * 72;
+    cinfo.density_unit = 1;
+    cinfo.write_JFIF_header = 1;
+
     // Start compression
     jpeg_start_compress(&cinfo, true);
     // Save exif and iptc information if any available

---

_______________________________________________
kimageshop mailing list
kimageshop@...
https://mail.kde.org/mailman/listinfo/kimageshop

On Thursday 05 November 2009, Cyrille Berger wrote:
> Here are two patches to fix DPI in krita for jpeg and png. I would
>  especially need someone to check units. Currently krita is consistant with
>  itself... Would be nice also, if we had the unit of resolution in the UI.
>  (and why do I have to divide by 72 ?)
>

It looks like internally, resolution is expressed as a factor of the KOffice
document resolution, which is 72 dpi. For instance:

QPointF KisImage::documentToPixel(const QPointF &documentCoord) const
{
    return QPointF(documentCoord.x() * xRes(), documentCoord.y() * yRes());
}

And in the dialogs, we show pixels per inch:

KisCustomImageWidget:

    doubleResolution->setValue(72.0 * resolution);
    doubleResolution->setDecimals(0);

--
Boudewijn Rempt | http://www.valdyas.org
_______________________________________________
kimageshop mailing list
kimageshop@...
https://mail.kde.org/mailman/listinfo/kimageshop

On Fri, Nov 6, 2009 at 1:15 AM, Cyrille Berger <cberger@...> wrote:

Here are two patches to fix DPI in krita for jpeg and png. I would especially
need someone to check units. Currently krita is consistant with itself...
Would be nice also, if we had the unit of resolution in the UI. (and why do I
have to divide by 72 ?)

Haven't you committed them? /bin/patch suggests me to use -R option for reverse patching. Are they reverse?


--
Dmitry Kazakov

_______________________________________________
kimageshop mailing list
kimageshop@...
https://mail.kde.org/mailman/listinfo/kimageshop

> On Fri, Nov 6, 2009 at 1:15 AM, Cyrille Berger <cberger@...> wrote:
>
> > Here are two patches to fix DPI in krita for jpeg and png. I would
> > especially
> > need someone to check units. Currently krita is consistant with itself...
> > Would be nice also, if we had the unit of resolution in the UI. (and why do
> > I
> > have to divide by 72 ?)
> >
>
> Haven't you committed them? /bin/patch suggests me to use -R option for
> reverse patching. Are they reverse?
>
>
> --
> Dmitry Kazakov
>

the review is for  the 2.1 branch, you are trying to apply it against trunk, where it already has been committed.

_______________________________________________
kimageshop mailing list
kimageshop@...
https://mail.kde.org/mailman/listinfo/kimageshop

Heh.. =)

> (and why do I have to divide by 72 ?)

Because resolution of the image is stored in "points per pixel" format. Where a "point" (pt) is a theoretical unit equal to 1/72 of the inch sharp.

Btw, I'd suggest you to use predefined macroses from ./libs/kobase/KoUnit.h instead of all this multiplications and divisions in place.


--
Dmitry Kazakov

_______________________________________________
kimageshop mailing list
kimageshop@...
https://mail.kde.org/mailman/listinfo/kimageshop

On Fri, Nov 6, 2009 at 5:09 PM, Dmitry Kazakov <dimula73@...> wrote:

Heh.. =)

> (and why do I have to divide by 72 ?)

Because resolution of the image is stored in "points per pixel" format. Where a "point" (pt) is a theoretical unit equal to 1/72 of the inch sharp.

Btw, I'd suggest you to use predefined macroses from ./libs/kobase/KoUnit.h instead of all this multiplications and divisions in place.

e.g. POINT_TO_CM(x_resolution)/100.0 instead of (x_resolution/100.0 / 72 * 2.54)

--
Dmitry Kazakov

_______________________________________________
kimageshop mailing list
kimageshop@...
https://mail.kde.org/mailman/listinfo/kimageshop

On Fri, Nov 6, 2009 at 10:27 AM, Boudewijn Rempt <boud@...> wrote:

On Thursday 05 November 2009, Cyrille Berger wrote:
> Here are two patches to fix DPI in krita for jpeg and png. I would
>  especially need someone to check units. Currently krita is consistant with
>  itself... Would be nice also, if we had the unit of resolution in the UI.
>  (and why do I have to divide by 72 ?)
>

It looks like internally, resolution is expressed as a factor of the KOffice
document resolution, which is 72 dpi. For instance:

QPointF KisImage::documentToPixel(const QPointF &documentCoord) const
{
   return QPointF(documentCoord.x() * xRes(), documentCoord.y() * yRes());
}

And in the dialogs, we show pixels per inch:

KisCustomImageWidget:

   doubleResolution->setValue(72.0 * resolution);
   doubleResolution->setDecimals(0);

I wouldn't say it is right to say that "internal resolution" is X dpi due to unit difference.

Krita works with special points those are neither pixels of the viewport nor pts. One pixel of KisImage has a size. This size is measured in pt's, (pt is a universal units equal to 1/72 inch).

Let's define a KisImage point as "ipx" (image pixel) (note that this "ipx" are not equivalent to a pixel of viewport)

Let's define a viewport pixel as "vpx" that depends on the size of your display (it is calculated using KoDpi in KoZoomHandler)

KoZoomHandler::m_resolution[XY] stores the number of "vpx"es per one pt.

The number of "ipx"es per one pt is defined by the user and depends on the value of KisImage

KisImage::[xy]Res stores the number of a "ipx"es per pt.


                          ipx per inch
[KisImage::xRes()] =-------------------------
                     pt per inch =const=72


                                    vpx per inch
[KoZoomHandler::m_resolution[XY]] = -------------------------
                               pt per inch =const=72




JPEG stores it's resolution in DPIs.
      
[dpi]= ipx per inch


 
This means that you have to translate a unit in the denominator of KisImage's resolution from pt's to inches. As this value stands in denominator, you have to do a backward operation:

DPI = INCHES_TO_POINT(KisImage::xRes());


You can take a look at how it works in:

krita/ui/canvas/kis_prescaled_projection.cc
libs/flake/KoZoomHandler.cc
libs/kobase/KoUnit.h

More than that, you can activate dbgRender kDegug-messages and KisPrescaledProjection will report you all the interesting information about dpis.




--
Dmitry Kazakov

_______________________________________________
kimageshop mailing list
kimageshop@...
https://mail.kde.org/mailman/listinfo/kimageshop