Bug 186217 – [LFC] Merge width and horizontal margin computation

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[alan]

to be able to closely follow the steps defined at https://www.w3.org/TR/CSS22/visudet.html

[alan]

Created attachment 341813 [details]
Patch

[alan]

Created attachment 341814 [details]
Patch

[alan]

Comment on attachment 341814 [details]
Patch

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

> Source/WebCore/layout/displaytree/DisplayBox.h:175
> +    struct EdgePair {
> +        LayoutUnit first;
> +        LayoutUnit second;
> +    };
> +    using HorizontalEdges = EdgePair;
> +    using VerticalEdges = EdgePair;
>  
> +    struct Edges {
>          LayoutUnit top;
>          LayoutUnit left;
>          LayoutUnit bottom;
>          LayoutUnit right;
>      };

Edges could be defined like this 
struct Edges {
    HorizontalEdges horizontal;
    VerticalEdges vertical;
}; 
but then top would read like edges.vertical.first instead of edges.top. I find edges.top more readable.

[Sam Weinig]

Comment on attachment 341814 [details]
Patch

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

> Source/WebCore/ChangeLog:9
> +        at https://www.w3.org/TR/CSS22/visudet.html, we should just merge width and horizontal maring computations

s/maring/margin/

> Source/WebCore/layout/displaytree/DisplayBox.h:167
> +    struct EdgePair {
> +        LayoutUnit first;
> +        LayoutUnit second;
> +    };
> +    using HorizontalEdges = EdgePair;

Could EdgePair be defined as:

using EdgePair = std::pair<LayoutUnit, LayoutUnit>;

Though, actually, I think what might be the cleanest is the really verbose:

struct HorizontalEdges {
    LayoutUnit left;
    LayoutUnit right;
};
struct VerticalEdges {
    LayoutUnit top;
    LayoutUnit bottom;
};
struct Edges {
    HorizontalEdges horizontal;
    VerticalEdges vertical;
};

[alan]

(In reply to Sam Weinig from comment #4)
> Comment on attachment 341814 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=341814&action=review
> 
> > Source/WebCore/ChangeLog:9
> > +        at https://www.w3.org/TR/CSS22/visudet.html, we should just merge width and horizontal maring computations
> 
> s/maring/margin/
> 
> > Source/WebCore/layout/displaytree/DisplayBox.h:167
> > +    struct EdgePair {
> > +        LayoutUnit first;
> > +        LayoutUnit second;
> > +    };
> > +    using HorizontalEdges = EdgePair;
> 
> Could EdgePair be defined as:
> 
> using EdgePair = std::pair<LayoutUnit, LayoutUnit>;
> 
> Though, actually, I think what might be the cleanest is the really verbose:
> 
> struct HorizontalEdges {
>     LayoutUnit left;
>     LayoutUnit right;
> };
> struct VerticalEdges {
>     LayoutUnit top;
>     LayoutUnit bottom;
> };
> struct Edges {
>     HorizontalEdges horizontal;
>     VerticalEdges vertical;
> };
That's exactly what I had as my first version but I though some people might find it too verbose. Thanks for the comment! I will gladly make this change!

[alan]

Created attachment 341823 [details]
Patch

[Antti Koivisto]

Comment on attachment 341823 [details]
Patch

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

> Source/WebCore/ChangeLog:11
> +        Use 0 computed marings for now.

margins

[alan]

Committed r232465: <https://trac.webkit.org/changeset/232465>

[Radar WebKit Bug Importer]

<rdar://problem/40771393>