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

What is the range of values allowed for sharpen? #3427

Closed
Camuvingian opened this issue Nov 3, 2022 · 3 comments
Closed

What is the range of values allowed for sharpen? #3427

Camuvingian opened this issue Nov 3, 2022 · 3 comments

Comments

@Camuvingian
Copy link

Camuvingian commented Nov 3, 2022

Question about an existing feature

What are you trying to achieve?

What is the range of values allowed for sharpen?

options.m2 [number ](https://developer.mozilla.org/docs/Web/JavaScript/Reference/Global_Objects/Number) the level of sharpening to apply to "jagged" areas. (optional, default 2.0)

When you searched for similar issues, what did you find that might be related?

I cannot find anywhere where this is clearly defined. 0 - 10,000 seems to be in the codebase, but this does not seem right from the example usage in the unit tests.

function sharpen (options, flat, jagged) {
  if (!is.defined(options)) {
    // No arguments: default to mild sharpen
    this.options.sharpenSigma = -1;
  } else if (is.bool(options)) {
    // Deprecated boolean argument: apply mild sharpen?
    this.options.sharpenSigma = options ? -1 : 0;
  } else if (is.number(options) && is.inRange(options, 0.01, 10000)) {
    // Deprecated numeric argument: specific sigma
    this.options.sharpenSigma = options;
    // Deprecated control over flat areas
    if (is.defined(flat)) {
      if (is.number(flat) && is.inRange(flat, 0, 10000)) {
        this.options.sharpenM1 = flat;
      } else {
        throw is.invalidParameterError('flat', 'number between 0 and 10000', flat);
      }
    }
    // Deprecated control over jagged areas
    if (is.defined(jagged)) {
      if (is.number(jagged) && is.inRange(jagged, 0, 10000)) {
        this.options.sharpenM2 = jagged;
      } else {
        throw is.invalidParameterError('jagged', 'number between 0 and 10000', jagged);
      }
    }
  } 

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

Unit test use very low non-integer numbers:

it('specific radius 3 (sigma 1.5) and levels 0.5, 2.5', function (done) {
    sharp(fixtures.inputJpg)
      .resize(320, 240)
      .sharpen(1.5, 0.5, 2.5)
      .toBuffer(function (err, data, info) {
        if (err) throw err;
        assert.strictEqual('jpeg', info.format);
        assert.strictEqual(320, info.width);
        assert.strictEqual(240, info.height);
        fixtures.assertSimilar(fixtures.expected('sharpen-3-0.5-2.5.jpg'), data, done);
      });
  });

what value range should I be providing in my use case of:

promises.push(
	new Promise((res, err) => {
		image
			.sharpen(undefined, 1, this.sharpenLevel)
			.png({ quality: this.cubeMapQuality })
			.toBuffer((err, buffer, info) => {
				res(buffer);
			});
	})
);

currently I accept a sharpenLevel of 0 - 100, is this right?

@lovell
Copy link
Owner

lovell commented Nov 3, 2022

libvips allows a range from zero to one million for these.

https://github.com/libvips/libvips/blob/17ca29adc9c52ef48cf5a9a59c26a83d0693fbbe/libvips/convolution/sharpen.c#L335-L368

Does your scenario require the existing, equally-arbitrary 10000 limit in sharp to be increased to match that of libvips?

I recommend the use of named parameters when calling sharpen(), i.e. provide explicit m1 and m2 properties as part of a single options Object. (The unit tests quoted here exists to test a now-deprecated approach.)

@lovell
Copy link
Owner

lovell commented Dec 7, 2022

Commit 2a71f18 ensures sharp allows the full range of values as accepted by libvips, plus enhances the docs in this regard a little too.

@lovell
Copy link
Owner

lovell commented Dec 21, 2022

v0.31.3 now available, thanks for the suggestion.

@lovell lovell closed this as completed Dec 21, 2022
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

2 participants