[webkit-reviews] review denied: [Bug 136801] REGRESSION: Text with a zero offset, zero blur shadow vanishes : [Attachment 238150] Patch

[bugzilla-daemon at webkit.org]

Simon Fraser (smfr) <simon.fraser at apple.com> has denied Myles C. Maxfield
<mmaxfield at apple.com>'s request for review:
Bug 136801: REGRESSION: Text with a zero offset, zero blur shadow vanishes
https://bugs.webkit.org/show_bug.cgi?id=136801

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

------- Additional Comments from Simon Fraser (smfr) <simon.fraser at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=238150&action=review


> Source/WebCore/rendering/TextPainter.cpp:82
> +    // often draw the ::last:: shadow and the text itself in a single call.

I don't like ::last::

> Source/WebCore/rendering/svg/SVGInlineTextBox.cpp:606
> +	       extraOffset =
TextPainter::applyShadowToGraphicsContext(*context, shadow, shadowRect,
onlyDrawsShadow,
> +		   TextPainter::isEmptyShadow(shadow,
MainTextIsOpaqueOrNot::MainTextIsNotOpaque), Horizontal);

I think the enumification is overzealous. This is the only call site with
default values for the last few parameters, and that could have been avoided by
giving them default values in the declaration. Your enumification creates
ugliness like
DrawsRealTextAlongWithShadowOrNot::DoesNotDrawRealTextAlongWithShadow in the
imp, which makes me throw up a little.