Summary: | Remove dependency on LayoutTypes.h from transformation code | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Emil A Eklund <eae> | ||||||||||||
Component: | Platform | Assignee: | Emil A Eklund <eae> | ||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||
Severity: | Normal | CC: | eric, jamesr, leviw, webkit.review.bot | ||||||||||||
Priority: | P2 | ||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||
Hardware: | Unspecified | ||||||||||||||
OS: | Unspecified | ||||||||||||||
Bug Depends on: | |||||||||||||||
Bug Blocks: | 93050 | ||||||||||||||
Attachments: |
|
Description
Emil A Eklund
2012-08-02 15:50:30 PDT
Created attachment 156185 [details]
Patch
Created attachment 156187 [details]
Patch
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 We'll also have to change it not to use MAX_LAYOUT_UNIT... Created attachment 156198 [details]
Patch
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. Created attachment 156211 [details]
Patch
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. Created attachment 156427 [details]
Patch for landing
Comment on attachment 156427 [details] Patch for landing Clearing flags on attachment: 156427 Committed r124658: <http://trac.webkit.org/changeset/124658> All reviewed patches have been landed. Closing bug. |