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

Pipeline chain order not respected when rotate is used after resize with fit mode "inside" #3756

Closed
3 tasks done
jozsefsallai opened this issue Aug 8, 2023 · 3 comments
Closed
3 tasks done

Comments

@jozsefsallai
Copy link

Possible bug

Is this a possible bug in a feature of sharp, unrelated to installation?

  • Running npm install sharp completes without error.
  • Running node -e "require('sharp')" completes without error.

If you cannot confirm both of these, please open an installation issue instead.

Are you using the latest version of sharp?

  • I am using the latest version of sharp as reported by npm view sharp dist-tags.latest.

If you cannot confirm this, please upgrade to the latest version and try again before opening an issue.

If you are using another package which depends on a version of sharp that is not the latest, please open an issue against that package instead.

What is the output of running npx envinfo --binaries --system --npmPackages=sharp --npmGlobalPackages=sharp?

  System:
    OS: Linux 5.15 Arch Linux
    CPU: (12) x64 AMD Ryzen 5 5600H with Radeon Graphics
    Memory: 13.00 GB / 15.48 GB
    Container: Yes
    Shell: 5.9 - /usr/bin/zsh
  Binaries:
    Node: 18.16.0 - ~/.asdf/installs/nodejs/18.16.0/bin/node
    Yarn: 1.22.19 - ~/.asdf/installs/nodejs/18.16.0/bin/yarn
    npm: 9.5.1 - ~/.asdf/plugins/nodejs/shims/npm
  npmPackages:
    sharp: ^0.32.4 => 0.32.4

What are the steps to reproduce?

Creating an image manipulation pipeline which first resizes the image to a set width of 400 pixels and then rotates it by 90 degrees. The example code in the documentation works correctly, but in this scenario, the fit property of the resize pipeline is set to fill. However, in my use case, this property needs to be set to inside.

What is the expected behaviour?

rotate -> resize should work consistently across both fill and inside fit modes and output images that have a height of 400 pixels. With the fit: "inside" property, the image does not have a height of 400 pixels after rotating it by 90 degrees. Instead, it retains the original image dimensions that were resulted by the resize operation. In other words, the order of the chained operations doesn't matter if the fit property is set to "inside".

I am not entirely sure if this is a bug or intended behavior, however, I would have expected it to work sequentially in all cases. I work with streams so I would prefer to avoid having to create new sharp instances and work around my setup just to get around this.

Please provide a minimal, standalone code sample, without other dependencies, that demonstrates this problem

Minimal reproduction repo with unit tests to exemplify: https://github.com/jozsefsallai/sharp-resize-rotate-inside-repro

Relevant code:

function runPipeline(inStream, outStream, pipeline) {
  const stream = inStream.pipe(pipeline).pipe(outStream);

  return new Promise((resolve, reject) => {
    stream.on("finish", resolve);
    stream.on("error", reject);
  });
}

async function rotateThenResizeFill(inStream, outStream) {
  // WORKS AS EXPECTED - produces an image that's rotated and has 400px width

  const pipeline = sharp().rotate(90).resize({ width: 400, fit: "fill" });
  return runPipeline(inStream, outStream, pipeline);
}

async function resizeThenRotateFill(inStream, outStream) {
  // WORKS AS EXPECTED - produces an image that's rotated and has 400px height

  const pipeline = sharp().resize({ width: 400, fit: "fill" }).rotate(90);
  return runPipeline(inStream, outStream, pipeline);
}

async function rotateThenResizeInside(inStream, outStream) {
  // WORKS AS EXPECTED - produces an image that's rotated and has 400px width

  const pipeline = sharp().rotate(90).resize({ width: 400, fit: "inside" });
  return runPipeline(inStream, outStream, pipeline);
}

async function resizeThenRotateInside(inStream, outStream) {
  // DOES NOT WORK AS EXPECTED - produces an image that's rotated and has 400px
  // width instead of height, the output is identical to rotateThenResizeInside

  const pipeline = sharp().resize({ width: 400, fit: "inside" }).rotate(90);
  return runPipeline(inStream, outStream, pipeline);
}

Please provide sample image(s) that help explain this problem

@lovell
Copy link
Owner

lovell commented Aug 14, 2023

Thanks for reporting, commit 5c19f6d fixes this and adds a test that would previously have failed. This will be included in v0.32.5.

@kleisauke
Copy link
Contributor

I've opened PR #3762 to simplify this logic. I think the previous logic still dates back prior commit eacb833 where auto-orient vs manual orient was not separated.

@lovell
Copy link
Owner

lovell commented Aug 15, 2023

v0.32.5 now available, thanks both.

@lovell lovell closed this as completed Aug 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants