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? > 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
