RESOLVED FIXED 100611
CSS canvas renders blurry at 2x pixel ratio
https://bugs.webkit.org/show_bug.cgi?id=100611
Summary CSS canvas renders blurry at 2x pixel ratio
Timothy Hatcher
Reported 2012-10-28 10:12:04 PDT
If you create a 2x pixel ratio CSS canvas it will render at 1x. <rdar://problem/12574189>
Attachments
Proposed Change (14.32 KB, patch)
2012-10-28 14:01 PDT, Timothy Hatcher
dino: review+
timothy: commit-queue-
Radar WebKit Bug Importer
Comment 1 2012-10-28 10:12:22 PDT
Timothy Hatcher
Comment 2 2012-10-28 14:01:59 PDT
Created attachment 171143 [details] Proposed Change
WebKit Review Bot
Comment 3 2012-10-29 00:01:44 PDT
Comment on attachment 171143 [details] Proposed Change Attachment 171143 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/14618624 New failing tests: platform/chromium/virtual/gpu/fast/canvas/canvas-as-image-hidpi.html fast/canvas/canvas-as-image-hidpi.html
Tom Hudson
Comment 4 2012-11-02 04:15:14 PDT
I think you're going to have to update ImageBufferSkia to pay attention to your new parameter; it's already got a member variable tracking its scale. Could we come up with a better name for the parameter and the enum values? I really can't tell reading the declaration or call sites what the new code is intended to do, or why you need to override the default parameter value.
Timothy Hatcher
Comment 5 2012-11-02 07:21:16 PDT
(In reply to comment #4) > I think you're going to have to update ImageBufferSkia to pay attention to your new parameter; it's already got a member variable tracking its scale. > > Could we come up with a better name for the parameter and the enum values? I really can't tell reading the declaration or call sites what the new code is intended to do, or why you need to override the default parameter value. It has a member tracking the resolution, but it never scaled like ImageBufferCG. So it isn't handling HiDPI the same as CG. I can add a FIXME and file a bug for Skia. But I don't know Skia/Chromium to be able to fix it.
Timothy Hatcher
Comment 6 2012-11-02 07:22:39 PDT
(In reply to comment #4) > Could we come up with a better name for the parameter and the enum values? I really can't tell reading the declaration or call sites what the new code is intended to do, or why you need to override the default parameter value. As far as the name goes I am open to ideas. But this was the best I could think of while keeping it terse.
Dean Jackson
Comment 7 2012-11-08 21:23:44 PST
Comment on attachment 171143 [details] Proposed Change View in context: https://bugs.webkit.org/attachment.cgi?id=171143&action=review > LayoutTests/ChangeLog:3 > + Test if -webkit-canvas in CSS uses the full backing store instead of alway 1x when rendering. Typo: always > LayoutTests/fast/canvas/canvas-as-image-hidpi.html:19 > + ctx.fillStyle = "rgb(200, 0, 0)"; > + ctx.fillRect(10, 10, 50, 50); > + > + ctx.fillStyle = "rgba(0, 0, 200, 0.5)"; > + ctx.fillRect(25, 25, 90, 90); I'm not sure this will actually test the change. This particular code drawn into 100x100 and then stretched would be indistinguishable from the 200x200 image. Or am I missing it? I can't think of a good way to test otherwise though. > Source/WebCore/ChangeLog:3 > + Make -webkit-canvas in CSS use the full backing store instead of alway 1x when rendering. Typo: always > Source/WebCore/platform/graphics/ImageBuffer.h:81 > + enum Scale { > + Scaled, > + Unscaled > + }; Should this be called ScaleBehavior or something? Scale is a slightly ambiguous word.
Timothy Hatcher
Comment 8 2012-11-09 07:11:46 PST
Comment on attachment 171143 [details] Proposed Change View in context: https://bugs.webkit.org/attachment.cgi?id=171143&action=review >> LayoutTests/fast/canvas/canvas-as-image-hidpi.html:19 >> + ctx.fillRect(25, 25, 90, 90); > > I'm not sure this will actually test the change. This particular code drawn into 100x100 and then stretched would be indistinguishable from the 200x200 image. Or am I missing it? > > I can't think of a good way to test otherwise though. I did run the test before and after the change and the edges were blurry before. >> Source/WebCore/platform/graphics/ImageBuffer.h:81 >> + }; > > Should this be called ScaleBehavior or something? Scale is a slightly ambiguous word. I can call it ScaleBehavior.
Timothy Hatcher
Comment 9 2012-11-09 12:41:05 PST
Nico Weber
Comment 10 2013-01-29 10:11:48 PST
(In reply to comment #5) > (In reply to comment #4) > > I think you're going to have to update ImageBufferSkia to pay attention to your new parameter; it's already got a member variable tracking its scale. > > > > Could we come up with a better name for the parameter and the enum values? I really can't tell reading the declaration or call sites what the new code is intended to do, or why you need to override the default parameter value. > > It has a member tracking the resolution, but it never scaled like ImageBufferCG. So it isn't handling HiDPI the same as CG. I can add a FIXME and file a bug for Skia. But I don't know Skia/Chromium to be able to fix it. That's intentional, chrome is matching the safari/ios model, not the safari/mac one: http://www.html5rocks.com/en/tutorials/canvas/hidpi/
Note You need to log in before you can comment on or make changes to this bug.