> On 02 Aug 2016, at 18:03, Albert Astals Cid <albert.ast...@canonical.com> > wrote: > > On Tue, Aug 2, 2016 at 1:56 PM, Gunnar Sletta <gun...@sletta.org> wrote: >> 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. > > I don't understand what needs to be done regarding PreserveAspectFit. > Isn't that the default mode of operation? >
Nope, the default fill mode is Image.Stretch. PreserveAspectFill is similar to PreserveAspectCrop. It uses a different boundingBox to figure out the scale factor and doesn't crop, but because it scales aspect-ratio aware, it needs the same thing as PreserveAspectCrop. While we're on the subject. You do know about Image::mipmap, right? It will give you visually optimal results when displaying a larger image into a smaller area. It does come a slight memory overhead cost though, and the renderer won't be able to batch it, so it works best for single images, not for images in a grid or list. cheers, Gunnar >> 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.ast...@canonical.com> >>> wrote: >>> >>> Ping anyone? >>> >>> On Mon, Jul 18, 2016 at 10:24 AM, Albert Astals Cid >>> <albert.ast...@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. >> _______________________________________________ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
