Closed
Bug 484150
Opened 17 years ago
Closed 17 years ago
Support image-rendering attribute
Categories
(Core :: SVG, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: longsonr, Assigned: longsonr)
References
Details
Attachments
(1 file, 4 obsolete files)
|
21.85 KB,
patch
|
Details | Diff | Splinter Review |
http://www.w3.org/TR/SVG11/painting.html#ImageRenderingProperty
See also bug 436932
I think this will initially only do things on Windows. Linux has bug 422179 and according to bug 436932 Mac doesn't work.
I'll ask roc or maybe vlad for a sr to cover the SVG bit that actually does the work.
Attachment #368213 -
Flags: review?(dbaron)
Did you run the mochitests in layout/style (and did they pass)?
Attachment #368213 -
Flags: review?(dbaron) → review+
Comment on attachment 368213 [details] [diff] [review]
patch
From a CSS perspective, the code looks fine, although I'm not sure if it's really a good idea to implement this property; is this really something we want authors to control? I suppose given the 'auto' value it's probably ok.
Something like this would give authors the control they want in bug 423756; but I really don't like "optimizeSpeed" vs. "optimizeQuality" in the spec... they really don't have any kind of meaning that authors can depend on, beyond just hints that can be ignored.
| Assignee | ||
Comment 4•17 years ago
|
||
Adds two missing lines. This version passes all the tests in layout/style.
Attachment #368213 -
Attachment is obsolete: true
Attachment #368332 -
Flags: superreview?(roc)
| Assignee | ||
Comment 5•17 years ago
|
||
image-rendering really needs an additional setting, something like disableResampling that would make it more dependable. Perhaps jwatt can take that up with the SVG wg. At the moment this is as close as we can get to as a cross-browser solution.
| Assignee | ||
Comment 6•17 years ago
|
||
remove repeated redundant line from patch.
Attachment #368332 -
Attachment is obsolete: true
Attachment #368362 -
Flags: superreview?(roc)
Attachment #368332 -
Flags: superreview?(roc)
We're free to add a -moz-disable-resampling value. I'd support that.
Comment on attachment 368362 [details] [diff] [review]
updated patch
Looks good.
Attachment #368362 -
Flags: superreview?(roc) → superreview+
Actually it would be awesome if we supported this for <img> too ... and if we supported -moz-disable-resampling.
Yep, I was just thinking the same thing, we should be able to add a -moz value here. -moz-disable-filtering? or resampling? Then we can support it on <img>, <canvas> and wherever else we render image-like things.
| Assignee | ||
Comment 11•17 years ago
|
||
I'll meet you halfway. This includes the style changes for -moz-disable-resampling but it still only does SVG images.
Attachment #368362 -
Attachment is obsolete: true
Attachment #368449 -
Flags: review?(dbaron)
| Assignee | ||
Comment 12•17 years ago
|
||
Oh, and it still passes all the layout/style mochitests.
Attachment #368449 -
Flags: review?(dbaron) → review+
Comment on attachment 368449 [details] [diff] [review]
with -moz-disable-resampling
I'm not sure how -moz-disable-resampling differs from optimizeSpeed, but I'll trust roc and vlad here.
One minor comment is that you might want to leave the "MOZ_" out of the NS_STYLE_IMAGE_RENDERING_MOZ_DISABLE_RESAMPLING constant name.
r=dbaron
On a hypothetical heavily GPU-accelerated graphics backend, optimizeSpeed might do something really fancy because it's "free". disable-resampling would still be required to do nearest-neighbour sampling.
This patch should actually pass CAIRO_FILTER_NEAREST for disable-resampling, though.
| Assignee | ||
Comment 15•17 years ago
|
||
addresses review comments and includes part of patch in bug 423756 for increased readability
Attachment #368449 -
Attachment is obsolete: true
| Assignee | ||
Comment 16•17 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
| Assignee | ||
Comment 17•17 years ago
|
||
For html I can see
nsImageFrame::PaintImage
nsLayoutUtils::DrawSingleImage
...
other internal nsLayoutUtils functions
nsThebesImage::Draw
I can get the css style in nsImageFrame but I don't have a pattern till nsThebesImage::Draw.
| Assignee | ||
Comment 18•17 years ago
|
||
Oops a bit early there.
Anyway, what's the best way to get the filter from the image frame to nsThebesImage?
extra argument? not sure how many callers I'd have to change.
extra methods in nsIRenderingContext and/or nsIImage to store/retrieve the filter state?
some combination of the above?
This is really bug 423756 but there's 43 people would get spammed there.
For <img> (and CSS background images, which we should probably also handle), I think adding a parameter to nsLayoutUtils::Draw* and nsThebesImage::Draw is the best approach.
Also, could you mention this extension to www-svg?
| Assignee | ||
Comment 21•17 years ago
|
||
(In reply to comment #20)
> Also, could you mention this extension to www-svg?
I've already done that.
Blocks: 423756
| Assignee | ||
Comment 22•17 years ago
|
||
-moz-disable-resampling has become -moz-crisp-edges now that bug 423756 has landed.
Comment 23•17 years ago
|
||
Is it supposed to work on all the platforms?
I've tested on Linux, Win7 and Mac.
Sounds like it only works on Mac.
I use this test: http://people.mozilla.com/~prouget/bugs/imageRenderingBug/
Screenshots:
Mac: http://people.mozilla.com/~prouget/bugs/imageRenderingBug/mac.png
Linux: http://people.mozilla.com/~prouget/bugs/imageRenderingBug/linux.png
Win7: http://people.mozilla.com/~prouget/bugs/imageRenderingBug/win7.png
Comment 24•17 years ago
|
||
... but with the canvas specific property (mozImageSmoothingEnabled), it works well on Linux.
| Assignee | ||
Comment 25•17 years ago
|
||
Firstly, you have the wrong bug. This one is for the image-rendering attribute on SVG images, not html images. html is bug 423756. Posting to that bug will spam a lot of people though.
Secondly what Firefox version are you testing on because the earliest this is supported in is Firefox 3.6
Thirdly it does not work on Linux. See bug 423756 comment 44 for why.
| Assignee | ||
Comment 26•17 years ago
|
||
Also you are scaling the images smaller so you've hit bug 486918. You need to scale images larger than they are to see an effect.
You need to log in
before you can comment on or make changes to this bug.
Description
•