Let width and height fill in a resize the type created - #72
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f752223278
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // which is what asking for a resizing type without a size means. | ||
| if parsed | ||
| .resize | ||
| .is_some_and(|resize| resize.width == 0 && resize.height == 0) |
There was a problem hiding this comment.
When client hints are enabled and the URL specifies resizing_type:fill or resize:fill without dimensions, this cleanup runs before apply_client_hints (src/service/mod.rs lines 140-149) and removes the explicit type. A Width header then takes the None branch in src/negotiation.rs lines 136-143 and constructs a Fit resize, so the request silently uses fit instead of the requested fill; previously the hint supplied the missing width on the retained Fill. Defer this cleanup until after client hints or preserve the requested type when constructing the hinted resize.
Useful? React with 👍 / 👎.
| sudo apt-get install -y build-essential pkg-config libvips-dev libvips openssl ca-certificates | ||
| # Debian ships libheif's codecs separately; without these, AVIF and HEIF | ||
| # output is reported as unsupported rather than encoded. | ||
| sudo apt-get install -y libheif-plugin-aomenc libheif-plugin-x265 libheif-plugin-dav1d libheif-plugin-libde265 |
There was a problem hiding this comment.
On Ubuntu 22.04 and Debian 12—both compatible with the documented libvips 8.12+ minimum—these split libheif-plugin-* packages are unavailable because those releases bundle the codecs differently, so this unconditional apt-get install exits nonzero and prevents the documented native installation. Limit this command to releases such as Ubuntu 24.04/Debian trixie that provide the split packages, or document release-appropriate package sets.
Useful? React with 👍 / 👎.
f752223 to
a3231f2
Compare
|
@codex review |
|
Codex Review: Didn't find any major issues. Chef's kiss. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
a3231f2 to
4ffd0b2
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4ffd0b2940
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| | ---------------- | ------- | ----------------------------------------------------------------------------------------------------------- | | ||
| | Rust toolchain | 1.90 | Install via [rustup](https://rustup.rs/). Ensure `cargo`, `rustc`, and `rustfmt` are on your `PATH`. | | ||
| | libvips | 8.12+ | Provides the core image processing primitives. Both development headers and runtime libraries are required. | | ||
| | libvips | 8.12+ | Provides the core image processing primitives. Both development headers and runtime libraries are required. 8.16+ is recommended and is what the published image ships: AVIF and HEIF encoding and `preserve_hdr`'s gain map all depend on it. Older versions work, losing those. | |
There was a problem hiding this comment.
For native installations using libvips 8.12–8.15 with the appropriate AV1 or HEVC encoder, this incorrectly says AVIF and HEIF encoding are unavailable: CHANGELOG.md lines 257–270 records that the 8.15 save-path problem was fixed and that AVIF is unaffected by missing HEVC support, while doc/5_processing_options.md lines 239–247 correctly makes codec availability build-dependent and reserves the 8.16 requirement for gain-map retention. This may cause users who need AVIF/HEIF output to perform an unnecessary libvips upgrade; limit the 8.16 statement to preserve_hdr gain maps and describe codec support separately.
Useful? React with 👍 / 👎.
4ffd0b2 to
f109396
Compare
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep it up! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
f109396 to
d5e4f76
Compare
|
@codex review |
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
d5e4f76 to
c5d80ea
Compare
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep it up! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
c5d80ea to
00f1f83
Compare
4e922a9 to
fdbae8f
Compare
fdbae8f to
f568989
Compare
f568989 to
d9f4de8
Compare
82cd523 to
87c12f6
Compare
87c12f6 to
e6f5104
Compare
e6f5104 to
82a4f3f
Compare
82a4f3f to
2662631
Compare
317bbf0 to
d835e25
Compare
resizing_type:fill/width:300/height:200 failed with "resize requires at least one non-zero dimension". width and height were kept in their own fields and reconciled into a resize only when none existed, and resizing_type creates one as a side effect, so the reconciliation never ran. They now write into the resize directly, which is how imgproxy has always modelled them: one target, whichever directive names it. A resizing type with no dimensions describes no target, so that case now returns the image unresized instead of failing at processing time.
d835e25 to
6d0d55a
Compare
1 participant
Stacked on #71.
resizing_type:fill/width:300/height:200failed with "resize requires at least one non-zero dimension".widthandheightwere kept in their own fields and reconciled into a resize only when none existed — andresizing_typecreates one as a side effect, so the reconciliation never ran. They now write into the resize directly, which is how imgproxy has always modelled them: one target, whichever directive names it. A laterwidthoverrides an earlier one whichever form set it.Also: a resizing type with no dimensions describes no target, so
resize:fillon its own now returns the image unresized instead of failing at processing time. imgproxy does the same.Validation