RESOLVED FIXED Bug 83072
Pixel access canvas APIs do not work transparently with high-DPI backing store
https://bugs.webkit.org/show_bug.cgi?id=83072
Summary Pixel access canvas APIs do not work transparently with high-DPI backing store
mitz
Reported 2012-04-03 14:31:38 PDT
Pixel access canvas APIs do not work transparently with high-DPI backing store
Attachments
Make getImageData, putImageData, and toDataURL operate in user space and downample/upsample as necessary (42.07 KB, patch)
2012-04-03 16:59 PDT, mitz
no flags
Archive of layout-test-results from ec2-cr-linux-03 (386.46 KB, application/zip)
2012-04-03 17:36 PDT, WebKit Review Bot
no flags
Make getImageData, putImageData, and toDataURL operate in user space and downample/upsample as necessary (42.49 KB, patch)
2012-04-04 09:58 PDT, mitz
simon.fraser: review+
mitz
Comment 1 2012-04-03 16:59:49 PDT
Created attachment 135459 [details] Make getImageData, putImageData, and toDataURL operate in user space and downample/upsample as necessary
WebKit Review Bot
Comment 2 2012-04-03 17:04:52 PDT
Attachment 135459 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1 Source/WebCore/platform/graphics/cg/ImageBufferDataCG.h:61: The parameter name "rect" adds no information, so it should be removed. [readability/parameter_name] [5] Source/WebCore/platform/graphics/cg/ImageBufferDataCG.h:61: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] Source/WebCore/platform/graphics/cg/ImageBufferDataCG.h:62: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 3 in 12 files If any of these errors are false positives, please file a bug against check-webkit-style.
WebKit Review Bot
Comment 3 2012-04-03 17:36:38 PDT
Comment on attachment 135459 [details] Make getImageData, putImageData, and toDataURL operate in user space and downample/upsample as necessary Attachment 135459 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/12321456 New failing tests: compositing/checkerboard.html animations/3d/matrix-transform-type-animation.html animations/3d/state-at-end-event-transform.html compositing/direct-image-compositing.html compositing/video-page-visibility.html animations/additive-transform-animations.html animations/3d/replace-filling-transform.html animations/opacity-transform-animation.html compositing/scrollbar-painting.html compositing/backface-visibility.html compositing/text-on-large-layer.html compositing/layers-inside-overflow-scroll.html compositing/animation/state-at-end-event-transform-layer.html compositing/sibling-positioning.html compositing/generated-content.html compositing/fixed-position-changed-in-composited-layer.html compositing/self-painting-layers.html animations/3d/change-transform-in-end-event.html animations/missing-values-last-keyframe.html compositing/absolute-position-changed-with-composited-parent-layer.html compositing/absolute-position-changed-in-composited-layer.html compositing/flat-with-transformed-child.html accessibility/aria-disabled.html animations/missing-values-first-keyframe.html compositing/backface-visibility-hierarchical-transform.html compositing/culling/clear-fixed-iframe.html compositing/compositing-visible-descendant.html compositing/fixed-position-changed-within-composited-parent-layer.html compositing/color-matching/image-color-matching.html compositing/color-matching/pdf-image-match.html
WebKit Review Bot
Comment 4 2012-04-03 17:36:41 PDT
Created attachment 135465 [details] Archive of layout-test-results from ec2-cr-linux-03 The attached test failures were seen while running run-webkit-tests on the chromium-ews. Bot: ec2-cr-linux-03 Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'> Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick
mitz
Comment 5 2012-04-03 17:44:46 PDT
Dimitri, do you know which ImageBuffer implementation is used in the configuration that’s produced attachment 135465 [details]?
Adam Barth
Comment 6 2012-04-03 22:52:37 PDT
JamesR probably knows which ImageBuffer implementation is used in cr-linux for the compositing tests.
mitz
Comment 7 2012-04-04 09:51:25 PDT
I have an idea about what might be causing this, so I am going to submit a new patch soon.
mitz
Comment 8 2012-04-04 09:58:43 PDT
Created attachment 135614 [details] Make getImageData, putImageData, and toDataURL operate in user space and downample/upsample as necessary
WebKit Review Bot
Comment 9 2012-04-04 10:01:20 PDT
Attachment 135614 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1 Source/WebCore/platform/graphics/cg/ImageBufferDataCG.h:61: The parameter name "rect" adds no information, so it should be removed. [readability/parameter_name] [5] Source/WebCore/platform/graphics/cg/ImageBufferDataCG.h:61: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] Source/WebCore/platform/graphics/cg/ImageBufferDataCG.h:62: The parameter name "size" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 3 in 12 files If any of these errors are false positives, please file a bug against check-webkit-style.
Tim Horton
Comment 10 2012-04-05 13:16:07 PDT
Comment on attachment 135614 [details] Make getImageData, putImageData, and toDataURL operate in user space and downample/upsample as necessary View in context: https://bugs.webkit.org/attachment.cgi?id=135614&action=review > Source/WebCore/platform/graphics/cairo/ImageBufferCairo.cpp:60 > +ImageBuffer::ImageBuffer(const IntSize& size, float /* resolutionScale */, ColorSpace, RenderingMode, DeferralMode, bool& success) > : m_data(size) > , m_size(size) > + , m_logicalSize(size) Why not store resolutionScale, even if it's not being used yet? (in all of the ImageBuffer*s) It lives on ImageBuffer unconditionally anyway. > Source/WebCore/platform/graphics/cg/ImageBufferCG.cpp:457 > + RetainPtr<CGColorSpaceRef> colorSpace(AdoptCF, CGColorSpaceCreateDeviceRGB()); Can you use the shared deviceRGBColorSpaceRef for all of these? (from GraphicsContextCG) > Source/WebCore/platform/graphics/cg/ImageBufferDataCG.cpp:42 > +using namespace std; > + Do we do this? (import the whole namespace, that is, I have no problem with you using stuff from it). > Source/WebCore/platform/graphics/cg/ImageBufferDataCG.cpp:191 > + vImageAffineWarp_ARGB8888(&src, &dst, 0, &scaleTransform, backgroundColor, kvImageEdgeExtend); Ooh!
mitz
Comment 11 2012-04-05 14:03:32 PDT
(In reply to comment #10) > (From update of attachment 135614 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=135614&action=review > > > Source/WebCore/platform/graphics/cairo/ImageBufferCairo.cpp:60 > > +ImageBuffer::ImageBuffer(const IntSize& size, float /* resolutionScale */, ColorSpace, RenderingMode, DeferralMode, bool& success) > > : m_data(size) > > , m_size(size) > > + , m_logicalSize(size) > > Why not store resolutionScale, even if it's not being used yet? (in all of the ImageBuffer*s) I wanted to make it explicit that things aren’t working yet in these ports. > > It lives on ImageBuffer unconditionally anyway. > > > Source/WebCore/platform/graphics/cg/ImageBufferCG.cpp:457 > > + RetainPtr<CGColorSpaceRef> colorSpace(AdoptCF, CGColorSpaceCreateDeviceRGB()); > > Can you use the shared deviceRGBColorSpaceRef for all of these? (from GraphicsContextCG) Yes, I can! > > > Source/WebCore/platform/graphics/cg/ImageBufferDataCG.cpp:42 > > +using namespace std; > > + > > Do we do this? (import the whole namespace, that is, I have no problem with you using stuff from it). Yes, this is the current policy. > > > Source/WebCore/platform/graphics/cg/ImageBufferDataCG.cpp:191 > > + vImageAffineWarp_ARGB8888(&src, &dst, 0, &scaleTransform, backgroundColor, kvImageEdgeExtend); > > Ooh! What what?
Simon Fraser (smfr)
Comment 12 2012-04-06 10:29:49 PDT
Comment on attachment 135614 [details] Make getImageData, putImageData, and toDataURL operate in user space and downample/upsample as necessary View in context: https://bugs.webkit.org/attachment.cgi?id=135614&action=review > Source/WebCore/platform/graphics/cg/ImageBufferCG.cpp:118 > + // FIXME: Should we automatically use lower resolution? ...use a lower... or ...use the lower... ?
mitz
Comment 13 2012-04-06 10:49:07 PDT
Fixed (for CG) in <http://trac.webkit.org/r113457>.
Beth Dakin
Comment 14 2012-04-06 14:50:41 PDT
(In reply to comment #13) > Fixed (for CG) in <http://trac.webkit.org/r113457>. It seems like this may have caused many layout test failures on SnowLeopard.
mitz
Comment 15 2012-04-06 15:22:18 PDT
(In reply to comment #14) > (In reply to comment #13) > > Fixed (for CG) in <http://trac.webkit.org/r113457>. > > It seems like this may have caused many layout test failures on SnowLeopard. I am going to see if I can reproduce those on Lion by disabling acceleration.
mitz
Comment 16 2012-04-06 16:58:48 PDT
Even without using Accelerate and IOSurface-backed canvas, I cannot reproduce failures on Lion. I am going to try to reproduce on Snow Leopard.
mitz
Comment 17 2012-04-06 20:15:36 PDT
With a debug build of r113531 on Mac OS X 10.6.8, I ran all the tests in canvas and fast/canvas, and did not reproduce the failures seen on the build bot.
mitz
Comment 18 2012-04-06 20:16:25 PDT
I am going to try a release build next.
mitz
Comment 19 2012-04-06 21:34:58 PDT
Also unable to reproduce those failures with a release build.
mitz
Comment 20 2012-04-08 20:39:27 PDT
Looking again at build.webkit.org, it appears as if the Snow Leopard Release (Tests) build has been exiting early after 500 failures even before r113457, but I can no longer see the results from those runs.
mitz
Comment 21 2012-04-08 21:27:01 PDT
I can get fast/canvas/canvas-arc-360-winding.html to fail if it is run after compositing/tiled-layers-hidpi.html (the Snow Leopard bots sometimes run all the tests using a single DumpRenderTree instance, so this effect becomes visible): run-webkit-tests --child-processes=1 compositing/tiled-layers-hidpi.html fast/canvas/canvas-arc-360-winding.html
mitz
Comment 22 2012-04-08 21:40:22 PDT
Filed bug 83453.
Note You need to log in before you can comment on or make changes to this bug.