Bug 82963 – REGRESSION (r101517): Animating the transform of a <rect> with shape-rendering: crispEdges leaves behind garbage

• Status: RESOLVED
• Resolution: FIXED
• Component: SVG

[Tim Horton]

Created attachment 135214 [details]
repro

Like the title says, animating the transform of a <rect> with shape-rendering: crispEdges leaves behind garbage. Probably the repaint rect is rounded to be too small by 1px or something.

See the attached test case. I ran into this with a very simple d3.js toy I was writing over the weekend, so I'm pretty sure it can adversely affect real world content. Bisection says 101517 is probably the culprit (makes sense to me!).

[Radar WebKit Bug Importer]

<rdar://problem/11170476>

[Tim Horton]

Created attachment 135215 [details]
screenshot

[Tim Horton]

Inflating the outer stroke rect (which is returned for strokeBoundingBox) of the RenderSVGRect by 1px fixes this.

It looks like CG can draw an extra 1px part of the stroke when drawing a rectangle with AA off, but not a path-made-from-a-rectangle, which is why we didn't run into this before 101517.

I have a patch to fix, pending a good test.

[Tim Horton]

Created attachment 135261 [details]
manual testcase

I can't make it reproduce in a test, something always causes extra repaint! (which happens if you click away from the browser, or scroll, or just wait a minute, or whatever, so it's not a very stable test to begin with).

[Tim Horton]

Created attachment 135266 [details]
patch sans test

[Tim Horton]

I cannot make this reproduce in WebKit1 at all (even in Safari). I can't make it reproduce in DRT or WKTR at all.

Simon, any qualms about checking in without a test? There's already a comment there which will hopefully dissuade anyone from removing the code.

[Simon Fraser (smfr)]

Nah, that's OK

[Tim Horton]

Landed in http://trac.webkit.org/changeset/113496

[Florin Malita]

I noticed this patch while looking into a somewhat similar Skia problem: https://bugs.webkit.org/show_bug.cgi?id=100374

m_strokeBoundingRect.inflate(1);

^^ this doesn't look right: we are not in device space here, and the above may or may not inflate the absolute repaint rect by one pixel (see the new test with a scale applied).

Since both CG and Skia seem to need repaint rect adjustments, maybe we can add a platform call for this purpose. But it's unclear what the best place for such an adjustment is: on one hand it makes sense to do it at the lowest level, where we know the stroke properties; on the other hand it's tricky to make screen space decisions here - but then the higher levels are clueless about a possible stroke on the edge of the repaint rect... Ideas?

[Florin Malita]

Created attachment 170701 [details]
Scaled test