Bug 144351 - Checkboxes on bugs.webkit.org are painted with stripes at some zoom levels.
Summary: Checkboxes on bugs.webkit.org are painted with stripes at some zoom levels.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: zalan
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2015-04-28 13:11 PDT by zalan
Modified: 2015-05-01 12:26 PDT (History)
3 users (show)

See Also:


Attachments
Patch (5.45 KB, patch)
2015-04-28 13:28 PDT, zalan
no flags Details | Formatted Diff | Diff
Patch (5.45 KB, patch)
2015-04-28 13:38 PDT, zalan
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description zalan 2015-04-28 13:11:38 PDT
1. Load a bugs.webkit.org with checkboxes.
2. Zoom in and out until the checkboxes appear striped.
Comment 1 zalan 2015-04-28 13:12:01 PDT
rdar://problem/19726362
Comment 2 zalan 2015-04-28 13:28:21 PDT
Created attachment 251883 [details]
Patch
Comment 3 Simon Fraser (smfr) 2015-04-28 13:31:35 PDT
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!
Comment 4 zalan 2015-04-28 13:38:29 PDT
Created attachment 251884 [details]
Patch
Comment 5 Said Abou-Hallawa 2015-04-28 13:48:00 PDT
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.
Comment 6 zalan 2015-04-28 13:58:34 PDT
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.
Comment 7 WebKit Commit Bot 2015-04-28 15:22:39 PDT
Comment on attachment 251884 [details]
Patch

Clearing flags on attachment: 251884

Committed r183511: <http://trac.webkit.org/changeset/183511>
Comment 8 WebKit Commit Bot 2015-04-28 15:22:43 PDT
All reviewed patches have been landed.  Closing bug.
Comment 9 Said Abou-Hallawa 2015-05-01 12:26:43 PDT
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().