Bug 149764 - [CSS Grid Layout] Don't need to reset auto-margins during grid items layout
Summary: [CSS Grid Layout] Don't need to reset auto-margins during grid items layout
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Javier Fernandez
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-10-02 15:03 PDT by Javier Fernandez
Modified: 2015-10-06 12:24 PDT (History)
9 users (show)

See Also:


Attachments
Patch (14.08 KB, patch)
2015-10-02 16:32 PDT, Javier Fernandez
no flags Details | Formatted Diff | Diff
Patch (14.09 KB, patch)
2015-10-06 10:49 PDT, Javier Fernandez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Javier Fernandez 2015-10-02 15:03:37 PDT
When dealing with auto-margins alignment we were reseting auto values for margins, when grid items don't need to be laid out, just before using them to set the grid items location. 

This operation can be avoided if we just compute them, when required, during the execution of the stretching logic. 

This change could help us to avoid performing extra layout calls on some grid items.
Comment 1 Javier Fernandez 2015-10-02 16:32:06 PDT
Created attachment 262361 [details]
Patch
Comment 2 Darin Adler 2015-10-06 09:22:10 PDT
Comment on attachment 262361 [details]
Patch

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

> Source/WebCore/rendering/RenderGrid.cpp:1505
> +    const RenderStyle* parentStyle = &style();

Better to leave this as a reference and put the & at each call site. That helps us in the future when we change the functions to take references instead of pointers.

> Source/WebCore/rendering/RenderGrid.cpp:1527
> +    const RenderStyle* parentStyle = &style();

Ditto.

> Source/WebCore/rendering/RenderGrid.cpp:1652
> +        // In order to properly adjust the Self Alignment values we need to consider the offset between tracks.

Not sure why it’s good to capitalize the words “Self Alignment”.
Comment 3 Javier Fernandez 2015-10-06 10:49:17 PDT
Created attachment 262524 [details]
Patch
Comment 4 WebKit Commit Bot 2015-10-06 12:24:23 PDT
Comment on attachment 262524 [details]
Patch

Clearing flags on attachment: 262524

Committed r190633: <http://trac.webkit.org/changeset/190633>
Comment 5 WebKit Commit Bot 2015-10-06 12:24:27 PDT
All reviewed patches have been landed.  Closing bug.