[webkit-reviews] review denied: [Bug 76527] Mask deformations when masked content is rotated : [Attachment 124002] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Sat Feb 25 03:51:03 PST 2012


Nikolas Zimmermann <zimmermann at kde.org> has denied Florin Malita
<fmalita at google.com>'s request for review:
Bug 76527: Mask deformations when masked content is rotated
https://bugs.webkit.org/show_bug.cgi?id=76527

Attachment 124002: Patch
https://bugs.webkit.org/attachment.cgi?id=124002&action=review

------- Additional Comments from Nikolas Zimmermann <zimmermann at kde.org>
I'd like to see the testcases using the repaint.js framework, instead of
reftests. Also I'm still not sure I understand it yet:
<g mask="ur(#mask)">
 <rect id="masker"..>
If you change the transform attribute for masker, it has no effect on the mask
in trunk?
If you'd use <rect id="masker" mask="..." /> and change the transform here, the
mask updates properly in trunk?

I'm not at all against, revisiting the whole resource updating logic, but I
wouldn't like it, if we had a mixture. Currently most resource updating logic
is piped through markForLayoutAndParentResourceInvalidation, and it would be a
new approach, if clippers/maskers/filters would track the target repaint rect
instead to save redraws. The question is: how can we unify this?

In general, I think tracking the absolute repaint rect should be an
optimization, and not a bug fix? Isn't it just a side-effect that this is now
fixed.
Are there no other ways of invalidation that still won't work?

What about, eg:
<g mask="url(#mask)">
    <svg id="svgWithViewBox" viewBox="0 0 1 100">
	<rect id="masker" x="...
    </svg
</g>

Now change the 'viewBox' attribute through a script. Does that work in trunk,
or with your patch? I don't fully understand it yet, it seems.


More information about the webkit-reviews mailing list