Bug 42110 – Inconsistent Handling of Text Drawing Mode Flags

• Status: RESOLVED
• Resolution: INVALID
• Component: WebCore Misc.

[Brent Fulgham]

During a recent IRC chat with Mihnea Ovidenea, he pointed out that the handling of text drawing modes in FontCairo was inconsistent.  In one place we check for the explicit type "cTextFill", while in others we check for the presence of the "cTextFill" bit in the textDrawingMode member.

A quick 'grep' of the WebKit sources indicate that text drawing mode is often set explicitly:

Example 1:
CanvasRenderingContext2D.cpp:
1687:        c->setTextDrawingMode(fill ? cTextFill : cTextStroke);

On first glance, this seems correct.  But consider that text drawing mode is a bitmask, which might already have one or more drawing styles set (e.g., cTextStroke), which the user does not want to lose, just because the text should be filled.

Example 2:
FontCGWin.cpp:
357:           bool hasSimpleShadow = graphicsContext->textDrawingMode() == cTextFill && shadowColor.isValid [...]

Shouldn't this be checking to see if 'text filled' flag is on, rather than that ONLY the text fill flag is set?  What if the text is clipped and filled (cTextFill & cTextClip)?

Likewise, RenderSVGResourceGradient.cpp:
199:           context->setTextDrawingMode(resourceMode & ApplyToFillMode ? cTextFill : cTextStroke);

[Brent Fulgham]

Tests that show this problem:

1. fast/text/stroking-decorations.html
2. fast/text/stroking.html
3. fast/text/shadow-translucent-fill.html

Issues only seem to occur with the Cairo ports (Win and Gtk).  Skia and CG backends both look fine.

[Brent Fulgham]

I chatted with Dave Hyatt on IRC to get a definitive ruling on this issue.

Dave saw no incorrect code with fill/stroke (in the cross-platform code at least).  Although it is true that the clip gets clobbered in the examples above, nobody even uses clip so we could just yank it.

Fill/stroke are the only values we're even supporting.  When building up the text style enumerations, he was just mapping to the complete cg api, but we (Apple) don't use clip at all, so could simplify things by just getting rid of the unused flags.

I asked if the combination of Fill & Stroke are correct for the cross-platform logic.  Dave checked and confirmed that the combination works fine.... the cross-platform code isn't incorrectly clobbering anywhere that he could see:
* Canvas clobbering the state is fine, since it sets it every time it draws text
* InlineTextBox doesn't clobber and does the right bitwise oring.

I didn't understand why the line in RenderSVGResourceGradient.cpp (line 199) would NOT cause the stroke to get turned off if you were trying to fill and stroke at the same time.

Dave responded that it would, but there's a save/restore.  Text drawing mode is part of the state you save/restore, so nothing is left in an incorrect state.

Bottom line, this is a non-issue.

[Brent Fulgham]

Closing after discussing issue with Dave Hyatt.