486936 - canvas should take account of image-rendering

Status: RESOLVED FIXED

(Core :: Graphics: Canvas2D, defect)

Description

[Robert Longson [:longsonr]]

See bug 423756 comment 88

Comment 1

[Robert Longson [:longsonr]]

Attachment: patch?

Hopefully the right approach, compiles but untested as I don't really know much about canvas.

Comment 2

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

Hmm.. that's not quite right; there are two pieces here, the filter applied to rendering the canvas itself when it's being CSS scaled, and whether the images rendered to the canvas are rendered with smoothing.  The latter should probably have an 'auto' mode which will honor the image's style (if there is one?  there may well not be a frame for an image that's to be drawn, I guess we'd just use the default then).  I can take a look at fixing up the patch in the next few days.

Comment 3

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

Actually I think Robert's patch is fine as far as it goes. It makes the rendering of the canvas onto a destination surface honour the canvas's image-rendering property. Rendering images or videos into the canvas using drawImage honors imageSmoothingEnabled. That's all OK right?

The only thing that's missing, possibly, is the 'auto' value you suggest to honor an image's style. I guess that could be useful but it would make sense to be a separate patch. (I guess in principle it would be logical for image formats themselves to specify how the image data should be resampled, but I guess that's not possible except for SVG images, so never mind.)

Comment 4

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

Comment on attachment 373520 [details] [diff] [review]
patch?

I encountered someone who actually needs this!

Comment 5

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

Comment on attachment 373520 [details] [diff] [review]
patch?

Ok; I think this is fine, it'd be nice if we could avoid passing the filter down to ThebesSufaceFromElement since in theory that should always be grabbing a 1:1 surface... but there may be cases where that's not true, or if we ever support SVG images, etc.

Comment 6

[Robert Longson [:longsonr]]

Attachment: update to tip

Comment 7

[Robert Longson [:longsonr]]

try server builds are here https://build.mozilla.org/tryserver-builds/[e-mail]-1242429208/

roc can the someone who needs this confirm that it does what he/she needs?

Comment 8

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

You've tested it, right?

Comment 9

[Robert Longson [:longsonr]]

That's my problem. I don't really know much about canvas and how to test it. It compiles and passes the existing reftests.

Comment 10

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

If you take an existing canvas demo (e.g. https://bug491595.bugzilla.mozilla.org/attachment.cgi?id=375888 ), set image-rendering:-moz-crispEdges and then zoom in, it should look blocky, not bilinear filtered.

Comment 11

[Robert Longson [:longsonr]]

(In reply to comment #10)
That works although it's -moz-crisp-edges

Comment 12

[Robert Longson [:longsonr]]

Attachment: updated patch with reftest

Comment 13

[Robert Longson [:longsonr]]

Attachment: updated

Comment 14

[Robert Longson [:longsonr]]

checked in http://hg.mozilla.org/mozilla-central/rev/a927df82dfcd and http://hg.mozilla.org/mozilla-central/rev/5b40dab2d442

Comment 15

[Robert Longson [:longsonr]]

Just needs the existing dev doc for image-rendering tweaking to remove the reference to this bug.

Comment 16

[j.j.]

(In reply to comment #15)

Done.