Bad behaviour with really extreme images

[Ilya Konkov]

Hello,

I undestand that this is really not very usable case, but anyway:

QImage image(1, 10000, QImage::Format_Mono);
image.fill(0);
image.save("image.png", "PNG");

Opening this image with gwenview gives:
ASSERT: "!image.isNull()" in file
/home/eruart/app/kdesvn/kdegraphics/gwenview/lib/imagescaler.cpp, line
145

"JPG" version dies too, but differently.

--
Ilya Konkov

-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/

[Aurélien Gâteau]

Post by Ilya Konkov
Hello,
QImage image(1, 10000, QImage::Format_Mono);
image.fill(0);
image.save("image.png", "PNG");
ASSERT: "!image.isNull()" in file
/home/eruart/app/kdesvn/kdegraphics/gwenview/lib/imagescaler.cpp, line
145

Interesting, maybe we should check that we don't try to resize to
something less than 1 pixel width or height? I guess what's happening
here is that width() / mZoom returns something too close to 0. Can you
check this?

Aurélien

-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/

[Ilya Konkov]

On Thu, Jul 24, 2008 at 5:51 PM, Aurélien Gâteau

Post by Aurélien Gâteau

Post by Ilya Konkov
Hello,
QImage image(1, 10000, QImage::Format_Mono);
image.fill(0);
image.save("image.png", "PNG");
ASSERT: "!image.isNull()" in file
/home/eruart/app/kdesvn/kdegraphics/gwenview/lib/imagescaler.cpp, line
145

Interesting, maybe we should check that we don't try to resize to
something less than 1 pixel width or height? I guess what's happening
here is that width() / mZoom returns something too close to 0. Can you
check this?
Aurélien
-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
_______________________________________________
Gwenview-general mailing list
https://lists.sourceforge.net/lists/listinfo/gwenview-general

Ok. With this changes gwenview doesn't fall on png image:

Index: lib/document/document.cpp
===================================================================
--- lib/document/document.cpp (revision 837350)
+++ lib/document/document.cpp (working copy)
@@ -330,6 +330,11 @@
if (loadingState() == Loaded) {
// Resample image from the full one
d->mDownSampledImageMap[invertedZoom] =
d->mImage.scaled(d->mImage.size() / invertedZoom, Qt::KeepAspectRatio,
Qt::FastTransformation);
+ kDebug() <<
"d->mDownSampledImageMap[invertedZoom].size()" <<
d->mDownSampledImageMap[invertedZoom].size();
+ if (!d->mDownSampledImageMap[invertedZoom].size().isValid()) {
+ d->mDownSampledImageMap[invertedZoom] = d->mImage;
+ return false;
+ }
return true;
}


And output:
gwenview(21254) Gwenview::Document::prepareDownSampledImageForZoom:
d->mDownSampledImageMap[invertedZoom].size() QSize(-1, -1)
QPainter::begin: Cannot paint on a null pixmap
QPainter::setCompositionMode: Painter not active


But still fails to open .jpg image:

Program received signal SIGFPE, Arithmetic exception.
[Switching to Thread 0xb4424b90 (LWP 21595)]
0xb7ebddbd in ?? () from /home/eruart/app/kde/lib/libgwenviewlib.so.4
(gdb)
Continuing.
KCrash: crashing... crashRecursionCounter = 2
KCrash: Application Name = gwenview path = <unknown> pid = 21586
sock_file=/home/eruart//.kde4/socket-eNote/kdeinit4__0
gwenview: Fatal IO error: client killed

--
Ilya Konkov
-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/

[Aurélien Gâteau]

Post by Ilya Konkov
On Thu, Jul 24, 2008 at 5:51 PM, Aurélien Gâteau

Post by Aurélien Gâteau

Post by Ilya Konkov
Hello,
QImage image(1, 10000, QImage::Format_Mono);
image.fill(0);
image.save("image.png", "PNG");
ASSERT: "!image.isNull()" in file
/home/eruart/app/kdesvn/kdegraphics/gwenview/lib/imagescaler.cpp, line
145

Interesting, maybe we should check that we don't try to resize to
something less than 1 pixel width or height? I guess what's happening
here is that width() / mZoom returns something too close to 0. Can you
check this?
Aurélien
-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
_______________________________________________
Gwenview-general mailing list
https://lists.sourceforge.net/lists/listinfo/gwenview-general

Index: lib/document/document.cpp
===================================================================
--- lib/document/document.cpp (revision 837350)
+++ lib/document/document.cpp (working copy)
@@ -330,6 +330,11 @@
if (loadingState() == Loaded) {
// Resample image from the full one
d->mDownSampledImageMap[invertedZoom] =
d->mImage.scaled(d->mImage.size() / invertedZoom, Qt::KeepAspectRatio,
Qt::FastTransformation);
+ kDebug() <<
"d->mDownSampledImageMap[invertedZoom].size()" <<
d->mDownSampledImageMap[invertedZoom].size();
+ if (!d->mDownSampledImageMap[invertedZoom].size().isValid()) {
+ d->mDownSampledImageMap[invertedZoom] = d->mImage;
+ return false;
+ }
return true;
}
d->mDownSampledImageMap[invertedZoom].size() QSize(-1, -1)
QPainter::begin: Cannot paint on a null pixmap
QPainter::setCompositionMode: Painter not active

The patch looks ok, except it should return true, IMHO because down
sampled is ready.

Post by Ilya Konkov
Program received signal SIGFPE, Arithmetic exception.
[Switching to Thread 0xb4424b90 (LWP 21595)]
0xb7ebddbd in ?? () from /home/eruart/app/kde/lib/libgwenviewlib.so.4
(gdb)
Continuing.
KCrash: crashing... crashRecursionCounter = 2
KCrash: Application Name = gwenview path = <unknown> pid = 21586
sock_file=/home/eruart//.kde4/socket-eNote/kdeinit4__0
gwenview: Fatal IO error: client killed

The cause of this should be easy to catche with the debugger, SIGFPE
makes me think about a divide by zero error.

Can you add test cases for both images to DocumentTest? Otherwise I am
sure this bug will come back to bite us again.

Aurélien

-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/

[Ilya Konkov]

Hello,

Post by Aurélien Gâteau
The patch looks ok, except it should return true, IMHO because down
sampled is ready.

I actually didn't consider it as a patch, but just a fix to show where
is the problem, because I didn't know if it was a proper way to handle
invalid size.
With new attached "patch" no asserts or crashes appear for the images.
And I think that setting some default picture for non-loaded
thumbnails is a good idea, because empty one looks weird =(

Post by Aurélien Gâteau
Can you add test cases for both images to DocumentTest? Otherwise I am
sure this bug will come back to bite us again.

I am not sure what did you want me to do exactly, so:
- I've added images to testLoad(), but test passes ok with these
pictures even without fixes and that's explainably, because crashes
appear only with scaled images and thumbnails.

- I've changed testLoadDownSampled() to catch crash with 1x10k.jpg
(with 0.4 zoom it passes well).


--
Ilya Konkov

-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/

[Ilya Konkov]

Post by Ilya Konkov
Hello,

Post by Aurélien Gâteau
The patch looks ok, except it should return true, IMHO because down
sampled is ready.

I actually didn't consider it as a patch, but just a fix to show where
is the problem, because I didn't know if it was a proper way to handle
invalid size.
With new attached "patch" no asserts or crashes appear for the images.
And I think that setting some default picture for non-loaded
thumbnails is a good idea, because empty one looks weird =(

Post by Aurélien Gâteau
Can you add test cases for both images to DocumentTest? Otherwise I am
sure this bug will come back to bite us again.

- I've added images to testLoad(), but test passes ok with these
pictures even without fixes and that's explainably, because crashes
appear only with scaled images and thumbnails.
- I've changed testLoadDownSampled() to catch crash with 1x10k.jpg
(with 0.4 zoom it passes well).
--
Ilya Konkov

Erm, somehow attaches have cleared.

--
Ilya Konkov

-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/

[Ilya Konkov]

Post by Ilya Konkov
Erm, somehow attaches have cleared.

Again, don't know what's wrong.
Filed bug to bugzilla with attachments:
http://bugs.kde.org/show_bug.cgi?id=168261



--
Ilya Konkov

-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/

[Aurélien Gâteau]

Post by Ilya Konkov
Hello,

Post by Aurélien Gâteau
The patch looks ok, except it should return true, IMHO because down
sampled is ready.

I actually didn't consider it as a patch, but just a fix to show where
is the problem, because I didn't know if it was a proper way to handle
invalid size.
With new attached "patch" no asserts or crashes appear for the images.
And I think that setting some default picture for non-loaded
thumbnails is a good idea, because empty one looks weird =(

Thanks for the patch. I applied it. It still fails for JPEG, but it
improves situation with PNG. You will notice I also improved the way
test data is defined so that the filename of the image is displayed on
error.

Aurélien

-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/