[Development] QImage::transformed returns shallow copy for QTransform::TxNone matrix type.

[Simon Hausmann]

Hi,


What I'm suggesting is not to use the cleanup function to release any memory. As you have explained that is not possible as you do not own the data. But you can use

the cleanup callback as a way to determine if Qt still has a reference to your image data or not. If your image _was_ transformed then there should not be any reference

anymore. If it wasn't called, then you do need to call copy() explicitly to release the reference.


Another way would be to call QImage::isDetached(), but that is not public API.


Both are approached that allow you to replace your existing check with something that is less dependent on the implementation of QImage::transformed - in case

it changes one day - but are supposed to make your code more robust.



Simon

________________________________
From: Tomasz Olszak <olszak.tomasz at gmail.com>
Sent: Monday, July 11, 2016 1:40:25 PM
To: Simon Hausmann
Cc: development at qt-project.org
Subject: Re: [Development] QImage::transformed returns shallow copy for QTransform::TxNone matrix type.

The pointer comes from external object and it's lifetime is managed by it. I need to ensure that I will not use it anymore after leaving certain function. Hence it don't even need to be thread operation, So using clean function won't help here.

The problem here is that you can't say if transformed returns shallow or deep copy. Result depends on argument value. Of course the transform returning shallow copy on identity matrix is nice but should be documented somewhere. After reading docs I expected deep copy like from copy function. But maybe it's only me :)

Thanks for profound explanation.


2016-07-11 12:10 GMT+01:00 Simon Hausmann <Simon.Hausmann at qt.io<mailto:Simon.Hausmann at qt.io>>:

Hi,


Thank you for the explanation of your use-case. What remains unclear to me is why it is necessary for you to ensure a deep copy?


Is it related to the fact that your image is created from a raw data pointer and after your worker thread is done, you cannot guarantee

for the life-time of the raw data anymore?


Perhaps you could use the QImage constructor that takes a cleanup function (and cleanupInfo pointer). If your worker thread

ends up making a copy of the image data as a result of your transformation, then your original copy can and will be deleted

when you discard the source of the transform() call. That is something that you can keep track of. Vice-versa if it's not being called,

then you know that transformed() (or generally speaking _any_ sequence of operations you may have done) still operate on a shallow

copy. Then you can do an explicit deep copy.


Generally speaking my impression - assuming I did understand your use-case correctly - is that your problem stems from the unknown

life-cycle of your data. I don't think the behavior of QImage::transformed() should be made worse for the common case where shallow

copying benefits performance and memory consumption.


Simon

________________________________
From: Development <development-bounces+simon.hausmann=qt.io at qt-project.org<mailto:qt.io at qt-project.org>> on behalf of Tomasz Olszak <olszak.tomasz at gmail.com<mailto:olszak.tomasz at gmail.com>>
Sent: Monday, July 11, 2016 12:53:51 PM
To: development at qt-project.org<mailto:development at qt-project.org>
Subject: Re: [Development] QImage::transformed returns shallow copy for QTransform::TxNone matrix type.

QImage::copy returns deep copy and has similar documentation so I assumed that when docs say copy it means deep copy.

Let's consider common case:

1. QImage img created from raw data pointer got from e.g. driver.
2. img used in thread to perform some transforms
3. Save result as deep copy

Ensuring step 3 is most efficient and does not perform unnecessary copying I need to write something like:

QTransform t;
if (m.type() == QTransform::TxNone)
    return img.copy();
else
    return img.transformed(m);

AFAIU img.transformed(m).copy() will copy twice when  m.type() != QTransform::TxNone right?

Don't you think that is seems strange and is not documented enough?

2016-07-11 10:59 GMT+01:00 Simon Hausmann <Simon.Hausmann at qt.io<mailto:Simon.Hausmann at qt.io>>:

Hi,


Could you elaborate on what you see as the discrepancy between docs and implementation? The docs don't say whether

it's a shallow or a deep copy, so it looks to me that the implementation is within the bounds of the docs.


Plus it seems sensible to return a shallow copy, doesn't it?



Simon

________________________________
From: Development <development-bounces+simon.hausmann=qt.io at qt-project.org<mailto:qt.io at qt-project.org>> on behalf of Tomasz Olszak <olszak.tomasz at gmail.com<mailto:olszak.tomasz at gmail.com>>
Sent: Monday, July 11, 2016 11:38:06 AM
To: development at qt-project.org<mailto:development at qt-project.org>
Subject: [Development] QImage::transformed returns shallow copy for QTransform::TxNone matrix type.

Hello,

QImage:: transformed(const QTransform &matrix, Qt::TransformationMode mode = Qt::FastTransformation) docs:

"Returns a copy of the image that is transformed using the given transformation matrix and transformation mode."

But if matrix.type() == QTransform::TxNone then shallow instead of deep copy is returned.

I'm happy to submit a fix but I don't know what is expected behaviour? Should implementation follow docs or docs or docs follow implementation here?

T.




-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.qt-project.org/pipermail/development/attachments/20160711/0066ff40/attachment.html>