Bug 83840

Summary: [chromium] Consolidate adjustTextRenderMode copypasta into Skia context
Product: WebKit Reporter: Adrienne Walker <enne>
Component: New BugsAssignee: Adrienne Walker <enne>
Status: RESOLVED FIXED    
Severity: Normal CC: danakj, enne, jamesr, senorblanco, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 83609    
Attachments:
Description Flags
Patch none

Description Adrienne Walker 2012-04-12 18:12:00 PDT 
[chromium] Consolidate adjustTextRenderMode copypasta into Skia context
Comment 1 Adrienne Walker 2012-04-12 18:16:09 PDT 
Created attachment 137009 [details]
Patch
Comment 2 Dana Jansens 2012-04-12 21:20:35 PDT 
Comment on attachment 137009 [details]
Patch

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

Nice work, this makes a lot of sense!

> Source/WebCore/platform/graphics/skia/PlatformContextSkia.cpp:663
> +void PlatformContextSkia::adjustTextRenderMode(SkPaint* paint)

Can we make this an "SkPaint&" ?
Comment 3 Stephen White 2012-04-13 08:16:22 PDT 
Comment on attachment 137009 [details]
Patch

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

>> Source/WebCore/platform/graphics/skia/PlatformContextSkia.cpp:663
>> +void PlatformContextSkia::adjustTextRenderMode(SkPaint* paint)
> 
> Can we make this an "SkPaint&" ?

Although WebKit coding style says use non-const refs for out parameters, it doesn't saying anything about inout params AFAICT.  My personal preference is to use pointers, as Enne did here, since it makes it clearer at the call site that the parameter is being modified.
Comment 4 Dana Jansens 2012-04-13 09:02:23 PDT 
Comment on attachment 137009 [details]
Patch

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

>>> Source/WebCore/platform/graphics/skia/PlatformContextSkia.cpp:663
>>> +void PlatformContextSkia::adjustTextRenderMode(SkPaint* paint)
>> 
>> Can we make this an "SkPaint&" ?
> 
> Although WebKit coding style says use non-const refs for out parameters, it doesn't saying anything about inout params AFAICT.  My personal preference is to use pointers, as Enne did here, since it makes it clearer at the call site that the parameter is being modified.

Oh ok! Good to know.
Comment 5 Adrienne Walker 2012-04-16 11:48:48 PDT 
senorblanco, jamesr: ping for review?
Comment 7 WebKit Review Bot 2012-04-18 21:51:37 PDT 
Comment on attachment 137009 [details]
Patch

Clearing flags on attachment: 137009

Committed r114607: <http://trac.webkit.org/changeset/114607>
Comment 8 WebKit Review Bot 2012-04-18 21:51:42 PDT 
All reviewed patches have been landed.  Closing bug.