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

[Tomasz Olszak]

Yes seems more robust but it also seems like using a sledgehammer to crack
a nut. Seems like it is not worth to burn you time for that. If everyone is
ok with docs I'm too :)

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

> 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>:
>
>> 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> on behalf of Tomasz Olszak <olszak.tomasz at gmail.com
>> >
>> *Sent:* Monday, July 11, 2016 12:53:51 PM
>> *To:* 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>:
>>
>>> 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> on behalf of Tomasz Olszak <
>>> olszak.tomasz at gmail.com>
>>> *Sent:* Monday, July 11, 2016 11:38:06 AM
>>> *To:* 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/e1a3238f/attachment.html>