1071645 - investigate: getting rid of _resizeGrid() and use CSS to determine the correct grid size

Status: RESOLVED FIXED

(Firefox :: New Tab Page, defect)

Description

[Tim Taubert [:ttaubert] (inactive)]

Not sure whether that is possible but we should investigate getting rid of the _resizeGrid() function that is called on load and when the content window is resized. Using JavaScript for layout is never a great idea although setting the right grid size almost only via CSS looks complicated.

The cell width, height, and margin can definitely be hard-coded. That's a small price to pay IMO for having a page that loads faster and doesn't hog the browser as much when resizing.

Comment 1

[Tomasz Kołodziejski [:tomasz]]

I'll work on it.

Comment 2

[Tomasz Kołodziejski [:tomasz]]

I did some tests and I have some numbers but they look extremely unreliable so they are not that useful. It looks like the only optimization I have played with that is really helping is scaling the images down to their right size.

I still have a plan for playing with css transforms to do the grid.


Those number are from TART's newtabLoadOnly.

reference:

Aggregated:
newtabLoadOnly.TART    Average (5): 495.88  Median: 492.7  stddev: 74.34
Individual animations:
newtabLoadOnly.TART: 583.4
newtabLoadOnly.TART: 422.4
newtabLoadOnly.TART: 492.7
newtabLoadOnly.TART: 423.5
newtabLoadOnly.TART: 557.4

(I tried this a couple of minues later with:)

Aggregated:
newtabLoadOnly.TART    Average (10): 360.44  Median: 348.4  stddev: 77.17
Individual animations:
newtabLoadOnly.TART: 405.4
newtabLoadOnly.TART: 286.9
newtabLoadOnly.TART: 468.0
newtabLoadOnly.TART: 296.2
newtabLoadOnly.TART: 474.3
newtabLoadOnly.TART: 288.4
newtabLoadOnly.TART: 394.2
newtabLoadOnly.TART: 283.9
newtabLoadOnly.TART: 404.7
newtabLoadOnly.TART: 302.5

float:left (12 tiles visible)

Aggregated:
newtabLoadOnly.TART    Average (5): 433.29  Median: 400.1  stddev: 56.52
Individual animations:
newtabLoadOnly.TART: 400.1
newtabLoadOnly.TART: 497.4
newtabLoadOnly.TART: 386.6
newtabLoadOnly.TART: 492.4
newtabLoadOnly.TART: 389.8

no transitions:

Aggregated:
newtabLoadOnly.TART    Average (5): 400.66  Median: 458.2  stddev: 91.55
Individual animations:
newtabLoadOnly.TART: 458.2
newtabLoadOnly.TART: 283.8
newtabLoadOnly.TART: 463.3
newtabLoadOnly.TART: 319.5
newtabLoadOnly.TART: 478.5

no transitions, no radiuses (is it legitimately faster?!)

Aggregated:
newtabLoadOnly.TART    Average (10): 354.41  Median: 352.3  stddev: 70.97
Individual animations:
newtabLoadOnly.TART: 272.7
newtabLoadOnly.TART: 382.3
newtabLoadOnly.TART: 289.5
newtabLoadOnly.TART: 469.6
newtabLoadOnly.TART: 257.2
newtabLoadOnly.TART: 412.8
newtabLoadOnly.TART: 331.2
newtabLoadOnly.TART: 373.5
newtabLoadOnly.TART: 325.0
newtabLoadOnly.TART: 430.4

images scaled down (low dpi), so much faster!

Aggregated:
newtabLoadOnly.TART    Average (5): 231.14  Median: 233.8  stddev: 11.09
Individual animations:
newtabLoadOnly.TART: 240.5
newtabLoadOnly.TART: 221.4
newtabLoadOnly.TART: 217.7
newtabLoadOnly.TART: 242.2
newtabLoadOnly.TART: 233.8

images scaled down + <img> instead of background url + no transitions + no round borders (comparable to just images scaled down):

Aggregated:
newtabLoadOnly.TART    Average (5): 259.89  Median: 251.1  stddev: 32.08
Individual animations:
newtabLoadOnly.TART: 280.1
newtabLoadOnly.TART: 234.0
newtabLoadOnly.TART: 304.9
newtabLoadOnly.TART: 251.1
newtabLoadOnly.TART: 229.3

Comment 3

[Tim Taubert [:ttaubert] (inactive)]

(In reply to Tomasz Kołodziejski [:tomasz] from comment #2)
> I did some tests and I have some numbers but they look extremely unreliable
> so they are not that useful. It looks like the only optimization I have
> played with that is really helping is scaling the images down to their right
> size.
> 
> I still have a plan for playing with css transforms to do the grid.

Resizing the window itself isn't actually what we should focus on. The important part is getting rid of the _resizeGrid() function that does sync layout using JavaScript. If that makes resizing a the browser window faster that's nice but getting rid of the sync layout on page load sounds a lot more interesting to me.

> images scaled down (low dpi), so much faster!

Yes, got the same results. We really should do that.

> images scaled down + <img> instead of background url + no transitions + no
> round borders (comparable to just images scaled down):

Yeah, using <img> didn't really pan out for me either when I tested.

Comment 4

[Tomasz Kołodziejski [:tomasz]]

Attachment: newtab-grid-css-translate.patch

This is a demo (only width resize works) rather that a patch. It uses css translate and js to do the layout. It has a drawback of using js for layout and an advantage of drawing all the tiles exactly once and just moving them around with css translation. Resize may work better on slow machines.

TART, however, does not report any speed improvements with this technique. But if anyone wants to give it a try on one of those slow machines, go ahead!

Comment 5

[Tomasz Kołodziejski [:tomasz]]

Attachment: newtab-grid-hard-coded.patch

I also did a demo with hard-coded cell widths/heights. It looks tiny better, maybe, but it requires all changes to margins/paddings/widths to modify this code. And so I'm not that sure that we want to merge this. Thoughts?

before:

Aggregated:
newtabLoadOnly.TART    Average (20): 381.91  Median: 385.0  stddev: 76.79
Individual animations:
newtabLoadOnly.TART: 334.4
newtabLoadOnly.TART: 473.1
newtabLoadOnly.TART: 333.3
newtabLoadOnly.TART: 522.8
newtabLoadOnly.TART: 310.5
newtabLoadOnly.TART: 375.8
newtabLoadOnly.TART: 312.6
newtabLoadOnly.TART: 430.6
newtabLoadOnly.TART: 300.3
newtabLoadOnly.TART: 394.2
newtabLoadOnly.TART: 400.0
newtabLoadOnly.TART: 477.2
newtabLoadOnly.TART: 294.3
newtabLoadOnly.TART: 493.3
newtabLoadOnly.TART: 297.9
newtabLoadOnly.TART: 414.7
newtabLoadOnly.TART: 294.4
newtabLoadOnly.TART: 430.6
newtabLoadOnly.TART: 295.4
newtabLoadOnly.TART: 452.7

after:

Aggregated:
newtabLoadOnly.TART    Average (20): 369.91  Median: 367.2  stddev: 79.83
Individual animations:
newtabLoadOnly.TART: 290.1
newtabLoadOnly.TART: 418.1
newtabLoadOnly.TART: 300.0
newtabLoadOnly.TART: 481.5
newtabLoadOnly.TART: 283.5
newtabLoadOnly.TART: 385.3
newtabLoadOnly.TART: 289.9
newtabLoadOnly.TART: 386.1
newtabLoadOnly.TART: 303.9
newtabLoadOnly.TART: 394.6
newtabLoadOnly.TART: 318.3
newtabLoadOnly.TART: 542.2
newtabLoadOnly.TART: 286.6
newtabLoadOnly.TART: 441.2
newtabLoadOnly.TART: 289.5
newtabLoadOnly.TART: 454.2
newtabLoadOnly.TART: 296.6
newtabLoadOnly.TART: 466.7
newtabLoadOnly.TART: 349.0
newtabLoadOnly.TART: 420.9

Comment 6

[Tomasz Kołodziejski [:tomasz]]

Attachment: newtab-grid-media-query.patch

It's another demo with media query to set the heights. The numbers:

Aggregated:
newtabLoadOnly.TART    Average (20): 363.30  Median: 378.0  stddev: 101.99
Individual animations:
newtabLoadOnly.TART: 384.1
newtabLoadOnly.TART: 372.0
newtabLoadOnly.TART: 274.5
newtabLoadOnly.TART: 666.1
newtabLoadOnly.TART: 260.9
newtabLoadOnly.TART: 384.8
newtabLoadOnly.TART: 263.2
newtabLoadOnly.TART: 360.8
newtabLoadOnly.TART: 320.3
newtabLoadOnly.TART: 432.4
newtabLoadOnly.TART: 268.6
newtabLoadOnly.TART: 383.9
newtabLoadOnly.TART: 263.9
newtabLoadOnly.TART: 386.2
newtabLoadOnly.TART: 522.8
newtabLoadOnly.TART: 384.1
newtabLoadOnly.TART: 261.1
newtabLoadOnly.TART: 385.9
newtabLoadOnly.TART: 265.4
newtabLoadOnly.TART: 424.8

This solution has another drawback of either having to add a lot more media queries or generate media queries on the fly for very high monitors. I don't think it buys us that much (if anything).

Comment 7

[Tomasz Kołodziejski [:tomasz]]

And once again the number for reference. It looks like the numbers are still very unreliable:

Aggregated:
newtabLoadOnly.TART    Average (20): 396.92  Median: 341.8  stddev: 321.00
Individual animations:
newtabLoadOnly.TART: 1742.6
newtabLoadOnly.TART: 251.6
newtabLoadOnly.TART: 382.6
newtabLoadOnly.TART: 259.6
newtabLoadOnly.TART: 376.5
newtabLoadOnly.TART: 263.6
newtabLoadOnly.TART: 401.5
newtabLoadOnly.TART: 361.8
newtabLoadOnly.TART: 347.1
newtabLoadOnly.TART: 254.8
newtabLoadOnly.TART: 359.9
newtabLoadOnly.TART: 272.7
newtabLoadOnly.TART: 362.4
newtabLoadOnly.TART: 336.4
newtabLoadOnly.TART: 355.4
newtabLoadOnly.TART: 260.6
newtabLoadOnly.TART: 358.1
newtabLoadOnly.TART: 265.6
newtabLoadOnly.TART: 391.9
newtabLoadOnly.TART: 333.4

Again it looks like it doesn't change anything.

Comment 8

[:Gavin Sharp [email: [e-mail]]]

Spoke to Tomasz on IRC, we're going to drop this bug for now.

Comment 9

[Marco Mucci [:MarcoM]]

Removed from IT 35.3

Comment 10

[Tomasz Kołodziejski [:tomasz]]

I think we can close it. Investigation has been done and the result was: it's not worth doing it as it does not give us any significant improvements.

Just for reference: the last idea was to use css media query to set the height of the container holding tiles. We'd have to either hard-code all the possible heights (pretty much map all the height intervals like [400, 600] to 400 which translates to: If there's not enough space for 3 tiles (say, we have 450px so just 50px for the next tile) then just show 2 tiles)) or generate media queries using js. These does not sound any better than the current approach.

The only worth investigating in the future is using css translate to move tiles around with no reflows. However, we currently don't support smooth transitions on window resize (see opera for example) so we can think about this in the future.

Comment 11

[:Gavin Sharp [email: [e-mail]]]

Good point - let's track this slightly differently. Forgot this was an investigation bug.