790205 - browser.newtab.preload=true causes excessive invalidation

Status: RESOLVED DUPLICATE of bug 786484

(Firefox :: General, defect)

Description

[Jared Wein [:jaws] (please needinfo? me)]

We can remove the opacity:.3 style on the invalid identity-block icon and replace it with a precomputed translucent icon. This will reduce some extra drawing commands.

Comment 1

[Jared Wein [:jaws] (please needinfo? me)]

We also should move these icons to a single sprite for perf reasons.

Comment 2

[Dão Gottwald [:dao]]

What kind of drawing commands? What exactly is the overhead and when does it kick in? Let's not micro-optimize at the cost of bloating code and increasing maintenance costs without the wins identified very clearly as being worthwhile.

Comment 3

[Jared Wein [:jaws] (please needinfo? me)]

Bas has a tool that is showing the drawing commands. The opacity of this icon is causing the whole toolbar to be cleared and redrawn when the icon opacity gets applied.

Comment 4

[Dão Gottwald [:dao]]

This sounds like a bug. Will bug 539356 fix this? Some other bug that's already filed? It's generally preferable to get the root cause fixed in such cases, as they'll likely affect web content too.

Comment 5

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Patch

Bug 539356 may fix it but it's not done yet. I agree that fixing the root cause is preferable, but I'm investigating ways that we can be more optimal given our current environment.

Comment 6

[Dão Gottwald [:dao]]

(In reply to Jared Wein [:jaws] from comment #5)
> Bug 539356 may fix it but it's not done yet.

AFAIK most parts landed and last I heard it was targeting Firefox 18. It would also be nice to verify whether it does or doesn't fix this, because if it doesn't, we'll need to file a bug on this.

Comment 7

[Dão Gottwald [:dao]]

Comment on attachment 660061 [details] [diff] [review]
Patch

(In reply to Jared Wein [:jaws] from comment #3)
> The opacity of this
> icon is causing the whole toolbar to be cleared and redrawn when the icon
> opacity gets applied.

Just tested this via nglayout.debug.paint_flashing and I don't see it.

Comment 8

[Jared Wein [:jaws] (please needinfo? me)]

There is an older (Sep 5) build of Bas' paint tool on his people.m.o website (http://people.mozilla.org/~bschouten/player2d.exe). This patch alone might not do the trick, but combined with bug 790225 we believe that it might fix the unnecessary clearing of the toolbars.

Bas says that he can see the extra paint flashing with this patch. We could merge this bug with bug 790225, but I'd like to keep the patches separate.

Comment 9

[Dão Gottwald [:dao]]

I mean I don't even see the toolbar being redrawn when the opacity changes without this patch, using the latest nightly.

Comment 10

[Jared Wein [:jaws] (please needinfo? me)]

To see the toolbar being redraw on today's Nightly, do the following:

1) Turn on paint flashing
2) Restore the window (not maximized)
3) Go to about:jared
4) See the toolbars flash
5) Hit CTRL+L to focus the location bar
6) Go to about:dao

Expected:
Only the identity icon and possibly the lucky charms should flicker.

Actual:
Both the tabs and navigation toolbar flicker.

Comment 11

[Jared Wein [:jaws] (please needinfo? me)]

Simpler steps to reproduce:

1) Turn on paint flashing
2) Give focus to the location bar
3) Type one letter to invalidate the address

Without the patch:
The toolbars will flicker.

With the patch:
Only the location bar will change colors.

Comment 12

[Dão Gottwald [:dao]]

(In reply to Jared Wein [:jaws] from comment #11)
> Simpler steps to reproduce:
> 
> 1) Turn on paint flashing
> 2) Give focus to the location bar
> 3) Type one letter to invalidate the address
> 
> Without the patch:
> The toolbars will flicker.
> 
> With the patch:
> Only the location bar will change colors.

I used these steps for comment 9. Still can't reproduce. Windows 7, hardware acceleration enabled.

Comment 13

[Jared Wein [:jaws] (please needinfo? me)]

I can make a screencast if you would like. I have force-enabled acceleration and it still happens on Windows 7.

Comment 14

[Dão Gottwald [:dao]]

(In reply to Jared Wein [:jaws] from comment #13)
> I can make a screencast if you would like.

Feel free, but this won't really help us move forward. I could make a screencast too :)

We need to somehow figure out what exactly is going wrong on your side. Can you reproduce it in a new profile?

Comment 15

[Dão Gottwald [:dao]]

I can't reproduce this with hardware acceleration disabled either.

Comment 16

[Dão Gottwald [:dao]]

Nor with a new profile. (The one I used before was mostly virgin anyway, though.)

Comment 17

[Dão Gottwald [:dao]]

I think we should probably back out bug 753448, since the problem it tries to fix (bug 752839) is already fixed independently by bug 716108.

Comment 18

[Dão Gottwald [:dao]]

Attachment: IRC log

Comment 19

[Jared Wein [:jaws] (please needinfo? me)]

Please do not morph this bug. Preloading the new tab page is about thumbnail loading, not tab animations.

Comment 20

[Jared Wein [:jaws] (please needinfo? me)]

Please file a new bug.

Comment 21

[Dão Gottwald [:dao]]

This bug has STR and the relevant background information and doesn't serve any other purpose anymore.

Comment 22

[Dão Gottwald [:dao]]

Oh, there's already bug 786484. Basically the same issue.