Bug 255387 – [IFC] InlineLevelBox::LayoutBounds is a glorified AscentAndDescent

• Component: Layout and Rendering
• Status: RESOLVED FIXED

[alan]

ssia

[alan]

Created attachment 465880 [details]
Patch

[Antti Koivisto]

Comment on attachment 465880 [details]
Patch

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

> Source/WebCore/layout/formattingContexts/inline/InlineLevelBox.h:76
>      struct LayoutBounds {
> -        InlineLayoutUnit height() const { return ascent + descent; }
> -        bool operator==(const LayoutBounds& other) const { return ascent == other.ascent && descent == other.descent; }
> +        LayoutBounds(AscentAndDescent);
> +        LayoutBounds() = default;
>  
> -        InlineLayoutUnit ascent { 0 };
> -        InlineLayoutUnit descent { 0 };
> +        InlineLayoutUnit ascent() const { return m_ascentAndDescent.ascent; }
> +        InlineLayoutUnit descent() const { return m_ascentAndDescent.descent; }
> +
> +        InlineLayoutUnit height() const { return m_ascentAndDescent.height(); }
> +        bool operator==(const LayoutBounds& other) const { return m_ascentAndDescent == other.m_ascentAndDescent; }
> +
> +    private:
> +        AscentAndDescent m_ascentAndDescent;
>      };

Maybe this type can be just replaced with AscentAndDescent directly? Or `using LayoutBounds = AscentAndDescent`?

[alan]

(In reply to Antti Koivisto from comment #2)
> Comment on attachment 465880 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=465880&action=review
> 
> > Source/WebCore/layout/formattingContexts/inline/InlineLevelBox.h:76
> >      struct LayoutBounds {
> > -        InlineLayoutUnit height() const { return ascent + descent; }
> > -        bool operator==(const LayoutBounds& other) const { return ascent == other.ascent && descent == other.descent; }
> > +        LayoutBounds(AscentAndDescent);
> > +        LayoutBounds() = default;
> >  
> > -        InlineLayoutUnit ascent { 0 };
> > -        InlineLayoutUnit descent { 0 };
> > +        InlineLayoutUnit ascent() const { return m_ascentAndDescent.ascent; }
> > +        InlineLayoutUnit descent() const { return m_ascentAndDescent.descent; }
> > +
> > +        InlineLayoutUnit height() const { return m_ascentAndDescent.height(); }
> > +        bool operator==(const LayoutBounds& other) const { return m_ascentAndDescent == other.m_ascentAndDescent; }
> > +
> > +    private:
> > +        AscentAndDescent m_ascentAndDescent;
> >      };
> 
> Maybe this type can be just replaced with AscentAndDescent directly? Or
> `using LayoutBounds = AscentAndDescent`?
yeah it was going to be one of the next steps, but you are right, there's no need for this intermediate step. will change.

[alan]

Created attachment 465888 [details]
Patch

[alan]

Created attachment 465895 [details]
Patch

[alan]

Created attachment 465897 [details]
Patch

[alan]

Created attachment 465906 [details]
Patch

[EWS]

Committed 262942@main (a055bb114d45): <https://commits.webkit.org/262942@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 465906 [details].

[Radar WebKit Bug Importer]

<rdar://problem/108030264>