<html>
    <head>
      <base href="https://bugs.webkit.org/" />
    </head>
    <body>
      <p>
        <div>
            <b><a class="bz_bug_link 
          bz_status_NEW "
   title="NEW - Add support for wide gamut for ShareableBitmap for image popovers"
   href="https://bugs.webkit.org/show_bug.cgi?id=164001#c3">Comment # 3</a>
              on <a class="bz_bug_link 
          bz_status_NEW "
   title="NEW - Add support for wide gamut for ShareableBitmap for image popovers"
   href="https://bugs.webkit.org/show_bug.cgi?id=164001">bug 164001</a>
              from <span class="vcard"><a class="email" href="mailto:thorton&#64;apple.com" title="Tim Horton &lt;thorton&#64;apple.com&gt;"> <span class="fn">Tim Horton</span></a>
</span></b>
        <pre>Comment on <span class=""><a href="attachment.cgi?id=292860&amp;action=diff" name="attach_292860" title="Patch">attachment 292860</a> <a href="attachment.cgi?id=292860&amp;action=edit" title="Patch">[details]</a></span>
Patch

View in context: <a href="https://bugs.webkit.org/attachment.cgi?id=292860&amp;action=review">https://bugs.webkit.org/attachment.cgi?id=292860&amp;action=review</a>

<span class="quote">&gt; Source/WebKit2/Shared/cg/ShareableBitmapCG.cpp:42
&gt; +    if ((flags &amp; ShareableBitmap::SupportsExtendedColor) &amp;&amp; screenSupportsExtendedColor()) {</span >

I'm not 100% sure we want this to depend on the screen. I believe images that bounce through ShareableBitmap can end up getting transmitted to other devices. Maybe we want callers to decide whether they set SupportsExtendedColor based on screenSupportsExtendedColor instead, since they will know whether there's a chance of it going elsewhere or not?

<span class="quote">&gt; Source/WebKit2/Shared/cg/ShareableBitmapCG.cpp:72
&gt; +    RetainPtr&lt;CGContextRef&gt; bitmapContext = adoptCF(CGBitmapContextCreateWithData(data(), m_size.width(), m_size.height(), m_pixelSize &lt;&lt; 1 /* multiply by 2 *8/4 = &lt;&lt; 1 */, m_size.width() * m_pixelSize, colorSpace, bitmapInfo(m_flags), releaseBitmapContextData, this));</span >

I think it's really OK to multiply by 2 instead of shifting here.

<span class="quote">&gt; Source/WebKit2/Shared/cg/ShareableBitmapCG.cpp:110
&gt; +    // FIXME: Make this use extended color, etc.</span >

I don't think this is optional? You're pointing at extended color data, won't that cause trouble if you make a non-extended-sRGB image from it? Or maybe it's OK because of the semicompatible nature of extended sRGB? What happens to out-of-range values?</pre>
        </div>
      </p>
      <hr>
      <span>You are receiving this mail because:</span>
      
      <ul>
          <li>You are the assignee for the bug.</li>
      </ul>
    </body>
</html>