Bug 176949

Summary: [Mac] Spelling, grammar and correction dots are painted upside down
Product: WebKit Reporter: Daniel Bates <dbates>
Component: Layout and RenderingAssignee: Daniel Bates <dbates>
Status: RESOLVED FIXED    
Severity: Normal CC: bdakin, bfulgham, jonlee, simon.fraser, webkit-bug-importer, zalan
Priority: P2 Keywords: InRadar, PlatformOnly
Version: WebKit Local Build   
Hardware: Mac   
OS: macOS 10.13   
See Also: https://bugs.webkit.org/show_bug.cgi?id=176951
Attachments:
Description Flags
Patch
simon.fraser: review+
[Screenshot] Before patch
none
[Screenshot] After patch
none
[Screenshot] TextEdit none

Description Daniel Bates 2017-09-14 13:24:08 PDT
Sunlight falls from the sky. It does not rise from the earth. The dots are upside down.
Comment 1 Radar WebKit Bug Importer 2017-09-14 13:25:05 PDT
<rdar://problem/34441098>
Comment 2 Daniel Bates 2017-09-14 13:26:35 PDT
Created attachment 320805 [details]
Patch
Comment 3 Daniel Bates 2017-09-14 13:27:01 PDT
Created attachment 320806 [details]
[Screenshot] Before patch
Comment 4 Daniel Bates 2017-09-14 13:27:13 PDT
Created attachment 320807 [details]
[Screenshot] After patch
Comment 5 Daniel Bates 2017-09-14 13:28:49 PDT
Created attachment 320808 [details]
[Screenshot] TextEdit

For comparison, attached screenshot of the same text seen in the Before patch and After patch screenshots rendered in TextEdit with spelling and grammar checking enabled.
Comment 6 Daniel Bates 2017-09-14 13:39:34 PDT
If you look closely at screenshots "After patch" (attachment #320807 [details]) and "TextEdit" (attachment #320808 [details]) you will notice that the dots are shifted to the right in the latter. Filed bug #176951 to address this issue.
Comment 7 Simon Fraser (smfr) 2017-09-14 14:15:35 PDT
Comment on attachment 320805 [details]
Patch

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

> Source/WebCore/platform/graphics/cocoa/GraphicsContextCocoa.mm:322
> +        CGContextClipToRect(context, destinationRect);

Might as well convert this function (replace the CGContextSaveGState above this diff) to use CGContextStateSaver.

> Source/WebCore/platform/graphics/cocoa/GraphicsContextCocoa.mm:333
>          // FIXME: Rather than getting the NSImage and then picking the CGImage from it, we should do what iOS does and
>          // just load the CGImage in the first place.

Should we actually do this bit now?
Comment 8 Daniel Bates 2017-09-14 14:47:46 PDT
(In reply to Simon Fraser (smfr) from comment #7)
> Comment on attachment 320805 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=320805&action=review
> 
> > Source/WebCore/platform/graphics/cocoa/GraphicsContextCocoa.mm:322
> > +        CGContextClipToRect(context, destinationRect);
> 
> Might as well convert this function (replace the CGContextSaveGState above
> this diff) to use CGContextStateSaver.
> 

OK.

> > Source/WebCore/platform/graphics/cocoa/GraphicsContextCocoa.mm:333
> >          // FIXME: Rather than getting the NSImage and then picking the CGImage from it, we should do what iOS does and
> >          // just load the CGImage in the first place.
> 
> Should we actually do this bit now?

I do not see the need to do this now. Moreover, I suggest we look to make the Mac code use a CGPattern as we do for iOS. This will achieve the same effect as the FIXME above (to have the Mac and iOS code paths use CG objects and operations) and will allow us to remove an earlier FIXME in this function: <https://trac.webkit.org/browser/trunk/Source/WebCore/platform/graphics/cocoa/GraphicsContextCocoa.mm?rev=221485#L304>.
Comment 9 Daniel Bates 2017-09-14 16:52:40 PDT
Committed r222065: <http://trac.webkit.org/changeset/222065>