1360415 - imageSmoothingEnabled, when false, turns off smoothing when scaling an image down

Status: RESOLVED FIXED

(Core :: Graphics: Canvas2D, enhancement, P5)

Description

[:dhouse]

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/

Comment 1

[Nicolas Silva [:nical]]

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?

Comment 2

[Nicolas Silva [:nical]]

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.

Comment 3

[Milan Sreckovic [:milan] (needinfo for best results)]

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

Comment 4

[Boris Zbarsky [:bzbarsky]]

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.

Comment 5

[Nicolas Silva [:nical]]

Attachment: Smooth when down-scaling in canvas.DrawImage

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.

Comment 6

[Nicolas Silva [:nical]]

> 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.

Comment 7

[Bas Schouten (:bas.schouten)]

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.

Comment 8

[Pulsebot]

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

Comment 9

[Ryan VanderMeulen [:RyanVM]]

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

Comment 10

[Chris Mills [:cmills]]

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!

Comment 11

[Nicolas Silva [:nical]]

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).

Comment 12

[:dhouse]

Thank you for making this change!

Comment 13

[Boris Zbarsky [:bzbarsky]]

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

I edited the wiki accordingly.