Skip to content
This repository has been archived by the owner on May 1, 2024. It is now read-only.

[Bug] DataTemplate instances are initiated with width/height set to -1 #12697

Closed
Tommigun1980 opened this issue Oct 31, 2020 · 15 comments
Closed
Assignees
Labels
a/collectionview a/layout s/unverified New report that has yet to be verified t/bug 🐛

Comments

@Tommigun1980
Copy link

Tommigun1980 commented Oct 31, 2020

Hi.

Imagine a collection view such as:

<CollectionView
    ItemSizingStrategy="MeasureFirstItem"
    ItemsSource="{Binding SomeItemsSource}"
    ItemTemplate="{StaticResource SomeDataTemplate}" />

Now imagine a data template such as:

<DataTemplate
    x:Key="SomeDataTemplate"
    x:DataType="models:SomeModel">
    <Image
        Source="...whatever..."
        HorizontalOptions="FillAndExpand"
        VerticalOptions="Start"
        HeightRequest="{Binding Width, Source={RelativeSource Self}, Converter={StaticResource MyImageHeightAspectRatioConverter}}" />
</DataTemplate>

... ie the instance's height will be set to its "width * aspect ratio".

However, the instance of the data template will be created with a width of -1 even though the width is available in the collection that the element will be added to. So it seems that the data template instances are initiated before being 'added' to the collection view, as the converter will initially be invoked with a width of -1.


This behaviour introduces at least two issues:

  1. The collection view seems to sample the height of the elements only once when ItemSizingStrategy is "MeasureFirstItem". Because the height is assigned from width, and is initially called with -1, all the elements in the collection view will be drawn as very short elements as they will not have their heights set at the time they are sampled. The height will be assigned via the converter once a width is available, but this seems to happen after the collection view has sampled the height of the first element.

  2. It causes a refresh view to jump around when refreshed, as reported at [Bug] RefreshView jumps around when refreshing items when ItemSizingStrategy="MeasureAllItems" #12622


Workaround:
My workaround for the time being is to use the following binding instead, which fetches the width from the parent container (collection view) instead:

HeightRequest="{Binding Width, Source={RelativeSource Mode=FindAncestor, AncestorType={x:Type CollectionView}}, Converter={StaticResource MyImageHeightAspectRatioConverter}}"

Now a width will be available for calculating height at the time the collection view samples the height of the first element.


Considering that the data template instances are going to be added to the collection view, is there any reason why the data template instances are not initiated after they have been added to the view, so that they'd get proper dimensions the first time the bindings are run, and only after that sample the first element's height?

In other words, would it be possible to create the instances and add them to the collection before initialising them? Or change the initialisation order to whatever is required in order to avoid this issue?

Thank you.

@rmarinho
Copy link
Member

rmarinho commented Nov 2, 2020

I don't see how that's possible... the instances will always have Width and Height to -1

@Tommigun1980
Copy link
Author

Tommigun1980 commented Nov 2, 2020

I don't see how that's possible... the instances will always have Width and Height to -1

Hi. What do you mean?
I am able to set proper dimensions by using the relative binding to its parent instead of itself - shouldn't this be known at the time when they are added based on its horizontal and vertical options, as long as its parent has dimensions set?

But if not, thank you for the answer. In that case there is still an issue with the collection view when using ItemSizingStrategy="MeasureFirstItem" -- it seems to sample the first child before it has had a chance to set a height unless the height is hard coded. It also causes a refresh view to jump around wildly when refreshing. Hence my workaround with the relative binding to its parent, to figure out its height without the -1 "step", which remedies both problems.

Thank you.

@hartez
Copy link
Contributor

hartez commented Nov 25, 2020

This doesn't have anything to do with how DataTemplates are initialized. What you're seeing is just the default initial values for the Image control before it has any actual sizing information. I'm attaching a project which demonstrates this with just an Image - no templates or CollectionViews or anything like that involved at all. Watch the console messages to see it happening.

When the Image is initialized, both the Width and Height values are -1 (because there is no sizing information). The binding of HeightRequest to Width kicks in, and the current value of -1 is passed to the converter. Later, when sizing information is available (which cannot happen until the native platform is available), the Image's Height and Width are set the the new values, and that new value for Width is passed to the converter.

So this is nothing to do with DataTemplates; it's just how the Width/Height properties work. Also, tying the HeightRequest to the Width property is probably not a good idea in the long run, at least if you're going to use the default setting for Aspect. The change in HeightRequest will cause the image to resize, which (depending on the size of the image) may cause a change in the Width and end up forcing one or more extra layout passes. You probably want Aspect.Fill if you are going to manually control the aspect ratio via a converter. And even then, tying the HeightRequest to an outside value (which can't be changed by changing the HeightRequest itself) is likely to be a safer option. Your workround of tying it to the CollectionView's width is a reasonable option (if your CollectionView is using a vertical-scrolling layout).

_12697 Repro.zip

@hartez hartez closed this as completed Nov 25, 2020
@Tommigun1980
Copy link
Author

Tommigun1980 commented Nov 26, 2020

Hi @hartez and thanks for the info.

  1. Also, tying the HeightRequest to the Width property is probably not a good idea in the long run, at least if you're going to use the default setting for Aspect.

Unfortunately what you suggest can't be done as the images are downloaded. I know their aspect ratio beforehand and need to space them in the collection beforehand (they have spinners until the images are downloaded). Just setting AspectFill would work if the images were preloaded with the app, but it's not possible to use it for dynamically downloaded images. The only way I think it's possible to do it is by setting the height as a product of their width.
What exactly is the recommended way to do this with dynamically downloaded images if explicitly setting their height (as aspect ratio times width via a converter) is not a good idea, as just using AspectFill won't cut it? All I want is for downloaded images to display correctly in a list/collection.

  1. This doesn't have anything to do with how DataTemplates are initialized. What you're seeing is just the default initial values for the Image control before it has any actual sizing information.

I understand that, and I was wondering if this could be changed so controls could be initialised to fill their parents at time 0 for elements in a list/collection, as that information is available in the hierarchy. If not then thank you for the information.

  1. The two issues I explained in this report are still valid, but iirc I have filed separate reports about them.

@programmation
Copy link

All I want is for downloaded images to display correctly in a list/collection.

Sorry to interrupt, but do you @Tommigun1980 have control of the server? If so, you could modify the data on the server so it can send image size information with the items in the list, which can be bound to the WidthRequest/HeightRequest of each item's cell image. Then each image can download separately but will arrive in a box that's already the right size for it.

@Tommigun1980
Copy link
Author

Tommigun1980 commented Nov 26, 2020

All I want is for downloaded images to display correctly in a list/collection.

Sorry to interrupt, but do you @Tommigun1980 have control of the server? If so, you could modify the data on the server so it can send image size information with the items in the list, which can be bound to the WidthRequest/HeightRequest of each item's cell image. Then each image can download separately but will arrive in a box that's already the right size for it.

Hi and thanks for the tip. I do and I know the exact dimensions of the images. The problem is to set up the images to said dimensions. Here's why knowing the exact dimensions doesn't really help and aspect ratio must be used instead:

  1. Let's say all images are 1000x1000 pixels.
  2. You set that as the height and scale to AspectFill.
  3. While the image downloads, the items in the collection are drawn as 1000 pixels tall and a spinner is displayed in all of them (via FFImageLoading's loading image property).
  4. After the images finish downloading, they now scale to their proper dimensions. So if the horizontal area is 400 pixels wide, the heights now get set to 400 pixels.
  5. This makes all the items jump around.
  6. This technique also requires one to use MeasureAllItems (instead of MeasureFirstItem). MeasureFirstItem seems to also mean "measure first item, and do it only once", as otherwise the initial size will be used even after images have finished downloading. This doesn't really matter though as I can use MeasureAllItems, just wanted to make a note of it.
  7. Another oddity is that when the 1kx1k images are drawn as 400x400, the images get cropped and not scaled properly, if they are smaller in size than the area where they get drawn. I haven't found any way of showing the full images on all screens while keeping their aspect ratios. I filed a bug report about this but was told this was by design. Personally I find it weird if there really is no way of displaying the full images, but alas, it's also not really related to this.

So I can't see any way of doing this without using an aspect ratio converter. Please clarify what you meant @hartez as it'd be tremendously useful.

@Tommigun1980
Copy link
Author

So without an aspect ratio converter here's an image while it's downloading (spinner not visible here but it'd be centred in there):
Screenshot 2020-11-26 at 6 13 43

... and when it finishes it's fine:
Screenshot 2020-11-26 at 6 12 06

Here's the loading with an aspect ratio converter:
Screenshot 2020-11-26 at 6 16 35

@programmation
Copy link

Sorry I meant to suggest scaling the actual size of the image container so it fits in the width of the collection view, rather than displaying it at the native size of the image. So if the data reports an image as 6400x3200, but your CV is only 800 wide, the width request/height request will be 800 & 400 respectively. Then your image downloads as 6400x3200 and will centre correctly within the available space.

@Tommigun1980
Copy link
Author

Tommigun1980 commented Nov 26, 2020

Sorry I meant to suggest scaling the actual size of the image container so it fits in the width of the collection view, rather than displaying it at the native size of the image. So if the data reports an image as 6400x3200, but your CV is only 800 wide, the width request/height request will be 800 & 400 respectively. Then your image downloads as 6400x3200 and will centre correctly within the available space.

Thank you. But how would it know, unless I read the dimensions from the parent container/collection in order to assign the dimensions? Which is equivalent to the aspect ratio converter, no?

@programmation
Copy link

Which is equivalent to the aspect ratio converter, no?

It may be, but I think you could do it in the ViewModel layer rather than requiring a Converter. So for example you could extend CollectionView to have a 2-way bindable property that sets its Width back into the ViewModel. This value can flow into the Cell ViewModels which provide calculated HeightRequests to match WidthRequests at the right aspect ratio for each image. You'd need an update function to update existing Cell ViewModels if the width of the CollectionView changes, as well as a way to inject the right width value into new Cell ViewModels as they are created.

@programmation
Copy link

The intention is to push width values back from the View (which sizes itself according to constraints you can't control directly) into the ViewModel (from where you can propagate the basis for a ratio calculation into ViewModels with bindable properties that can adjust the actual heights of Cells in the View).

@programmation
Copy link

And because it's done in the ViewModel, each View then binds and sizes itself correctly whether it's brand-new or recycled off the re-use stack.

@Tommigun1980
Copy link
Author

Hi. I still don't see what all that complexity would accomplish. All I want to do is display image urls in a collection.
But thanks for the suggestion, I appreciate it!

@programmation
Copy link

If I understand the problem correctly, the issue is that CV cells are resizing themselves too much because images download asynchronously after the cells have initially sized themselves. The initial size is wrong because the size of the image - or better still, its aspect ratio - isn't known until the image has arrived at the device. I agree my initial idea is too complex, so perhaps you could do something simpler, for example:
The cell data should include the aspect ratio of the image. Then each cell's image container has its HeightRequest value tied to the Width value by the aspect ratio. (Probably using your aspect ratio value converter). However the aspect ratio data for this calculation is supplied at the time the cell's View is bound to its ViewModel, rather than after the image has downloaded. This will eliminate the resize when the image becomes available, because the Cell will be created at the right vertical size initially.

@Tommigun1980
Copy link
Author

If I understand the problem correctly, the issue is that CV cells are resizing themselves too much because images download asynchronously after the cells have initially sized themselves. The initial size is wrong because the size of the image - or better still, its aspect ratio - isn't known until the image has arrived at the device. I agree my initial idea is too complex, so perhaps you could do something simpler, for example:
The cell data should include the aspect ratio of the image. Then each cell's image container has its HeightRequest value tied to the Width value by the aspect ratio. (Probably using your aspect ratio value converter). However the aspect ratio data for this calculation is supplied at the time the cell's View is bound to its ViewModel, rather than after the image has downloaded. This will eliminate the resize when the image becomes available, because the Cell will be created at the right vertical size initially.

Thank you again. That would just add so much complexity for something that should be trivial that I would rather use the converter as it's one line and done. I also think it would be wrong to manually calculate the scaled destination dimensions and let the collection adjust itself to that, as it won't cope with device rotations etc. Of course everything can be implemented but at that point I'm basically making a semi layout engine. This is such a common use case, download images and show them in a list, that I think it should just work out of the box without anything extra... which it does - it does work properly with the converter. I am mostly interested in what alternative, which doesn't involve reimplementing everything, @hartez was thinking of when he advised not to calculate the height as a product of its width.
Thanks.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
a/collectionview a/layout s/unverified New report that has yet to be verified t/bug 🐛
Projects
None yet
Development

No branches or pull requests

5 participants