Discussion:
Review Request 129929: Port away from KImageIO
Martin Tobias Holmedahl Sandsmark
2017-02-06 22:36:00 UTC
Permalink
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129929/
-----------------------------------------------------------

Review request for KDE Graphics and Martin Koller.


Repository: kolourpaint


Description
-------

KImageIO is apparently deprecated, so use similar code to what is now used in Gwenview.


Diffs
-----

document/kpDocument.cpp 69aa8d83
document/kpDocument_Open.cpp 7c0be0b4
document/kpDocument_Save.cpp 1434832a
document/kpDocument_Selection.cpp efdd735a

Diff: https://git.reviewboard.kde.org/r/129929/diff/


Testing
-------

Saving as different formats works as expected here.


Thanks,

Martin Tobias Holmedahl Sandsmark
Michael Pyne
2017-02-07 03:10:02 UTC
Permalink
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129929/#review102435
-----------------------------------------------------------


Ship it!




Ship It!

- Michael Pyne
Post by Martin Tobias Holmedahl Sandsmark
-----------------------------------------------------------
https://git.reviewboard.kde.org/r/129929/
-----------------------------------------------------------
(Updated Feb. 6, 2017, 10:36 p.m.)
Review request for KDE Graphics and Martin Koller.
Repository: kolourpaint
Description
-------
KImageIO is apparently deprecated, so use similar code to what is now used in Gwenview.
Diffs
-----
document/kpDocument.cpp 69aa8d83
document/kpDocument_Open.cpp 7c0be0b4
document/kpDocument_Save.cpp 1434832a
document/kpDocument_Selection.cpp efdd735a
Diff: https://git.reviewboard.kde.org/r/129929/diff/
Testing
-------
Saving as different formats works as expected here.
Thanks,
Martin Tobias Holmedahl Sandsmark
Martin Koller
2017-02-07 08:05:56 UTC
Permalink
Post by Michael Pyne
Ship It!
No, that won't work.
Post by Michael Pyne
I'm working on porting kolourpaint to kf5.
KDELIBS4SUPPORT_DEPRECATED_EXPORT QStringList typeForMime(const QString
&mimeType);
The comment says: Use QMimeType::name() instead().
However this seems incorrect.
typeForMime() returned a format string usable for QImage::save(), e.g.
"image/png" returns "PNG"
QMimeType::name() returns the name of the mime type, which is again
"image/png", which I can not pass to QImage::save()
(typeForMime() gives the X-KDE-ImageFormat field from the desktop file, and
the mime type is in the X-KDE-MimeType field)
So, is there a REAL alternative for this method ?
Ah, you want QMimeType::suffixes() instead, since QImage types are essentially
file extensions.
no, this is not the same.
suffixes() returns a QStringList(!), for instance this would return "jpg", "jpeg" for image/jpeg.
But the QImage::save() method needs exactly ONE specific string as format.
How should my code know which one to use ?
I had plans a while back to submit a patch to Qt to do implement the
equivalent of typeForMime and its inverse properly (using the data in the
QImageFormat plugin metadata), but never got round to finishing it. Hopefully
I'll have the time and energy to pick that up again at some point.
I'd say a better solution would be to simply have no need for such a method, e.g.
allow that QImageWriter (QImage, QPimap) accepts a QMimeType.


- Martin


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129929/#review102435
-----------------------------------------------------------
-----------------------------------------------------------
https://git.reviewboard.kde.org/r/129929/
-----------------------------------------------------------
(Updated Feb. 6, 2017, 10:36 p.m.)
Review request for KDE Graphics and Martin Koller.
Repository: kolourpaint
Description
-------
KImageIO is apparently deprecated, so use similar code to what is now used in Gwenview.
Diffs
-----
document/kpDocument.cpp 69aa8d83
document/kpDocument_Open.cpp 7c0be0b4
document/kpDocument_Save.cpp 1434832a
document/kpDocument_Selection.cpp efdd735a
Diff: https://git.reviewboard.kde.org/r/129929/diff/
Testing
-------
Saving as different formats works as expected here.
Thanks,
Martin Tobias Holmedahl Sandsmark
Martin Tobias Holmedahl Sandsmark
2017-06-03 19:27:00 UTC
Permalink
Post by Martin Koller
Post by Michael Pyne
Ship It!
No, that won't work.
Post by Michael Pyne
Post by Michael Pyne
I'm working on porting kolourpaint to kf5.
KDELIBS4SUPPORT_DEPRECATED_EXPORT QStringList typeForMime(const QString
&mimeType);
The comment says: Use QMimeType::name() instead().
However this seems incorrect.
typeForMime() returned a format string usable for QImage::save(), e.g.
"image/png" returns "PNG"
QMimeType::name() returns the name of the mime type, which is again
"image/png", which I can not pass to QImage::save()
(typeForMime() gives the X-KDE-ImageFormat field from the desktop file, and
the mime type is in the X-KDE-MimeType field)
So, is there a REAL alternative for this method ?
Ah, you want QMimeType::suffixes() instead, since QImage types are essentially
file extensions.
no, this is not the same.
suffixes() returns a QStringList(!), for instance this would return "jpg", "jpeg" for image/jpeg.
But the QImage::save() method needs exactly ONE specific string as format.
How should my code know which one to use ?
Post by Michael Pyne
I had plans a while back to submit a patch to Qt to do implement the
equivalent of typeForMime and its inverse properly (using the data in the
QImageFormat plugin metadata), but never got round to finishing it. Hopefully
I'll have the time and energy to pick that up again at some point.
I'd say a better solution would be to simply have no need for such a method, e.g.
allow that QImageWriter (QImage, QPimap) accepts a QMimeType.
My patch doesn't try to guess which one from a QStringList(), that's what the old code does. With this patch it uses preferredSuffix(), which solves the problem.


- Martin Tobias Holmedahl


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129929/#review102435
-----------------------------------------------------------
Post by Martin Koller
-----------------------------------------------------------
https://git.reviewboard.kde.org/r/129929/
-----------------------------------------------------------
(Updated Feb. 6, 2017, 10:36 p.m.)
Review request for KDE Graphics and Martin Koller.
Repository: kolourpaint
Description
-------
KImageIO is apparently deprecated, so use similar code to what is now used in Gwenview.
Diffs
-----
document/kpDocument.cpp 69aa8d83
document/kpDocument_Open.cpp 7c0be0b4
document/kpDocument_Save.cpp 1434832a
document/kpDocument_Selection.cpp efdd735a
Diff: https://git.reviewboard.kde.org/r/129929/diff/
Testing
-------
Saving as different formats works as expected here.
Thanks,
Martin Tobias Holmedahl Sandsmark
Martin Koller
2017-06-04 12:20:44 UTC
Permalink
Post by Martin Tobias Holmedahl Sandsmark
Post by Martin Koller
Post by Michael Pyne
Ship It!
No, that won't work.
Post by Michael Pyne
Post by Michael Pyne
I'm working on porting kolourpaint to kf5.
KDELIBS4SUPPORT_DEPRECATED_EXPORT QStringList typeForMime(const QString
&mimeType);
The comment says: Use QMimeType::name() instead().
However this seems incorrect.
typeForMime() returned a format string usable for QImage::save(), e.g.
"image/png" returns "PNG"
QMimeType::name() returns the name of the mime type, which is again
"image/png", which I can not pass to QImage::save()
(typeForMime() gives the X-KDE-ImageFormat field from the desktop file, and
the mime type is in the X-KDE-MimeType field)
So, is there a REAL alternative for this method ?
Ah, you want QMimeType::suffixes() instead, since QImage types are essentially
file extensions.
no, this is not the same.
suffixes() returns a QStringList(!), for instance this would return "jpg", "jpeg" for image/jpeg.
But the QImage::save() method needs exactly ONE specific string as format.
How should my code know which one to use ?
Post by Michael Pyne
I had plans a while back to submit a patch to Qt to do implement the
equivalent of typeForMime and its inverse properly (using the data in the
QImageFormat plugin metadata), but never got round to finishing it. Hopefully
I'll have the time and energy to pick that up again at some point.
I'd say a better solution would be to simply have no need for such a method, e.g.
allow that QImageWriter (QImage, QPimap) accepts a QMimeType.
My patch doesn't try to guess which one from a QStringList(), that's what the old code does. With this patch it uses preferredSuffix(), which solves the problem.
Either you or I misunderstand something here.

My point is, KImageIO::typeForMime() is NOT the same as mimeType.preferredSuffix().
The former returns a keyword which QImage knows and uses to select a format to save as.
I'm not confident that QImage would select the same plugin when you pass a filename suffix.
Can you prove that ALL image formats KDE supports can be written with the respective filename suffix as format keyword ?
(And you still would have the possibility that a user has installed another imageformat plugin which does not
use the suffix as keyword)
--
Best regards/Schöne Grüße

Martin
A: Because it breaks the logical sequence of discussion
Q: Why is top posting bad?

() ascii ribbon campaign - against html e-mail
/\ - against proprietary attachments

Geschenkideen, Accessoires, Seifen, Kulinarisches: www.lillehus.at
Luigi Toscano
2017-10-05 20:21:19 UTC
Permalink
Post by Martin Koller
Post by Michael Pyne
Ship It!
No, that won't work.
Post by Michael Pyne
Post by Michael Pyne
I'm working on porting kolourpaint to kf5.
KDELIBS4SUPPORT_DEPRECATED_EXPORT QStringList typeForMime(const QString
&mimeType);
The comment says: Use QMimeType::name() instead().
However this seems incorrect.
typeForMime() returned a format string usable for QImage::save(), e.g.
"image/png" returns "PNG"
QMimeType::name() returns the name of the mime type, which is again
"image/png", which I can not pass to QImage::save()
(typeForMime() gives the X-KDE-ImageFormat field from the desktop file, and
the mime type is in the X-KDE-MimeType field)
So, is there a REAL alternative for this method ?
Ah, you want QMimeType::suffixes() instead, since QImage types are essentially
file extensions.
no, this is not the same.
suffixes() returns a QStringList(!), for instance this would return "jpg", "jpeg" for image/jpeg.
But the QImage::save() method needs exactly ONE specific string as format.
How should my code know which one to use ?
Post by Michael Pyne
I had plans a while back to submit a patch to Qt to do implement the
equivalent of typeForMime and its inverse properly (using the data in the
QImageFormat plugin metadata), but never got round to finishing it. Hopefully
I'll have the time and energy to pick that up again at some point.
I'd say a better solution would be to simply have no need for such a method, e.g.
allow that QImageWriter (QImage, QPimap) accepts a QMimeType.
My patch doesn't try to guess which one from a QStringList(), that's what the old code does. With this patch it uses preferredSuffix(), which solves the problem.
Any update on this? Martin (Koller), the answer from Martin (Sandsmark) seems to address the issue that you raised.


- Luigi


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129929/#review102435
-----------------------------------------------------------
Post by Martin Koller
-----------------------------------------------------------
https://git.reviewboard.kde.org/r/129929/
-----------------------------------------------------------
(Updated Feb. 6, 2017, 11:36 p.m.)
Review request for KDE Graphics and Martin Koller.
Repository: kolourpaint
Description
-------
KImageIO is apparently deprecated, so use similar code to what is now used in Gwenview.
Diffs
-----
document/kpDocument.cpp 69aa8d83
document/kpDocument_Open.cpp 7c0be0b4
document/kpDocument_Save.cpp 1434832a
document/kpDocument_Selection.cpp efdd735a
Diff: https://git.reviewboard.kde.org/r/129929/diff/1/
Testing
-------
Saving as different formats works as expected here.
Thanks,
Martin Tobias Holmedahl Sandsmark
Martin Koller
2017-12-03 12:37:16 UTC
Permalink
Post by Martin Koller
Post by Michael Pyne
Ship It!
No, that won't work.
Post by Michael Pyne
Post by Michael Pyne
I'm working on porting kolourpaint to kf5.
KDELIBS4SUPPORT_DEPRECATED_EXPORT QStringList typeForMime(const QString
&mimeType);
The comment says: Use QMimeType::name() instead().
However this seems incorrect.
typeForMime() returned a format string usable for QImage::save(), e.g.
"image/png" returns "PNG"
QMimeType::name() returns the name of the mime type, which is again
"image/png", which I can not pass to QImage::save()
(typeForMime() gives the X-KDE-ImageFormat field from the desktop file, and
the mime type is in the X-KDE-MimeType field)
So, is there a REAL alternative for this method ?
Ah, you want QMimeType::suffixes() instead, since QImage types are essentially
file extensions.
no, this is not the same.
suffixes() returns a QStringList(!), for instance this would return "jpg", "jpeg" for image/jpeg.
But the QImage::save() method needs exactly ONE specific string as format.
How should my code know which one to use ?
Post by Michael Pyne
I had plans a while back to submit a patch to Qt to do implement the
equivalent of typeForMime and its inverse properly (using the data in the
QImageFormat plugin metadata), but never got round to finishing it. Hopefully
I'll have the time and energy to pick that up again at some point.
I'd say a better solution would be to simply have no need for such a method, e.g.
allow that QImageWriter (QImage, QPimap) accepts a QMimeType.
My patch doesn't try to guess which one from a QStringList(), that's what the old code does. With this patch it uses preferredSuffix(), which solves the problem.
Any update on this? Martin (Koller), the answer from Martin (Sandsmark) seems to address the issue that you raised.
My point is: a file suffix is not related to the QImageWriter format string.
Probably all the current image-IO plugins use - by chance - the same format string as the preferred file suffix, however I can't say for sure.
And I think this is the reason why the KDE desktop file entry X-KDE-ImageFormat was invented in the first place.
Please correct me if I'm wrong.


- Martin


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129929/#review102435
-----------------------------------------------------------
Post by Martin Koller
-----------------------------------------------------------
https://git.reviewboard.kde.org/r/129929/
-----------------------------------------------------------
(Updated Feb. 6, 2017, 10:36 p.m.)
Review request for KDE Graphics and Martin Koller.
Repository: kolourpaint
Description
-------
KImageIO is apparently deprecated, so use similar code to what is now used in Gwenview.
Diffs
-----
document/kpDocument.cpp 69aa8d83
document/kpDocument_Open.cpp 7c0be0b4
document/kpDocument_Save.cpp 1434832a
document/kpDocument_Selection.cpp efdd735a
Diff: https://git.reviewboard.kde.org/r/129929/diff/1/
Testing
-------
Saving as different formats works as expected here.
Thanks,
Martin Tobias Holmedahl Sandsmark
Loading...