Bug 193583 - CSS auto focus-ring outlines don't render on iOS
Summary: CSS auto focus-ring outlines don't render on iOS
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: WebKit Local Build
Hardware: iPhone / iPad iOS 12
: P2 Normal
Assignee: Daniel Bates
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-01-18 11:36 PST by Daniel Bates
Modified: 2019-01-20 14:38 PST (History)
4 users (show)

See Also:


Attachments
Patch (20.81 KB, patch)
2019-01-18 12:16 PST, Daniel Bates
no flags Details | Formatted Diff | Diff
Patch (20.86 KB, patch)
2019-01-18 12:20 PST, Daniel Bates
no flags Details | Formatted Diff | Diff
To Land (20.45 KB, patch)
2019-01-18 14:25 PST, Daniel Bates
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Daniel Bates 2019-01-18 11:36:27 PST
We should support drawing focus rings on iOS.
Comment 1 Daniel Bates 2019-01-18 11:36:36 PST
<rdar://problem/6508697>
Comment 2 Daniel Bates 2019-01-18 12:16:46 PST
Created attachment 359520 [details]
Patch
Comment 3 Daniel Bates 2019-01-18 12:20:20 PST
Created attachment 359522 [details]
Patch

Remove ColorIOS.mm from Xcode project and add to SourceCocoa.txt so that it can be part of unified builds.
Comment 4 Simon Fraser (smfr) 2019-01-18 12:41:18 PST
Comment on attachment 359522 [details]
Patch

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

> Source/WebCore/platform/ios/ColorIOS.mm:37
> +    // FIXME: Make this work for a UIColor that was created from a pattern.

Also make it work with DispayP3 colors.

> Source/WebCore/platform/ios/ColorIOS.mm:46
> +    static const double scaleFactor = nextafter(256.0, 0.0);
> +    return makeRGBA(scaleFactor * redComponent, scaleFactor * greenComponent, scaleFactor * blueComponent, scaleFactor * alpha);

Why not makeRGBA32FromFloats()?
Comment 5 Daniel Bates 2019-01-18 14:14:31 PST
(In reply to Simon Fraser (smfr) from comment #4)
> Comment on attachment 359522 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=359522&action=review
> 
> > Source/WebCore/platform/ios/ColorIOS.mm:37
> > +    // FIXME: Make this work for a UIColor that was created from a pattern.
> 
> Also make it work with DispayP3 colors.

Will amend this comment before landing.

> 
> > Source/WebCore/platform/ios/ColorIOS.mm:46
> > +    static const double scaleFactor = nextafter(256.0, 0.0);
> > +    return makeRGBA(scaleFactor * redComponent, scaleFactor * greenComponent, scaleFactor * blueComponent, scaleFactor * alpha);
> 
> Why not makeRGBA32FromFloats()?

So, you're staying makeRGBA32FromFloats() is analogous to this code? I took this code from ColorMac.m: <https://trac.webkit.org/browser/trunk/Source/WebCore/platform/graphics/mac/ColorMac.mm?rev=240041#L56>.
Comment 6 Daniel Bates 2019-01-18 14:19:25 PST
(In reply to Daniel Bates from comment #5)
> > > Source/WebCore/platform/ios/ColorIOS.mm:46
> > > +    static const double scaleFactor = nextafter(256.0, 0.0);
> > > +    return makeRGBA(scaleFactor * redComponent, scaleFactor * greenComponent, scaleFactor * blueComponent, scaleFactor * alpha);
> > 
> > Why not makeRGBA32FromFloats()?
> 
> So, you're staying makeRGBA32FromFloats() is analogous to this code? I took
> this code from ColorMac.m:
> <https://trac.webkit.org/browser/trunk/Source/WebCore/platform/graphics/mac/
> ColorMac.mm?rev=240041#L56>.

I am going to defer doing this for now as makeRGBA32FromFloats() does not seem to produce identical results from code inspection as well as by the comment in Color.cpp, <https://trac.webkit.org/browser/trunk/Source/WebCore/platform/graphics/Color.cpp?rev=240041#L84>. If I make this change for ColorIOS.mm then we should make it for ColorMac.mm.
Comment 7 Daniel Bates 2019-01-18 14:25:43 PST
Created attachment 359539 [details]
To Land
Comment 8 Daniel Bates 2019-01-18 14:26:48 PST
Committed r240174: <https://trac.webkit.org/changeset/240174>
Comment 9 Simon Fraser (smfr) 2019-01-18 14:28:51 PST
(In reply to Daniel Bates from comment #6)
> (In reply to Daniel Bates from comment #5)
> > > > Source/WebCore/platform/ios/ColorIOS.mm:46
> > > > +    static const double scaleFactor = nextafter(256.0, 0.0);
> > > > +    return makeRGBA(scaleFactor * redComponent, scaleFactor * greenComponent, scaleFactor * blueComponent, scaleFactor * alpha);
> > > 
> > > Why not makeRGBA32FromFloats()?
> > 
> > So, you're staying makeRGBA32FromFloats() is analogous to this code? I took
> > this code from ColorMac.m:
> > <https://trac.webkit.org/browser/trunk/Source/WebCore/platform/graphics/mac/
> > ColorMac.mm?rev=240041#L56>.
> 
> I am going to defer doing this for now as makeRGBA32FromFloats() does not
> seem to produce identical results from code inspection as well as by the
> comment in Color.cpp,
> <https://trac.webkit.org/browser/trunk/Source/WebCore/platform/graphics/
> Color.cpp?rev=240041#L84>. If I make this change for ColorIOS.mm then we
> should make it for ColorMac.mm.

We have multiple code paths that do float -> 0-255 color component conversions, and I think we probably have 3 types of rounding:
1. Multiply by nextafter(256.0, 0.0);
2. colorFloatToRGBAByte() which is lroundf(255.0f * f)
3. f * 255 and truncate.

We need to standardize on one.
Comment 10 Daniel Bates 2019-01-18 14:56:18 PST
Committed attempt to fix the iOS build in <https://trac.webkit.org/changeset/240177>.
Comment 11 Daniel Bates 2019-01-18 15:51:36 PST
Committed another build fix in <https://trac.webkit.org/changeset/240179/>.
Comment 12 Daniel Bates 2019-01-18 15:51:50 PST
Committed another build fix in <https://trac.webkit.org/changeset/240185/>.