Bug 83072

Created attachment 135459 [details] Make getImageData, putImageData, and toDataURL operate in user space and downample/upsample as necessary

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.

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

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

Dimitri, do you know which ImageBuffer implementation is used in the configuration that’s produced attachment 135465 [details]?

JamesR probably knows which ImageBuffer implementation is used in cr-linux for the compositing tests.

I have an idea about what might be causing this, so I am going to submit a new patch soon.

Created attachment 135614 [details] Make getImageData, putImageData, and toDataURL operate in user space and downample/upsample as necessary

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.

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!

(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?

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... ?

Fixed (for CG) in <http://trac.webkit.org/r113457>.

(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.

(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.

Even without using Accelerate and IOSurface-backed canvas, I cannot reproduce failures on Lion. I am going to try to reproduce on Snow Leopard.

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.

I am going to try a release build next.

Also unable to reproduce those failures with a release build.

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.

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

Filed bug 83453.