436932 - Add mozImageSmoothingEnabled property to canvas

Status: RESOLVED FIXED

(Core :: Graphics: Canvas2D, defect)

Description

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

This is modeled on the SVG image-rendering property, at http://www.w3.org/TR/SVG/painting.html#ImageRenderingProperty:

 attribute string imageRenderingQuality;

'auto' (default): The user agent shall make appropriate tradeoffs to balance speed and quality, but quality shall be given more importance than speed.

'optimizeQuality': Emphasize quality over rendering speed.

'optimizeSpeed': Emphasize speed over rendering quality.

No specific image sampling algorithm is specified for any of these properties, with the exception that, at a minimum, nearest-neighbour resampling should be used.  One alternative is to specify 'best', 'good', 'fast', with "good" being the default, as opposed to the SVG names; I think those names are more descriptive, but there might be value in keeping the names consistent with SVG, especially if that property bubbles up into general CSS usage.

Comment 1

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Attachment: add imageRenderingQuality

Add imageRenderingQuality to the canvas 2d context, and have 'optimizeSpeed' translate into CAIRO_FILTER_FAST.

Comment 2

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Should this be mozImageRenderingQuality?

@@ -2728,6 +2744,12 @@ nsCanvasRenderingContext2D::CreateImageD
     rv = ncc->GetJSContext(&ctx);
     NS_ENSURE_SUCCESS(rv, rv);
 
+    PRUint32 argc;
+    jsval *argv = nsnull;
+
+    ncc->GetArgc(&argc);
+    ncc->GetArgvPtr(&argv);
+
     JSAutoRequest ar(ctx);
 
     int32 w, h;

I presume this hunk was not intended here.

Comment 3

[Doron Rosenberg (IBM)]

Any chance this could go in for the next release of firefox?  I've ran into an IBM issue where we were resizing images in the canvas and needed the resizing to not try to smooth the image (it was making some images with text in them less readable for example).

Comment 4

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to comment #2)
> Should this be mozImageRenderingQuality?

I think vlad was trying to get this added to the spec

Comment 5

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Doron, this is not intended to guarantee that "low" or "optimizeSpeed" gives nearest-neighbour sampling, so I would not recommend using it for your case.

After the WHATWG discussion about this, I'm kind of down on the idea. I'd much prefer to have compositor and do automatic quality reduction when necessary to sustain the desired frame rate.

Comment 6

[Doron Rosenberg (IBM)]

So would it be worth filing a bug about turning off image resize "smoothing" or is that something that probably won't go in?

Comment 7

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Maybe you want to use the imagedata API, load your image into a canvas and then manually upscale by drawing one rect for each pixel.

Comment 8

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

(In reply to comment #7)
> Maybe you want to use the imagedata API, load your image into a canvas and then
> manually upscale by drawing one rect for each pixel.

This is probably a bad idea -- it ensures that you'll get horribly slow performance if you ever need to do quick animation.  But yes, the intent isn't to force nearest-neighbour (though perhaps a way to do that should be added, since I think an interpolation on/off switch is generally useful).

I guess I need to catch up on the whatwg discussion; I think that this API makes sense, because even in the presence of a compositor that can take all things into account, there is value in the author being able to say "I'm ok with a lower quality rendering here, just make it fast", or conversely, "I want this to be the highest quality possible, regardless of the time it takes".  We should not take that control away from the author.

Comment 9

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 323966 [details] [diff] [review]
add imageRenderingQuality

I don't think the imagedata part of your test is appropriate, since as we discussed, there's no reason optimizeSpeed should necessarily disable sampling. We need something else for that.

Other than that, this is OK, but I think I prefer Philip's suggestion on the list to just support "auto" and "high" values. And I don't think we should ship this without slightly more WHATWG consensus than we've got.

Comment 10

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

So maybe we don't do imageRenderingQuality, but instead just have a boolean imageSmoothingEnabled property?  There are valid reasons to disable image smoothing for visual effects anyway, so that seems like it would be simpler.

Comment 11

[Mark Finkle (:mfinkle) (use needinfo?)]

We'd love to get some kind of filter property to support zooming in Fennec. Currently, scaling with drawWindow takes about 220ms per call on an N8x0. We want to animate the zoom, so we make several calls over a short time period. These scaled images should be "optimizeSpeed" and the final scaled image can be "optimizeQuality"

Comment 12

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I don't think imageSmoothingEnabled directly addresses the use cases that inspired this bug, but I certainly think it's a reasonable thing to add. Call it mozImageSmoothingEnabled until we get some more WHATWG feedback.

Comment 13

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Well I guess it does "directly address" them, but it's a less appropriate tool for the job than comment #0 or comment #9.

Comment 14

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

(In reply to comment #13)
> Well I guess it does "directly address" them, but it's a less appropriate tool
> for the job than comment #0 or comment #9.

Yeah; the original job was really "have a way for fennec to specify that it wants fast image scaling"; I wanted to do that without hardcoding nearest-neighbour.  The Quality idea turned into a bit of a quagmire, so we should just add it directly.

Comment 15

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Attachment: add mozImageSmoothingEnabled

Simpler variant to get us to the same end state as the other approach to this bug.  Add a boolean to indicate whether image smoothing/filtering shuold be enabled or not.  If it's not enabled, nearest neighbour sampling is used.  Enabled by default to match current state.

Comment 16

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 341173 [details] [diff] [review]
add mozImageSmoothingEnabled

+        nsCanvasRenderingContext2D::ImageRenderingQuality imageRenderingQuality;

We don't want this, nor the definition of ImageRenderingQuality.

+    retVal = ((CurrentState().imageSmoothingEnabled)) == PR_TRUE;

Just "retVal = CurrentState().imageSmoothingEnabled"

+        PRBool imageSmoothingEnabled;

PRPackedBool

Comment 17

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Attachment: add mozImageSmoothingEnabled, v2

Forgot to qrefresh.

Comment 18

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Attachment: add mozImageSmoothingEnabled, v3

Now with qrefresh and review comments addressed!

Comment 19

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Checked in.

Comment 20

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Reopening -- this was failing on Mac, likely the a bug in the quartz surface not passing the filter down correctly.  Argh. :(

Comment 21

[Mark Finkle (:mfinkle) (use needinfo?)]

Vlad - any time to look at this?

Comment 22

[Takeshi Kurosawa :takenspc]

(In reply to comment #20)
> Reopening -- this was failing on Mac, likely the a bug in the quartz surface
> not passing the filter down correctly.  Argh. :(

cairo-quartz doesn't treat |CAIRO_FILTER_FAST| as |CAIRO_FILTER_NEAREST|.
http://mxr.mozilla.org/mozilla-central/source/gfx/cairo/cairo/src/cairo-quartz-surface.c#446

CAIRO_FILTER_FAST -> kCGInterpolationLow
CAIRO_FILTER_NEAREST -> kCGInterpolationNone

Should we use |CAIRO_FILTER_NEAREST| (ie. 3) for this?
# Actually I don't have any mac boxes so I can't test it :(

Comment 23

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Ah, that's probably it.  I should probably change that in the quartz surface as well.  Will fix and test, still have time to get this in.

Comment 24

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

http://hg.mozilla.org/mozilla-central/rev/cfd5af38abc6 -- FAST/NEAREST was indeed the problem on OSX.

Comment 25

[Eric Shepherd [:sheppy]]

Documented:

https://developer.mozilla.org/en/Canvas_tutorial/Using_images#Controlling_image_scaling_behavior