Bug 276096

There is no reason to see this problem with only PGF images... zoom level don't care about image format.

Please checkout my last changes performed tody about libpgf and try again.

Gilles Caulier

Same issue. Even the small preview that appears if you click on the icon inbetween the horizontal and the vertical scroll-bar at the bottom-right just shows an empty frame if the zoom is set to 100%. Works with non-pgf pictures.

Can you attach PGF file to this entry ?

which option you use about preview. Do you load full image size in this mode ?

Gilles Caulier

It's the image editor, not the preview in the album view. 

If the image fits completely into the image pane in the editor at 100% it is shown. If it is larger and would not be shown completely the issue occurs.

Currently I cannot attach a sample picture because even with lossy compression set to 9 (lowest quality) a 2 MPx picture is bigger than the 1 MB max attachment size.

send it to me by private mail

If you make the image editor's window small enough you can reproduce it with the test images from digikam-software-compilation/core/tests/databases/testimages/a2/pgf>

Attached is what I see at zoom 120% and 100%

Created attachment 61396 [details]
120% zoom

Created attachment 61397 [details]
100% zoom

Incredible, 

converting digikam-software-compilation/core/tests/databases/testimages/a2/pgf/foto001q5.pgf to png, and problem disappear...

i can reproduce the problem. Change zoom level, and look artifacts with canvas tile mechanism. I already see this problem in old past...

It due to a precision issue with tile size computation, probably about image size. code is there :

https://projects.kde.org/projects/extragear/graphics/digikam/repository/revisions/master/entry/utilities/imageeditor/canvas/canvas.cpp#L628

Gilles Caulier




Gilles Cauier

Created attachment 61405 [details]
canvas tile artifacts

Zoom to 768% and look the result.

I think image data are not corrupted. Export this image as well to PNG, and load it. All is fine.

Gilles Caulier

Git commit 3f5e6a3c587aa27573f6b421928f4f9af2da10fc by Marcel Wiesweg.
Committed on 30/06/2011 at 22:09.
Pushed by mwiesweg into branch 'master'.

If the image has no alpha channel, we can avoid QPixmap scanning the whole image for
transparent pixels if we specify the correct QImage format.
As a side effect, this fixes bug 276096, but as explained there, there's a libpgf regression as well.

CCBUG: 276096

M  +2    -2    libs/dimg/dimg.cpp     

http://commits.kde.org/digikam/3f5e6a3c587aa27573f6b421928f4f9af2da10fc

This one was difficult to debug. 

At the root, there is a libpgf regression: When the image has no alpha 
channel, the alpha byte is set to 0, instead of 255.
(we initialize the memory with 0xFF, which seems unnecessary, given that 
libpgf seems to fully write the memory including the alpha byte).

Afterwards, the story goes so that we see the problem only in a very specific 
case:
Whenever we scale, the alpha byte is ignored because hasAlpha is false, so all 
is well. At 100%, no scaling is done, and we pass the data as is to QImage and 
then to QPixmap. QPixmap sees transparent pixels and produces a transparent 
pixmap.

I will commit an optimization which as a side-effect fixes the problem here - 
if alpha is false, the image is RGB32 and not ARGB32, which frees QPixmap from 
scanning the image for transparent pixels.

but I think the underlying regression must be fixed in libpgf, it should 
preferably write 255 to the alpha channel, or not touch the alpha channel (and 
say so in the API docs)

Raphael,

Please take a look in Marcel comment #13. There is a regression in libpgf since i updated the library in digiKam core to 6.1.24.

Gilles Caulier

Marcel,

> This one was difficult to debug.

sorry for that one. I suspect we introduced this behaviour when attempting to fix the non-determinism (i.e. we now initialize all buffers).

- Raphael

Can anyone confirm that this problem also exists in 1.9.0?

Thanks
- Raphael

In 1.9.0, libpgf is not updated with last version. In fact 1.9.0 is not maintained.

But i can do it using an external version of libpgf and recompiling 1.9.0 as well. I will do it today.

Gilles Caulier

Just tested with 1.9.0 (from package repository, using libPGF 6.09.44) on another Ubuntu VM. The problem already exists. I'll see what I can do about that.
- Raphael

Ok thanks Raphael...

Gilles Caulier

Is it possible that the image out buffer (QImage) is somehow initialized to 0x04 (contrary to 0xFF)? In CPGFimage::GetBitmap I see that 'buff' consists of 0x04 values. Explicitly writing 255 to the (otherwise untouched by libPGF) alpha channel has the desired effect of showing the image.

- Raphael

Code excerpt:

            memset(data, sizeof(data), 0xFF);

            pgf.Read(level, CallbackForLibPGF, this);
            pgf.GetBitmap(m_sixteenBit ? width*8 : width*4,
                          (UINT8*)data,
                          m_sixteenBit ? 64 : 32,
                          NULL,
                          CallbackForLibPGF, this);

Hmmm...could it be that sizeof(data) is 4?

I thought the problem is with https://projects.kde.org/projects/extragear/graphics/digikam/repository/revisions/abd03787b7b1e3a392803edc5b40dfb25a69423c/entry/libs/threadimageio/pgfutils.cpp#L80 with the uninitialized QImage buffer (I'm sure I saw a corresponding stacktrace today).
But for pgfloader, should it not be memset(data, 0xFF, width*height*(m_sixteenBit ? 8 : 4)) ?

- Raphael

Created attachment 61825 [details]
memset patch for missing alpha channel values

https://bugs.kde.org/show_bug.cgi?id=276096#c22

> > memset(data, sizeof(data), 0xFF);
> But for pgfloader, should it not be memset(data, 0xFF,
> width*height*(m_sixteenBit ? 8 : 4)) ?

Here we have the 0x4:
We set the first 0xFF bytes to the value of sizeof(data), which is 4 ;-)

Marcel, Raphael,

What's new in this file ? It can be closed for 2.0.0 ?

Gilles Caulier

No related changes in https://projects.kde.org/projects/extragear/graphics/digikam/repository/revisions/master/entry/libs/dimg/loaders/pgfloader.cpp (maybe in another branch/revision?)
Problem identified, patch (https://bugs.kde.org/attachment.cgi?id=61825) submitted. When applied everything should be fine.

- Raphael

Git commit a33630e56184ad1370f668d744c443a1f3d3ea80 by Marcel Wiesweg.
Committed on 23/07/2011 at 15:39.
Pushed by mwiesweg into branch 'master'.

Fix memset call (thanks to Raphael Schweizer)

BUG: 276096

M  +2    -1    NEWS
M  +1    -1    libs/dimg/loaders/pgfloader.cpp

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