Bug 78775 - [chromium] Clipping/Transforms applied in wrong order in opaque paint tracking
Summary: [chromium] Clipping/Transforms applied in wrong order in opaque paint tracking
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Dana Jansens
URL:
Keywords:
Depends on:
Blocks: 78755
  Show dependency treegraph
 
Reported: 2012-02-15 19:47 PST by Dana Jansens
Modified: 2012-02-16 15:51 PST (History)
6 users (show)

See Also:


Attachments
Patch (9.41 KB, patch)
2012-02-15 19:58 PST, Dana Jansens
no flags Details | Formatted Diff | Diff
Patch (13.17 KB, patch)
2012-02-16 10:46 PST, Dana Jansens
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dana Jansens 2012-02-15 19:47:05 PST
[chromium] Clipping/Transforms applied in wrong order in opaque paint tracking
Comment 1 Dana Jansens 2012-02-15 19:48:27 PST
The clip is applied in device coordinates, before transforming the painted rect into device coordinates. This makes any translations get doubly represented, and gives incorrect paint tracking results.
Comment 2 Dana Jansens 2012-02-15 19:58:36 PST
Created attachment 127301 [details]
Patch
Comment 3 WebKit Review Bot 2012-02-15 23:58:04 PST
Comment on attachment 127301 [details]
Patch

Attachment 127301 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/11534680

New failing tests:
compositing/scrollbar-painting.html
compositing/overflow/clip-content-under-overflow-controls.html
Comment 4 Dana Jansens 2012-02-16 10:33:45 PST
(I edited the scrollbar-painting.html test to remove the body margin/padding for easier numbers.)

===

Before this change, we see the draw in the bottom left corner as:

diddraw 85.000000 85.000000 15.000000x15.000000

The device clip is 0,0 15x15. So the draw is ignored for opaque tracking as it assumes it is not drawn at all.

===

After this change, we see it as:

diddraw 85.000000 85.000000 15.000000x15.000000
canvasTransform 0.000000 0.000000 15.000000x15.000000 1
clipped 0.000000 0.000000 15.000000x15.000000 1
targetTransform 0.000000 0.000000 15.000000x15.000000 1
opaque 1

ie. The transform puts us at 0,0 where the clip is applied, and the opaque paint can be used now for opaque tracking (and turning off blending). Making this our lovely off-by-1 friend again.

===

The paint in question here is

#0  WebCore::OpaqueRegionSkia::markRectAsOpaque (this=0x7fffde4523a0, rect=...) at ../../third_party/WebKit/Source/WebCore/platform/graphics/skia/OpaqueRegionSkia.cpp:293
#1  0x00007fffedcea9a2 in WebCore::OpaqueRegionSkia::didDraw (this=0x7fffde4523a0, context=0x7fffde452370, transform=..., rect=..., paint=..., drawsOpaque=true, fillsBounds=true) at ../../third_party/WebKit/Source/WebCore/platform/graphics/skia/OpaqueRegionSkia.cpp:271
#2  0x00007fffedcea1d8 in WebCore::OpaqueRegionSkia::didDrawRect (this=0x7fffde4523a0, context=0x7fffde452370, transform=..., fillRect=..., paint=..., bitmap=0x0) at ../../third_party/WebKit/Source/WebCore/platform/graphics/skia/OpaqueRegionSkia.cpp:169
#3  0x00007fffedcee2f6 in WebCore::PlatformContextSkia::didDrawRect (this=0x7fffde452370, rect=..., paint=..., bitmap=0x0) at ../../third_party/WebKit/Source/WebCore/platform/graphics/skia/PlatformContextSkia.cpp:611
#4  0x00007fffedce1f37 in WebCore::GraphicsContext::fillRect (this=0x7fffdd8cdaa0, rect=..., color=..., colorSpace=WebCore::ColorSpaceDeviceRGB) at ../../third_party/WebKit/Source/WebCore/platform/graphics/skia/GraphicsContextSkia.cpp:865
#5  0x00007fffed5f0f48 in WebCore::RenderLayer::paintScrollCorner (this=0x7fffe03a2438, context=0x7fffdd8cdaa0, paintOffset=..., damageRect=...) at ../../third_party/WebKit/Source/WebCore/rendering/RenderLayer.cpp:2496

Which is:
        context->fillRect(absRect, Color::white, box->style()->colorSpace());

With absRect = {85, 85, 15x15}. Clearly an opaque paint.

===

I will include new linux baselines in this CL.
Comment 5 Dana Jansens 2012-02-16 10:46:04 PST
Created attachment 127407 [details]
Patch

Includes new baselines for the off-by-1 fails. I suspect the mac platform baselines can go away after this.
Comment 6 Stephen White 2012-02-16 11:24:50 PST
Comment on attachment 127407 [details]
Patch

OK.  r=me
Comment 7 WebKit Review Bot 2012-02-16 15:51:18 PST
Comment on attachment 127407 [details]
Patch

Clearing flags on attachment: 127407

Committed r107988: <http://trac.webkit.org/changeset/107988>
Comment 8 WebKit Review Bot 2012-02-16 15:51:23 PST
All reviewed patches have been landed.  Closing bug.