Bug 93037 – Remove dependency on LayoutTypes.h from transformation code

• Status: RESOLVED
• Resolution: FIXED
• Component: Platform

[Emil A Eklund]

TransformationMatrix includes LayoutTypes.h which is in rendering, this is a layering violation and makes using the it outside of rendering complicated.

[Emil A Eklund]

Created attachment 156185 [details]
Patch

[Emil A Eklund]

Created attachment 156187 [details]
Patch

[Levi Weintraub]

Comment on attachment 156187 [details]
Patch

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

> Source/WebCore/ChangeLog:9
> +        TransformationMatrix includes LayoutTypes.h which is in rendering,
> +        this is a layering violation. Remove this dependency as it is no longer

Grammar Levi says: This is a run on. Period or conjunction needed :p

[Emil A Eklund]

We'll also have to change it not to use MAX_LAYOUT_UNIT...

[Emil A Eklund]

Created attachment 156198 [details]
Patch

[Levi Weintraub]

Comment on attachment 156198 [details]
Patch

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

> Source/WebCore/platform/FractionalLayoutUnit.h:188
> +    static FractionalLayoutUnit clamp(double value)
> +    {
> +        return clampTo<FractionalLayoutUnit>(value, FractionalLayoutUnit::min(), FractionalLayoutUnit::max());
> +    }

I don't like the duplication of this code here and in LayoutTypes.h. Seems like if this is where this needs to live to prevent layering problems, we should eliminate the version in LayoutTypes.

[Emil A Eklund]

Created attachment 156211 [details]
Patch

[Levi Weintraub]

Comment on attachment 156211 [details]
Patch

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

> Source/WebCore/ChangeLog:16
> +        * rendering/LayoutTypes.h:
> +        Remove clampToLayoutUnit
> +        

We don't usually put whitespace here, and this could just be one line.

> Source/WebCore/ChangeLog:21
> +        Add FractionalLayoutUnit::clamp method that clamps a double to a
> +        FractionalLayoutUnit.
> +        

Ditto.

> Source/WebCore/ChangeLog:27
> +        Use clamp/max/min from FractionalLayoutUnit instead of going through
> +        LayoutUnit abstraction.
> +        

Ditto.

[Emil A Eklund]

Created attachment 156427 [details]
Patch for landing

[WebKit Review Bot]

Comment on attachment 156427 [details]
Patch for landing

Clearing flags on attachment: 156427

Committed r124658: <http://trac.webkit.org/changeset/124658>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.