Bug 144351 – Checkboxes on bugs.webkit.org are painted with stripes at some zoom levels.

• Component: Layout and Rendering
• Status: RESOLVED FIXED

[alan]

1. Load a bugs.webkit.org with checkboxes.
2. Zoom in and out until the checkboxes appear striped.

[alan]

rdar://problem/19726362

[alan]

Created attachment 251883 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 251883 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=251883&action=review

> LayoutTests/fast/forms/checkbox-painting-with-hr.html:8
> +	    margin-top: -10px;
> +		border: 1px dashed white;

Whitespace!

[alan]

Created attachment 251884 [details]
Patch

[Said Abou-Hallawa]

Comment on attachment 251884 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=251884&action=review

> Source/WebCore/platform/graphics/cg/GraphicsContextCG.cpp:300
> +        CGContextSaveGState(context);

Can't we have a smart CG SaverRestorer class instead? So we do not have we worry about restoring the CG before every return statement.

[alan]

Comment on attachment 251884 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=251884&action=review

>> Source/WebCore/platform/graphics/cg/GraphicsContextCG.cpp:300
>> +        CGContextSaveGState(context);
> 
> Can't we have a smart CG SaverRestorer class instead? So we do not have we worry about restoring the CG before every return statement.

Yes, it'd be great to have one, but since GraphicsContextCG is pretty static, the risk of missing a restore is extremely low.

[WebKit Commit Bot]

Comment on attachment 251884 [details]
Patch

Clearing flags on attachment: 251884

Committed r183511: <http://trac.webkit.org/changeset/183511>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Said Abou-Hallawa]

Comment on attachment 251884 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=251884&action=review

>>> Source/WebCore/platform/graphics/cg/GraphicsContextCG.cpp:300
>>> +        CGContextSaveGState(context);
>> 
>> Can't we have a smart CG SaverRestorer class instead? So we do not have we worry about restoring the CG before every return statement.
> 
> Yes, it'd be great to have one, but since GraphicsContextCG is pretty static, the risk of missing a restore is extremely low.

Actually we do have one. I just found it. Its name is GraphicsContextStateSaver. So the above code can be something like this:

GraphicsContextStateSaver saver(*this, false);
if (drawsDashedLine) {
    saver.save();

And there is no need for the other restoring code. The disadvantage of using it here might be it is not platform specific. So it does more than just calling CGContextSaveGState()/CGContextRestoreGState().