[widget audit] ImageView

[mhsmith]

Audit of Image and ImageView.

Builds on #1964.

Removes the ability to set an image by URL.

Fixes #1923
Fixes #1795
Fixes #1745
Fixes #1665

Audit checklist

• Core tests ported to pytest
• Core tests at 100% coverage
• Core docstrings complete, and in Sphinx format
• Documentation complete and consistently formatted
• cocoa backend at 100% coverage
• winforms backend at 100% coverage
• gtk backend at 100% coverage
• iOS backend at 100% coverage
• android backend at 100% coverage
• Widget support table updated
• Widget Issue backlog reviewed

[mhsmith]

Setting image = None does not currently remove an existing image: this will have to be fixed.

[mhsmith]

This also needs to describe what will happen if the ImageView is constrained by its container, or resized by its style. There are a few existing issues related to this, e.g.:

• ImageView causes resize to act weird #1795
• Image View in Box #1923

Currently the only reliable workaround is to set both width and height in the ImageView's style. When you do that with an <img> in HTML, the image is resized to those exact dimensions, disregarding its aspect ratio. So we should probably do the same, and if we need to make this configurable in future, we can add support for the object-fit attribute.

However, a more common use case is probably to set the width OR the height, and let the other one be determined by the image's aspect ratio. CSS supports this, but I guess Pack would just set the given dimension, and leave the other one at its instrinsic size.

This is not ideal, but it's at least easy for the programmer to understand. So my suggestion is:

• Add read-only width and height properties to the Image class, to enable the programmer to calculate the aspect ratio and do this kind of scaling manually.
• Create an issue to remind us to implement the CSS behavior in the future.

I've only mentioned width and height here, but we should also consider what would happen with flex.

[freakboy3742]

It turns out supporting aspect-ratio preservation in the width-only and height-only case actually isn't that hard (at least, on Cocoa and iOS... I'm sure GTK will make the regret those words shortly). We have access to both the image size and the style in rehint(); so we can set the intrinsic size to something that is derived from those properties.

We don't strictly need to expose public width/height attributes on Image - the ImageView can access the native image object associated with the native image view - but it also doesn't hurt to expose them either. Regardless, they're generally useful attributes with trivial implementations, so we might as well expose them.

As for flex: I think the most compliant thing we can do is:

• set the intrinsic size of the the image to be at_least(0) in the flow axis (so - height if it's a column box, width on a row box).
• set the intrinsic size of the image in the cross axis to be at_least() the explicitly provided size, or the image native size if no explicit size is provided.
• Modify the image rendering to preserve aspect ratio unless both height and width are specified.

AFAICT, this matches HTML in all cases except the case where only flex=1 is defined. In that case, I think HTML tries to use the size of the container to present an aspect-ratio preserving size - but we don't have access to the size of the container, so we'll have to live with (and presumably document) the discrepancy.

[mhsmith]

Suggested change

	Raises ``FileNotFoundError`` if the a path is provided, but that path does not
	Raises ``FileNotFoundError`` if a path is provided, but that path does not

[mhsmith]

Suggested change

	def save(self, path):
	def save(self, path: str | Path):

[mhsmith]

I've added some more scaling tests to the example app, and reduced the size of the image file to allow one of the widgets to use the intrinsic size. This reveals a few problems on Cocoa (I haven't checked the other platforms):

• The minimum size of the window cuts off the bottom of the Pillow image.
• The second-bottom Brutus image, with flex and no width, is able to expand but not contract, which contradicts the documentation. I'm not sure whether we should change the documentation or the implementation here: we should check what the CSS mapping would do in this situation, and possibly change that too.
• I don't understand what's going on with the spacing around the bottom Brutus image, which has both flex and width. The widget has a minimum height which is taller than the image itself, but somewhat less than the other flex widget above. Then as I increase the window size, it first increases the spacing around the bottom image until the bottom two widgets are the same height, and then increases them both equally.

[freakboy3742]

• The minimum size of the window cuts off the bottom of the Pillow image.

This is a combination of a bug that I was peripherally aware of, combining with a bug highlighted by item 3 on your list.

The bug is Cocoa doesn't re-evaluate the minimum size after layout changes. The initial minimum layout is evaluated as 458x556; but once there's a 640x556 window, the extra width alters the layout (that's the bug caused by item 3). This evaluates a layout of 640x578, but that doesn't alter the min size (and thus the size of the window itself). This shouldn't be a problem, as there's no actual layout change going on - but it's a bug worth noting, as adding a widget to an existing layout should alter the minimum window size. I'm inclined to log this as separate issue, which we should dig into when we get to testing Window.

• The second-bottom Brutus image, with flex and no width, is able to expand but not contract, which contradicts the documentation. I'm not sure whether we should change the documentation or the implementation here: we should check what the CSS mapping would do in this situation, and possibly change that too.

Based on my experimentation, I think the docs are wrong in this case; reproducing the imageview example, a flex=1, no width, no height image render with a width of at least the height of the image.

Aspect ratio is another matter... we definitely need to update the docs here to add object-fit: contain to images if the image does not define both width and height.

• I don't understand what's going on with the spacing around the bottom Brutus image, which has both flex and width. The widget has a minimum height which is taller than the image itself, but somewhat less than the other flex widget above. Then as I increase the window size, it first increases the spacing around the bottom image until the bottom two widgets are the same height, and then increases them both equally.

This turns out to be a bug in pack.

A minimum layout size is computed, and applied to the window. This then generates the "real" layout pass that alters positions of widgets; subtracting padding, there is leaves 620x536 available for layout. The images with fixed (either explicit or implicit) heights consume 365px of height, leaving 171px. The last 2 Brutus images are both flex=1, so they get an even distribution of the remaining space - 85 px each. This means the last brutus image expands from 150x43 to 150x85 (with padding above and below to fill the space, so as to preserve aspect ratio).

The problem is that the second last brutus has a minimum height of 128px, which is more than the flex allocation. With the padding applies to the last brutus, the new layout is 43px taller than the original minimum size; which means the layout no longer fits into the originally computed minimum.

The fix is twofold - firstly, the intrinsic size of flexible widgets needs to be subtracted from the available space when laying out a column; and then, when flexible space is allocated, flexible space is only allocated to widgets that haven't exceeded the flexible allocation.

[mhsmith]

Looks good to me, but I can't merge the PR because I was the one who created it.