:dhouse Reporter
	Description • 7 years ago

imageSmoothingEnabled, when false, turns off smoothing when scaling an image down.

As of https://github.com/whatwg/html/commit/5854cb07f075e5219df8ad645370bb2818cf5a62, the spec states that the smoothing setting applies only to scaling up a bitmap image. Because of this, we can change to a smoothing algorithm for down-scaling images.

Chrome matched this change at https://chromium.googlesource.com/chromium/src/+/master/third_party/WebKit/Source/modules/canvas2d/BaseRenderingContext2D.cpp#1089 from https://bugs.chromium.org/p/chromium/issues/detail?id=269089.

I want for Firefox's implementation to be changed also so that PDF.js can easily/simply implement the PDF spec's image interpolate setting, which also only applies to up-scaling images, with imageSmoothingEnabled and not need to check if the image is being scaled up or down: https://github.com/mozilla/pdf.js/pull/8338

Here is an example showing the issue, http://jsfiddle.net/c8mvgm0j/4/

	Nicolas Silva [:nical] Assignee
	Comment 1 • 7 years ago

This looks easy to implement. I don't know who at mozilla (if anyone) was involved in the discussions around this particular edit to the canvas spec, I haven't been able to trace back to the discussion itself looking at the github issues and the whatwg mailing list. Currently we comply to the specification by using nearest filtering when downscaling in drawImage with imageSmoothingEnabled = false, since the spec leaves it up to the user agent, but chrome decided to change to using linear filtering in that case.
It's not obvious to me what is desirable and it would help to see the original discussion that motivated loosening the spec. In chrome's bug tracker someone complained about the change. I sort of see the appeal of making things easier for pdf.js, but expect we'd need to see more detailed reasoning to warrant changing gecko's behavior.

Milan, do you know who owns canvas2d spec type things?

	Nicolas Silva [:nical] Assignee
	Comment 2 • 7 years ago

I thought about this some more and it kind of makes sense to do linear since sampling artifacts when downscaling rarely looks good. Still, it'd be good to have more context.

	Milan Sreckovic [:milan] (needinfo for best results)
	Comment 3 • 7 years ago

Boris has been known to have opinions on the canvas spec.  +1 on making it linear though.

	Boris Zbarsky [:bzbarsky]
	Comment 4 • 7 years ago

I think the idea if having this frob was to give the option of upscaling without interpolation for "pixel graphics" kind of things.  So I think it's fine to stop applying it while downscaling.

	Nicolas Silva [:nical] Assignee
	Comment 5 • 7 years ago

The change is pretty simple. r? Bas for the implementation, although I should probably wait for someone involved with the canvas spec to validate that we want the behavior change.

	Nicolas Silva [:nical] Assignee
	Comment 6 • 7 years ago

> I should probably wait for someone involved with the canvas spec to validate that we want the behavior change.

> I think the idea if having this frob was to give the option of upscaling without interpolation for "pixel graphics" kind of things.  So I think it's fine to stop applying it while downscaling.

Ah, I missed this comment. I take it as go, then.

	Bas Schouten (:bas.schouten)
	Comment 7 • 7 years ago

Comment on attachment 8886157 [details] [diff] [review]
Smooth when down-scaling in canvas.DrawImage

Review of attachment 8886157 [details] [diff] [review]:
-----------------------------------------------------------------

Complete the comment by adding that if any dimension is upscaled, we consider the image as being upscaled.

	Pulsebot
	Comment 8 • 7 years ago

Pushed by nsilva@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/5015dd655132
Enable smoothing in canvas.drawImage when down-scaling, even with imageSmoothingEnabled=false. r=bzbarsky

	Ryan VanderMeulen [:RyanVM]
	Comment 9 • 7 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/5015dd655132

	Chris Mills [:cmills]
	Comment 10 • 7 years ago

I have documented this by adding in a browser compat note:

https://developer.mozilla.org/en-US/docs/Web/API/CanvasRenderingContext2D/drawImage#Browser_compatibility

And adding a note to the Fx56 rel notes:

https://developer.mozilla.org/en-US/Firefox/Releases/56#DOM

Let me know if this looks OK, or if more is needed. Thanks!

	Nicolas Silva [:nical] Assignee
	Comment 11 • 7 years ago

I think that it's worth being specific in the doc that we are now smoothing when down-scaling with imageSmoothingEnabled=false (we were already smoothing when down-scaling if imageSmoothingEnabled=true).

	:dhouse Reporter
	Comment 12 • 7 years ago

Thank you for making this change!

	Boris Zbarsky [:bzbarsky]
	Comment 13 • 7 years ago

> I think that it's worth being specific in the doc

I edited the wiki accordingly.