[Webkit-unassigned] [Bug 119324] [CSS Masking] -webkit-mask-repeat: space does not work

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=119324


Dirk Schulze <krit at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #208098|review?                     |review-
               Flag|                            |




--- Comment #6 from Dirk Schulze <krit at webkit.org>  2013-08-04 23:50:33 PST ---
(From update of attachment 208098)
View in context: https://bugs.webkit.org/attachment.cgi?id=208098&action=review

r- because of some change request. I didn't check the change in ModelObject yet.

> Source/WebCore/ChangeLog:6
> +        Reviewed by NOBODY (OOPS!).

You need to explain what you do, why you do it and why this is the right thing to archive the right behavior.

> Source/WebCore/ChangeLog:24
> +        (WebCore::RenderBoxModelObject::paintFillLayerExtended):
> +        (WebCore::RenderBoxModelObject::calculateBackgroundImageGeometry):

Smaller inline comments can help to understand what you do.

> Source/WebCore/platform/graphics/cg/ImageCG.cpp:136
> +        // On Leopard and newer, this code now only runs for partially decoded images whose buffers do not yet match the overall size of the image.
> +        static const CGPatternCallbacks patternCallbacks = { 0, drawPatternCallback, 0 };
> +        CGAffineTransform matrix = CGAffineTransformMake(narrowPrecisionToCGFloat(patternTransform.a()), 0, 0, narrowPrecisionToCGFloat(patternTransform.d()), adjustedX, adjustedY);
> +        matrix = CGAffineTransformConcat(matrix, CGContextGetCTM(context));
> +        // The top of a partially-decoded image is drawn at the bottom of the tile. Map it to the top.
> +        matrix = CGAffineTransformTranslate(matrix, 0, size().height() - h);
> +        RetainPtr<CGPatternRef> pattern = adoptCF(CGPatternCreate(subImage.get(), CGRectMake(0, 0, tileRect.width(), tileRect.height()),
> +            matrix, tileRect.width() + spaceSize().width(), tileRect.height() + spaceSize().height(),
> +            kCGPatternTilingConstantSpacing, true, &patternCallbacks));
> +        if (!pattern)
> +            return;
> +

It would be great if you can use the Pattern class already in WebKit to do that, so that it works in all ports, not just Mac. You may need to add some more code to Pattern to make that possible. Can you check this first please? Resetting review flag for now.

> LayoutTests/css3/background/background-repeat-border-expected.html:16
> +            var urls = Array(), size = Array(), position = Array(), repeat = Array(),
> +                origin = Array(), clip = Array();

Does that all need to be JS? Can't you just write a simple test case to make sure that it works?

> LayoutTests/css3/background/background-repeat-border-expected.html:18
> +            function addMasks() {

If you are testing background, then mask might be misleading.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.