[webkit-reviews] review denied: [Bug 53055] REGRESSION: Rendering artifacts on a rotated, pattern filled shape : [Attachment 104612] style fix

[bugzilla-daemon at webkit.org]

Nikolas Zimmermann <zimmermann at kde.org> has denied  review:
Bug 53055: REGRESSION: Rendering artifacts on a rotated, pattern filled shape
https://bugs.webkit.org/show_bug.cgi?id=53055

Attachment 104612: style fix
https://bugs.webkit.org/attachment.cgi?id=104612&action=review

------- Additional Comments from Nikolas Zimmermann <zimmermann at kde.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=104612&action=review


I'm not sure about the SystemInterface stuff, as I have no idea how it works,
but I trust you on that. Still some questions:

> Source/WebCore/ChangeLog:11
> +	   Introduce wkCGPatternCreateWithImageAndTransform, and make use of it

> +	   when tiling patterns in both directions. This helps to prevent
> +	   pixel-cracking along pattern tiling boundaries.

It's so wonderful to have someone from Apple working on SVG, with the abilities
to do such things as extend the WebKitSystemInterface :-)

> Source/WebCore/ChangeLog:15
> +	   Fix a rounding error in the size of the temporary image created to
> +	   render SVG pattern tiles into, which was introduced in r65665.
> +	   We now round SVG tile image size up in all cases.

I think I introduced the ceilf logic on purpose. The ceilf alone makes no
sense, but it's used in conjunction with following code: in
PassOwnPtr<ImageBuffer> RenderSVGResourcePattern::createTileImage:

...
    if (!SVGImageBufferTools::createImageBuffer(absoluteTileBoundaries,
clampedAbsoluteTileBoundaries, tileImage, ColorSpaceDeviceRGB))
	return nullptr;

    GraphicsContext* tileImageContext = tileImage->context();
    ASSERT(tileImageContext);

    // The image buffer represents the final rendered size, so the content has
to be scaled (to avoid pixelation).
    tileImageContext->scale(FloatSize(absoluteTileBoundaries.width() /
tileBoundaries.width(),
				      absoluteTileBoundaries.height() /
tileBoundaries.height()));

Is this obsolete now? How are other platforms affected by your change to avoid
the ceil?
Maskers contain similar code to avoid fix-up rounding differences by the ceil.

> Source/WebCore/ChangeLog:17
> +	   No new tests. (OOPS!)

Can you add a test?