Skip to content
This repository was archived by the owner on Mar 19, 2021. It is now read-only.

Commit bdb1307

Browse files
emiliomoz-wptsync-bot
authored andcommitted
Refactor grid types to preserve repeat() at computed value time and use cbindgen.
I'm _really_ sorry for the size of the patch. I tried to do this in two steps but it was a lot of work and pretty ugly. This patch makes us use cbindgen for grid-template-{rows,columns}, in order to: * Make us preserve repeat() at computed-value time. This is per spec since interpolation needs to know about repeat(). Except for subgrid, which did the repeat expansion at parse-time and was a bit more annoying (plus it doesn't really animate yet so we don't need it to comply with the spec). * Tweaks the WPT tests for interpolation to adopt the resolution at: w3c/csswg-drafts#3503. Trade-off here, as this patch stands, is that this change makes us use less long-living memory, since we expand repeat() during layout, but at the cost of a bit of CPU time during layout (conditional on the property applying though, which wasn't the case before). It should be very easy to store a cached version of the template, should this be too hot (I expect it isn't), or to change the representation in other ways to optimize grid layout code if it's worth it. Another trade-off: I've used SmallPointerArray to handle line-name merging, pointing to the individual arrays in the style data, rather than actually heap-allocating the merged lists. This would also be pretty easy to change should we measure and see that it's not worth it. This patch also opens the gate to potentially improving memory usage in some other ways, by reference-counting line-name lists for example, though I don't have data that suggests it is worth it. In general, this patch makes much easier to tweak the internal representation of the grid style data structures. Overall, I think it's a win, the amount of magic going on in that mako code was a bit huge; it took a bit to wrap my head around it. This patch comments out the style struct size assertions. They will be uncommented in a follow-up patch which contains some improvements for this type, which are worth getting reviewed separately. Also, this patch doesn't remove as much code as I would've hoped for because of I tried not to change most of the dom/grid code for inspector, but I think a fair bit of the nsGridContainerFrame.cpp code that collects information for it can be simplified / de-copy-pasted to some extent. But that was a pre-existing problem and this patch is already quite massive. Differential Revision: https://phabricator.services.mozilla.com/D36598 bugzilla-url: https://bugzilla.mozilla.org/show_bug.cgi?id=1519958 gecko-commit: a1eb75e785ded866d66184c8b59b5a5b907a34fb gecko-integration-branch: autoland gecko-reviewers: mats, boris
1 parent 1013746 commit bdb1307

File tree

2 files changed

+7
-18
lines changed

2 files changed

+7
-18
lines changed

css/css-grid/animation/grid-template-columns-interpolation.html

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,7 @@
118118
]);
119119

120120
// Exercise <track-list> (with <track-repeat>)
121+
// https://drafts.csswg.org/css-grid/#repeat-interpolation
121122
test_no_interpolation({
122123
property: 'grid-template-columns',
123124
from: "1fr repeat(2, 2fr 30px) 1fr",
@@ -130,18 +131,11 @@
130131
to: "2fr repeat(3, 3fr 40px) 2fr"
131132
});
132133

133-
test_interpolation({
134+
test_no_interpolation({
134135
property: 'grid-template-columns',
135136
from: "repeat(2, 2fr 30px)",
136137
to: "repeat(4, 40px)"
137-
}, [
138-
{at: -1, expect: "2fr 20px 2fr 20px"},
139-
{at: 0, expect: "repeat(2, 2fr 30px)"},
140-
{at: 0.4, expect: "2fr 34px 2fr 34px"},
141-
{at: 0.6, expect: "40px 36px 40px 36px"},
142-
{at: 1, expect: "repeat(4, 40px)"},
143-
{at: 2, expect: "40px 50px 40px 50px"}
144-
]);
138+
});
145139

146140
test_interpolation({
147141
property: 'grid-template-columns',

css/css-grid/animation/grid-template-rows-interpolation.html

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,7 @@
118118
]);
119119

120120
// Exercise <track-list> (with <track-repeat>)
121+
// https://drafts.csswg.org/css-grid/#repeat-interpolation
121122
test_no_interpolation({
122123
property: 'grid-template-rows',
123124
from: "1fr repeat(2, 2fr 30px) 1fr",
@@ -130,18 +131,12 @@
130131
to: "2fr repeat(3, 3fr 40px) 2fr"
131132
});
132133

133-
test_interpolation({
134+
// See https://github.com/w3c/csswg-drafts/issues/3503
135+
test_no_interpolation({
134136
property: 'grid-template-rows',
135137
from: "repeat(2, 2fr 30px)",
136138
to: "repeat(4, 40px)"
137-
}, [
138-
{at: -1, expect: "2fr 20px 2fr 20px"},
139-
{at: 0, expect: "repeat(2, 2fr 30px)"},
140-
{at: 0.4, expect: "2fr 34px 2fr 34px"},
141-
{at: 0.6, expect: "40px 36px 40px 36px"},
142-
{at: 1, expect: "repeat(4, 40px)"},
143-
{at: 2, expect: "40px 50px 40px 50px"}
144-
]);
139+
});
145140

146141
test_interpolation({
147142
property: 'grid-template-rows',

0 commit comments

Comments
 (0)