Bug 276096 - PGF images not shown if zoom is set to 100%
Summary: PGF images not shown if zoom is set to 100%
Status: RESOLVED FIXED
Alias: None
Product: digikam
Classification: Applications
Component: ImageEditor-Canvas (show other bugs)
Version: 2.0.0
Platform: openSUSE Linux
: NOR normal
Target Milestone: ---
Assignee: Digikam Developers
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-06-20 01:01 UTC by S. Burmeister
Modified: 2017-08-07 08:38 UTC (History)
2 users (show)

See Also:
Latest Commit:
Version Fixed In: 2.0.0
Sentry Crash Report:


Attachments
120% zoom (361.25 KB, image/png)
2011-06-28 08:11 UTC, S. Burmeister
Details
100% zoom (191.88 KB, image/png)
2011-06-28 08:12 UTC, S. Burmeister
Details
canvas tile artifacts (756.90 KB, image/png)
2011-06-28 10:43 UTC, caulier.gilles
Details
memset patch for missing alpha channel values (593 bytes, patch)
2011-07-12 22:41 UTC, Raphael Schweizer
Details

Note You need to log in before you can comment on or make changes to this bug.
Description S. Burmeister 2011-06-20 01:01:26 UTC
Version:           2.0.0 (using KDE 4.6.4) 
OS:                Linux

With the latest PGF changes the image editor just shows an empty pane when the zooming is set to 100%.

Reproducible: Didn't try

Steps to Reproduce:
Open the image editor with some picture and set the zooming to 100%.
Close the ditor and open a PGF in the editor via pressing F4. No picture is shown.
Click on "fit to screen" to see the image.
Comment 1 caulier.gilles 2011-06-20 16:09:18 UTC
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
Comment 2 S. Burmeister 2011-06-21 16:20:42 UTC
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.
Comment 3 caulier.gilles 2011-06-21 16:57:10 UTC
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
Comment 4 S. Burmeister 2011-06-21 22:50:53 UTC
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.
Comment 5 Marcel Wiesweg 2011-06-22 22:11:23 UTC
send it to me by private mail
Comment 6 S. Burmeister 2011-06-28 08:06:59 UTC
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%
Comment 7 S. Burmeister 2011-06-28 08:11:39 UTC
Created attachment 61396 [details]
120% zoom
Comment 8 S. Burmeister 2011-06-28 08:12:26 UTC
Created attachment 61397 [details]
100% zoom
Comment 9 caulier.gilles 2011-06-28 10:41:58 UTC
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
Comment 10 caulier.gilles 2011-06-28 10:43:56 UTC
Created attachment 61405 [details]
canvas tile artifacts
Comment 11 caulier.gilles 2011-06-28 10:45:18 UTC
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
Comment 12 Marcel Wiesweg 2011-07-11 17:03:05 UTC
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
Comment 13 Marcel Wiesweg 2011-07-11 17:03:45 UTC
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)
Comment 14 caulier.gilles 2011-07-11 17:44:29 UTC
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
Comment 15 Raphael Schweizer 2011-07-11 23:52:22 UTC
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
Comment 16 Raphael Schweizer 2011-07-12 00:43:03 UTC
Can anyone confirm that this problem also exists in 1.9.0?

Thanks
- Raphael
Comment 17 caulier.gilles 2011-07-12 04:23:47 UTC
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
Comment 18 Raphael Schweizer 2011-07-12 10:14:08 UTC
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
Comment 19 caulier.gilles 2011-07-12 10:20:49 UTC
Ok thanks Raphael...

Gilles Caulier
Comment 20 Raphael Schweizer 2011-07-12 13:21:06 UTC
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
Comment 21 Marcel Wiesweg 2011-07-12 18:42:06 UTC
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?
Comment 22 Raphael Schweizer 2011-07-12 22:33:38 UTC
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
Comment 23 Raphael Schweizer 2011-07-12 22:41:26 UTC
Created attachment 61825 [details]
memset patch for missing alpha channel values

https://bugs.kde.org/show_bug.cgi?id=276096#c22
Comment 24 Marcel Wiesweg 2011-07-13 07:08:33 UTC
> > 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 ;-)
Comment 25 caulier.gilles 2011-07-21 09:34:42 UTC
Marcel, Raphael,

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

Gilles Caulier
Comment 26 Raphael Schweizer 2011-07-21 22:21:52 UTC
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
Comment 27 Marcel Wiesweg 2011-07-23 14:00:27 UTC
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