Closed Bug 1360415 Opened 5 years ago Closed 4 years ago
Smoothing Enabled, when false, turns off smoothing when scaling an image down
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/
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?
Priority: -- → P5
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.
Boris has been known to have opinions on the canvas spec. +1 on making it linear though.
Flags: needinfo?(milan) → needinfo?(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.
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.
Assignee: nobody → nical.bugzilla
Attachment #8886157 - Flags: review?(bas)
> 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 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.
Attachment #8886157 - Flags: review?(bas) → review+
Pushed by email@example.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/5015dd655132 Enable smoothing in canvas.drawImage when down-scaling, even with imageSmoothingEnabled=false. r=bzbarsky
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!
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).
Thank you for making this change!
> I think that it's worth being specific in the doc I edited the wiki accordingly.
You need to log in before you can comment on or make changes to this bug.