Bug 149764

Summary: [CSS Grid Layout] Don't need to reset auto-margins during grid items layout
Product: WebKit Reporter: Javier Fernandez <jfernandez>
Component: Layout and RenderingAssignee: Javier Fernandez <jfernandez>
Severity: Normal CC: commit-queue, darin, esprehn+autocc, glenn, hyatt, jfernandez, kondapallykalyan, rego, svillar
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Description Flags
Patch none

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]
Comment 2 Darin Adler 2015-10-06 09:22:10 PDT
Comment on attachment 262361 [details]

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();


> 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]
Comment 4 WebKit Commit Bot 2015-10-06 12:24:23 PDT
Comment on attachment 262524 [details]

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.