<html>
    <head>
      <base href="https://bugs.webkit.org/">
    </head>
    <body>
      <p>
        <div>
            <b><a class="bz_bug_link 
          bz_status_NEW "
   title="NEW - Implement the 'ic' unit from CSS Values 4"
   href="https://bugs.webkit.org/show_bug.cgi?id=204276#c28">Comment # 28</a>
              on <a class="bz_bug_link 
          bz_status_NEW "
   title="NEW - Implement the 'ic' unit from CSS Values 4"
   href="https://bugs.webkit.org/show_bug.cgi?id=204276">bug 204276</a>
              from <span class="vcard"><a class="email" href="mailto:tho22@apple.com" title="Kiet Ho <tho22@apple.com>"> <span class="fn">Kiet Ho</span></a>
</span></b>
        <pre>Comment on <span class=""><a href="attachment.cgi?id=439128&action=diff" name="attach_439128" title="Patch">attachment 439128</a> <a href="attachment.cgi?id=439128&action=edit" title="Patch">[details]</a></span>
Patch

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

<span class="quote">>>>>> Source/WebCore/platform/graphics/FontMetrics.h:173
>>>>> +    std::optional<float> m_ideogramWidth;
>>>> 
>>>> Why is this an optional? Why not have platformGlyphInit() implement the fallback behavior, rather than computeUnzoomedNonCalcLengthDouble() implement the fallback behavior?
>>> 
>>> I just feel like Font should truthfully indicate whether the ideogram width is available or not, so its users can make the correct decision. Right now computeUnzoomedNonCalcLengthDouble() falls back to 1em, but maybe in the future some other code might want to use m_ideogramWidth for something else?
>> 
>> That doesn't make any sense to me. The spec dictates a particular fallback behavior. There is one client, which implements the fallback behavior. Storing an optional stores an unnecessary extra 4 bytes per font.
>> 
>> If there were multiple clients who wanted different behavior here, that would make sense. But there isn't, so better to save those 4 bytes.

> I agree, I'll fix it as you said.</span >

I don't think it's possible to implement the fallback behavior inside Font/FontMetrics, because the fallback metric (1em) comes from FontCascadeDescription.m_specifiedSize, which depends on whatever font size being specified in CSS. This is not available when the Font is initialized.</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>