Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: setting width and height to Contain is not preserving aspect ratio of tall images #6554

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

saltkid
Copy link

@saltkid saltkid commented Jan 5, 2025

fixes #3708, fixes #4407


The scaling for "Contain" uses the wrong direct proportion formula for tall images. This is due to the image's aspect being width / height. The original formula scaled_width = scaled_height / aspect expands to

$$\frac{\text{scaled\_width}}{\text{scaled\_height}} = \frac{\text{height}}{\text{width}}$$

which inversely scaled the width of tall images.

The formula is now based on two scaling factors based on the image's dimension relative to the window's. Now, only one computation is needed to cover for all scenarios of computing width instead of having to go through if else branches. Same goes for height.

$$\text{scale\_width} = \frac{\text{window\_width}}{\text{image\_width}}$$ $$\text{scale\_height} = \frac{\text{window\_height}}{\text{image\_height}}$$

These were derived from the principle of direct proportion k=x/y where k is a scaling factor, and x y are the widths of the image and window for example. The larger k is, the larger the window is relative to the image on the width side. Same goes for the height.

Contain needs the minimum scaling factor to ensure both dimensions fit within the window. Cover needs the maximum scaling factor to ensure the image's smaller dimension fits in the window, allowing the larger dimension to crop, as expected. Both of which are the behavior described in the docs.

Example

config used

local wezterm = require("wezterm")
local config = wezterm.config_builder()

config.background = {
	{
		source = {
			Color = "#161616",
		},
		height = "100%",
		width = "100%",
	},
	{
		source = {
			File = os.getenv("HOME") .. "/pictures/wt_bg/center-uniform-0.1/drip yo.jpg",
		},
		width = --[[ VARIABLE ]],
		height =  --[[ VARIABLE ]],
		vertical_align = "Top",
		horizontal_align = "Center",
		repeat_x = "NoRepeat",
		repeat_y = "NoRepeat",
		opacity = 0.2,
	},
}
config.enable_tab_bar = false

return config
  1. with both width and height set to "Contain"
old new
image image
  1. both width and height set to "Cover" (no changes)
old new
image image

art by @AZMC15

When both width and height is set to `Contain`, the scaling uses the
wrong direct proportional scaling formula for tall images. This is due
to `aspect` being `width / height`:
By dividing (original), the formula expands to `scaled_width /
scaled_height : height / width` which inversely scaled the width of tall
images. By multiplying instead, the formula correctly expands to
`scaled_width / scaled_height : width / height` which preserves aspect
ratio when scaling.
@saltkid saltkid marked this pull request as draft January 8, 2025 06:38
…", min for "Contain"

This was derived from the principle of direct proportion `x=ky` where
`k` is the scaling factor, and `x` `y` are the dimensions. Scaling
factor is based on image's dimension relative to the window's.

Contain needs the minimum scaling factor to ensure both dimensions fit
within the window. Cover needs maximum scaling factor to ensure the
image's smaller dimension fits in the window, allowing the larger
dimension to crop, as expected.
@saltkid saltkid marked this pull request as ready for review January 8, 2025 07:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant