QImage::scaled description and c++ code inconsistency

[hpaulh]

In the QImage::scaled function there is the following code:

@QSize newSize = size();
newSize.scale(s, aspectMode);
if (newSize == size())
return *this;@

But the description of the scaled functions says:

bq. Returns a copy of the image...

"link to the description of scaled function":http://doc-snapshot.qt-project.org/qt5-5.4/qimage.html#scaled

In my opinion this causes a confusion, because if the scaled image desired size is the size of the original image, the original image itself gets returned and not a copy of it. This can lead to program malfunctions as deleting the original image results in deleting the scaled image too.

As I am a beginner with Qt and Qt forums, please don't get mad if this is not the place for such problems. I am just trying to improve the great thing that Qt represents.

Thanks,
Paul H

[Francknos]

It's normal because if you make return *this;
You return a pointer of object deleted ! Because "return" delete "this".

You have to return QImage or something like that.

[hpaulh]

That is the code found in the qt source code. I believe I cannot or should not try to modify it.

[Francknos]

Where you find it ?

Do you have more code ?

[Rondog]

I don't see a problem with this.

'this' is a pointer to the item and '*this' is the value that the pointer is pointing to (QImage in this case). There is no pointer to the original image involved. From the users point of view there is no difference.

This check is probably there for performance reasons. If the new size is the same as the original no scaling is required so just return a copy of the original. Further in the source code there is probably something that is more complex returning an updated QImage(new_size).

[hpaulh]

the code can be found here
"scaled function":https://qt.gitorious.org/qt/qt/source/57756e72adf2081137b97f0e689dd16c770d10b1:src/gui/image/qimage.cpp#L4373

[hpaulh]

To farther clear what I wanted to say:

running the following code:

@unsigned char * data = new unsigned char[64 * 64]{};
QImage testImg(data, 64, 64, QImage::Format_RGB888);
QImage scaledImg1 = testImg.scaled(64, 64);
QImage scaledImg2 = testImg.scaled(63, 63);

std::cout << "testImg:" << testImg.data_ptr() << "\nscaledImg1:" << scaledImg1.data_ptr() << "\nscaledImg2:" << scaledImg2.data_ptr();@

outputs:

testImg:00000000061E00D0
scaledImg1:00000000061E00D0
scaledImg2:00000000061E0190D

In my opinion the first two pointers should not be pointing to the same address or according to the function description it should not.

Hope this is more clear now.

[Rondog]

There might be more going on in the background. QString, for example, uses something called 'copy on write'. With this you can have many QString variables all pointing to one item. I don't know if this applies to QImage but it might.

If the two copies are the same then you should find that changing one of them (drawing an extra line or something) would update both. You will need to add some extra code to modify one of them and display both side by side.

[SGaist]

Hi,

Like Rondog wrote, QImage is one of the "implicitly shared class":http://doc.qt.io/qt-5/implicit-sharing.html#implicit-data-sharing from Qt. So as long as you're not modifying the image, there's no use to create a copy from it.

[JKSH]

[quote author="hpaulh" date="1423665080"]To farther clear what I wanted to say:

running the following code:

@unsigned char * data = new unsigned char[64 * 64]{};
QImage testImg(data, 64, 64, QImage::Format_RGB888);
QImage scaledImg1 = testImg.scaled(64, 64);
QImage scaledImg2 = testImg.scaled(63, 63);

std::cout << "testImg:" << testImg.data_ptr() << "\nscaledImg1:" << scaledImg1.data_ptr() << "\nscaledImg2:" << scaledImg2.data_ptr();@

outputs:

testImg:00000000061E00D0
scaledImg1:00000000061E00D0
scaledImg2:00000000061E0190D

In my opinion the first two pointers should not be pointing to the same address or according to the function description it should not. [/quote]After you print the pointer values, modify scaledImg1 using setPixel() and print the pointer values again. You'll see what Rondog and SGaist meant.

[hpaulh]

[quote author="SGaist" date="1423694493"]Hi,

Like Rondog wrote, QImage is one of the "implicitly shared class":http://doc.qt.io/qt-5/implicit-sharing.html#implicit-data-sharing from Qt. So as long as you're not modifying the image, there's no use to create a copy from it.[/quote]

Thanks for your reply.

The problem is when you have a code like this:

@QImage image(...);
QImage scaledImage = image.scaled(QSize(same as the image));
someVector.push_back(scaledImage);@

When image goes out of scope, the scaledImage on the vector gets deleted too because it's the same (or at least the data pointer).

And this brings the initial stated problem: maybe if it's not too much of a trouble, state in the description of the scaled function that this situation might occur. I only posted this in the idea that other users can avoid having the same problem.

Thanks!

[hpaulh]

[quote author="JKSH" date="1423699718"][quote author="hpaulh" date="1423665080"]To farther clear what I wanted to say:

running the following code:

@unsigned char * data = new unsigned char[64 * 64]{};
QImage testImg(data, 64, 64, QImage::Format_RGB888);
QImage scaledImg1 = testImg.scaled(64, 64);
QImage scaledImg2 = testImg.scaled(63, 63);

std::cout << "testImg:" << testImg.data_ptr() << "\nscaledImg1:" << scaledImg1.data_ptr() << "\nscaledImg2:" << scaledImg2.data_ptr();@

outputs:

testImg:00000000061E00D0
scaledImg1:00000000061E00D0
scaledImg2:00000000061E0190D

In my opinion the first two pointers should not be pointing to the same address or according to the function description it should not. [/quote]After you print the pointer values, modify scaledImg1 using setPixel() and print the pointer values again. You'll see what Rondog and SGaist meant.[/quote]

Thanks for your reply. Indeed, after using setPixel on the scaledImg1 the data pointer is changed.

Anyway, the initial stated problem was that the description of the scaled function is somehow misleading by the fact that I was expecting "a copy" of the image and not the same image. I just wanted to argue that maybe in the future the description gets updated so other people wont face the same problem.

[JKSH]

You're welcome.

[quote author="hpaulh" date="1423729910"]The problem is when you have a code like this:

@QImage image(...);
QImage scaledImage = image.scaled(QSize(same as the image));
someVector.push_back(scaledImage);@

When image goes out of scope, the scaledImage on the vector gets deleted too because it's the same (or at least the data pointer).[/quote]No it won't.

Qt value classes use reference counting. When image goes out of scope, the destructor checks the reference counter. It sees that another copy (scaledImage) is using that same vector, so it won't delete the vector.

[quote author="hpaulh" date="1423730259"]Anyway, the initial stated problem was that the description of the scaled function is somehow misleading by the fact that I was expecting "a copy" of the image and not the same image. I just wanted to argue that maybe in the future the description gets updated so other people wont face the same problem.[/quote]But there IS a copy!

@
// Like you said...
testImg.data_ptr() == scaledImg1.data_ptr();

// But...
&testImg != &scaledImg1;
@

scaledImg1 is a copy of testImg, even though it shares the same data as testImg.

This type of copying is called shallow copying: Multiple copies share the same block of data memory. The opposite is deep copying, where each copy is allocated its own unique data memory.

This is documented at http://doc.qt.io/qt-5/implicit-sharing.html.

[hpaulh]

Thank you all for making lots of things more clear now!