[Development] Image and PreserveAspectCrop

Gunnar Sletta gunnar at sletta.org
Tue Aug 2 13:56:03 CEST 2016


Hi,

I think the most sensible solution is to handle this inside the image provder. When I've seen this problem previously, it has was solved using a custom image provider for local files, but as you say, when the resources are scattered across the local and network alike, then you want the default internals to handle it correctly.

So I agree with you that "Solution" is the way forward, rather than A or B :) 

I suspect the behavioural change won't be that big a deal, as it will in fact show up only as a slightly sharper image.

A bit unfortunate that we have to add a V2 version of the image provider, though. Would be nice if we could extend what is already there somehow. However you end up solving it, it needs to be a bit more flexible than what you do in the proposed patch, as you also have to cover PreserveAspectFit. Perhaps using some flags as the last parameter there instead of a bool so we keep it open to add other states later on without having to introduce a new class. 

cheers,
Gunnar


> On 02 Aug 2016, at 09:11, Albert Astals Cid <albert.astals at canonical.com> wrote:
> 
> Ping anyone?
> 
> On Mon, Jul 18, 2016 at 10:24 AM, Albert Astals Cid
> <albert.astals at canonical.com> wrote:
>> There is a problem when trying to optimally[*] show an Image with
>> PreserveAspectCrop fillMode.
>> [*]optimally => as best looking as possible while using as litte
>> memory as possible
>> 
>> You can see that problem in the screenshot at http://i.imgur.com/LSSlFEB.png
>> that corresponds with the code at http://paste.ubuntu.com/19480453/
>> 
>> As you can see when displaying a landscape (width > height) image in a
>> square Image Item the optimal way
>> is to set the source size height only, but if the image is portrait
>> (height > width) then the optimal way
>> is to set the source size width only.
>> 
>> The requirement my program has is to have the best rendering quality
>> and memory usage for Image Items using PreserveAspectCrop.
>> Image sources are totally arbitrary, they can be from disk, from the
>> internet or even from QQuickImageProviders
>> (since we are plugin based and plugins can bring their own QQuickImageProvider)
>> 
>> This can be fixed in several ways.
>> 
>> Workaround A
>> **********
>> Changing the Image Item source size comparing the aspect ratio of the
>> image file with the one Image Item.
>> 
>> You can see an implementation of that workaround at
>> http://bazaar.launchpad.net/~unity-team/unity8/trunk/view/head:/plugins/Dash/CroppedImageMinimumSourceSize.qml
>> 
>> The problem with this workaround is that half of the times you end up
>> loading the image a second time.
>> This means extra CPU and potentially network usage.
>> 
>> 
>> 
>> Workaround B
>> **********
>> Implementing your own image provider that does compare the aspect
>> ratios before loading the image.
>> 
>> You can see a partial implementation of this workaround at
>> https://code.launchpad.net/~aacid/unity8/croppedImageMinimumSourceSizeProvider/+merge/300176
>> 
>> There are two problems with this workaround:
>> * You end up implementing quite a bit of duplicated functionality
>> from qquickpixmapcache.cpp
>> * For the chained image providers (i.e. the original source was an
>> image provider url) you
>>   still have to query the image provider twice half of the times
>> 
>> 
>> 
>> Solution
>> ********
>> Implementing the change in QtQuick internals so that when
>> PreserveAspectCrop fillMode is used
>> together with a sourceSize that has both width and height it does
>> return the optimal image
>> 
>> You can see a work in progress implementation of this solution at
>> https://codereview.qt-project.org/#/c/165299/
>> And how the previews could would look at
>> http://i.imgur.com/NRoXNzy.png (notice how the last column now is good
>> in both cases)
>> 
>> There are two issues with this solution:
>> * It's a small behaviour change (but in my opinion for the better)
>> * Needs new api for the QQuickImageProvider to be able to implement
>> it, so we either need the proposed
>>   QQuickImageProviderV2 or with a new "bool
>> shouldPreserveAspectRatioCrop(url, requestSize)" getter in the
>>   existing QQuickImageProvider API
>> 
>> 
>> 
>> All in all I think the solution i propose for QtQuick is acceptable
>> but i would like some agreeing that is fine adding new API before
>> finishing the patch.




More information about the Development mailing list