997304 - Social/Loop panels are blank on my Windows machine

Status: VERIFIED FIXED

(Core :: Graphics, defect)

Description

[Maire Reavy [:mreavy]]

A Windows build that works on Mark B's machine doesn't show the UI for the call URL generation box on mine.  I thought it was blank, but it's actually white text on a white background.  The content is totally functional;  with careful clicking and copy/pasting, I could generate a URL and have a call.

----

Here's the output from my browser console window:

Could not read chrome manifest 'file:///C:/Users/mreavy/Desktop/firefox-loop4/firefox/chrome.manifest'.
mutating the [[Prototype]] of an object will cause your code to run very slowly; instead create the object with the correct initial [[Prototype]] value using Object.create Preferences.jsm:378
Could not read chrome manifest 'file:///C:/Users/mreavy/Desktop/firefox-loop4/firefox/browser/extensions/%7B972ce4c6-7e08-4474-a285-3208198ce6fd%7D/chrome.manifest'.
Yay! LoopService.js:39
MozSocialAPI injectController: unable to attachToWindow for chrome://global/content/commonDialog.xul: TypeError: containingBrowser is null MozSocialAPI.jsm:84
push.services.mozilla.com : server does not support RFC 5746, see CVE-2009-3555
value is not a non-null object browser.js:14747
docShell.QueryInterface(...).sessionHistory is null content-sessionStore.js:230
1397670442843	Services.HealthReport.HealthReporter	WARN	Saved state file does not exist.

1397670442844	Services.HealthReport.HealthReporter	WARN	No prefs data found.

MozSocialAPI injectController: unable to attachToWindow for chrome://browser/content/devtools/webconsole.xul: TypeError: containingBrowser is null

Comment 1

[Maire Reavy [:mreavy]]

I have a Win 7, Lenovo 520

Comment 2

[Dan Mosedale (:dmosedale, :dmose)]

You might experiment with turning on Net and CSS logging and see if anything else interesting shows up in the browser window (eg CSS failing to load or barfing in some weird way).

You can start the browser with a specific profile and the jsconsole at start time (so that it doesn't miss any warnings) with something like...

./firefox -P my-loop-profile -jsconsole

Comment 3

[Maire Reavy [:mreavy]]

I'm going to investigate this further.  If anyone sees this same problem on other systems, please let me know.

Comment 4

[Mark Banner (:standard8)]

I've just reproduced this on a Lenovo W520 running Windows 7 - however, I've reproduced it on Talkilla (a social provider), which suggests that it is something to do with the panels.

Moving to Firefox/SocialAPI as a first stab of where to put this, but I suspect it's a different FF issue.

Comment 5

[Mark Banner (:standard8)]

Also an issue on 31.0a2 (2014-04-29). Not an issue on 30.0b1 (the candidate beta build), nor on 29.0 release.

Requesting tracking, as this will break users with social providers installed that use panels - they won't be able to see any of the panel, even though the elements are still clickable.

Comment 6

[Mark Hammond [:markh] [:mhammond]]

FWIW, panels WFM on Windows 7 - both loop's and social's

Comment 7

[Mark Banner (:standard8)]

Ok, this works for me in safe mode, but disabling all my add-ons doesn't fix it. Weird.

Maire, can you see if it works in safe mode please?

Comment 8

[Mark Banner (:standard8)]

I've reproduced this again on nightly builds, and it turns out that setting gfx.direct2d.disabled to true and restarting, fixes the issue.

Hence moving to core/graphics as this looks more like a graphics issue.

Comment 9

[Mark Banner (:standard8)]

I have a Lenovo W520 ThinkPad running Windows 7, here's the graphics driver info:

Graphics
--------

Adapter Description: Intel(R) HD Graphics Family
Adapter Description (GPU #2): NVIDIA Quadro 1000M
Adapter Drivers: igdumd64 igd10umd64 igd10umd64 igdumdx32 igd10umd32 igd10umd32
Adapter Drivers (GPU #2): nvd3dumx,nvwgf2umx,nvwgf2umx nvd3dum,nvwgf2um,nvwgf2um
Adapter RAM: Unknown
Adapter RAM (GPU #2): 2048
Device ID: 0x0126
Device ID (GPU #2): 0x0dfa
DirectWrite Enabled: false (6.2.9200.16571)
Driver Date: 3-6-2011
Driver Date (GPU #2): 5-25-2011
Driver Version: 8.15.10.2321
Driver Version (GPU #2): 8.17.12.6871
GPU #2 Active: false
GPU Accelerated Windows: 0/1 Basic (OMTC)
Vendor ID: 0x8086
Vendor ID (GPU #2): 0x10de
WebGL Renderer: Google Inc. -- ANGLE (Intel(R) HD Graphics Family Direct3D9Ex vs_3_0 ps_3_0)
windowLayerManagerRemote: true
AzureCanvasBackend: skia
AzureContentBackend: cairo
AzureFallbackCanvasBackend: cairo
AzureSkiaAccelerated: 0

Comment 10

[Sylvestre Ledru [:Sylvestre]]

Milan, do you have someone in mind who could help here? Thanks

Comment 11

[Milan Sreckovic [:milan] (needinfo for best results)]

Regression range would help, and I'm trying to see if we have access to the machine you mention.  In the meantime, it sounds a lot like bug 1014378, except that OMTC may have made it worse.

Comment 12

[Maire Reavy [:mreavy]]

For anyone experiencing this problem, we have a workaround.  We will want this fixed for MVP.

Comment 13

[Mark Banner (:standard8)]

Coarse regression range:

Last Good: 690c810c8e3e (2014-04-10)
First Bad: d8c1b10c3a3d (2014-04-11)
Pushlog: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=690c810c8e3e&tochange=d8c1b10c3a3d

For some reason, the script isn't working for the fine regression range on mozilla-inbound, I suspect the builds aren't available. The windows machine I have has not enough capacity to build.

Comment 14

[Sylvestre Ledru [:Sylvestre]]

Milan, does the regression window help you? Thanks

Comment 15

[Milan Sreckovic [:milan] (needinfo for best results)]

Regression window should help a lot - Bas, there is Nical's and Matt's changes in here, can you coordinate with them to see if we can find which one causes this? I'm still hoping all the Intel graphics issues are the same underlying issue under the covers, which is why I'm going after each one that comes in :)

Comment 16

[Bas Schouten (:bas.schouten)]

Can you guys try updating the Intel driver and seeing if the issue persist? We should find a range of intel drivers to blacklist from Direct2D.

In the meanwhile, Matt, you have one of these exact machines, any chance you could check if with some effort you can reproduce these sort of issues?

Comment 17

[Mark Banner (:standard8)]

I've tried updating the intel driver from the website, but it says I can't install it due to having a custom driver already installed.

So I took a look on getting an update from Lenovo, and the only graphics driver they had updated the NVIDIA driver but not the intel one :-( The relevant lines now look like:

Driver Date: 3-6-2011
Driver Date (GPU #2): 10-28-2013
Driver Version: 8.15.10.2321
Driver Version (GPU #2): 9.18.13.1269

Comment 19

[Mark Banner (:standard8)]

Hardware info from bug 1029388 - this seems to be a more up-to-date driver but still having the same issue:

Adapter Description	Intel(R) HD Graphics 3000
Adapter Drivers	igdumd64 igd10umd64 igd10umd64 igdumd32 igd10umd32 igd10umd32
Adapter RAM	Unknown
Device ID	0x0126
Direct2D Enabled	true
DirectWrite Enabled	true (6.2.9200.16571)
Driver Date	1-29-2014
Driver Version	9.17.10.3347
GPU #2 Active	false
GPU Accelerated Windows	1/1 Direct3D 10
Vendor ID	0x8086
WebGL Renderer	Google Inc. -- ANGLE (Intel(R) HD Graphics 3000 Direct3D9Ex vs_3_0 ps_3_0)
windowLayerManagerRemote	false
AzureCanvasBackend	direct2d
AzureContentBackend	direct2d
AzureFallbackCanvasBackend	cairo
AzureSkiaAccelerated	0

Comment 20

[Bas Schouten (:bas.schouten)]

(In reply to Mark Banner (:standard8) from comment #19)
> Hardware info from bug 1029388 - this seems to be a more up-to-date driver
> but still having the same issue:
> 
> Adapter Description	Intel(R) HD Graphics 3000
> Adapter Drivers	igdumd64 igd10umd64 igd10umd64 igdumd32 igd10umd32 igd10umd32
> Adapter RAM	Unknown
> Device ID	0x0126
> Direct2D Enabled	true
> DirectWrite Enabled	true (6.2.9200.16571)
> Driver Date	1-29-2014
> Driver Version	9.17.10.3347
> GPU #2 Active	false
> GPU Accelerated Windows	1/1 Direct3D 10
> Vendor ID	0x8086
> WebGL Renderer	Google Inc. -- ANGLE (Intel(R) HD Graphics 3000 Direct3D9Ex
> vs_3_0 ps_3_0)
> windowLayerManagerRemote	false
> AzureCanvasBackend	direct2d
> AzureContentBackend	direct2d
> AzureFallbackCanvasBackend	cairo
> AzureSkiaAccelerated	0

I'm assuming you've disabled OMTC, but are having this issue when you have OMTC enabled?

Comment 21

[Pavel Cvrcek [:JasnaPaka]]

No, I have OMTC enabled (layers.offmainthreadcomposition.enabled=true). It is all about HWA enabled/disabled. With HWA disabled all works fine, problem is just when I have HWA enabled.

Comment 22

[Bas Schouten (:bas.schouten)]

(In reply to Pavel Cvrcek (Mozilla.cz) [:JasnaPaka] from comment #21)
> No, I have OMTC enabled (layers.offmainthreadcomposition.enabled=true). It
> is all about HWA enabled/disabled. With HWA disabled all works fine, problem
> is just when I have HWA enabled.

How about if you specifically disabled Direct2D though? gfx.direct2d.disabled -> true

Comment 23

[Pavel Cvrcek [:JasnaPaka]]

(In reply to Bas Schouten (:bas.schouten) from comment #22)
> How about if you specifically disabled Direct2D though?
> gfx.direct2d.disabled -> true

It solves this problem for me.

Comment 24

[Matt Woodrow (:mattwoodrow)]

Ok, I can reproduce this locally by installing talkilla and using the steps suggested in bug 1029388. I suspect that I wasn't able to reproduce this earlier because 'social/loop panels' might not mean the same as adding a social sidebar.

Bisected locally to https://hg.mozilla.org/mozilla-central/rev/3a2528da6fe8

Comment 25

[Matt Woodrow (:mattwoodrow)]

When we have Direct2D enabled, but are drawing to a BasicLayerManager then we end up using a gdi/cairo DrawTarget for layers drawing (which is what is happening for these popups).

We still create our Mask Layers in direct2d format though, since we only take the platform default into account.

We trying to draw the mask, we have to readback and SourceSurfaceD2D::GetAsImageSurface is failing here:
http://mxr.mozilla.org/mozilla-central/source/gfx/2d/SourceSurfaceD2D.cpp#184

Not sure if this is actually intel/w520 specific or if the STR were misinterpreted by other people too.

Bas: Any idea why this would fail? I guess DrawBitmap with an A8 surface doesn't make much sense.

I can try come up with a patch to get us using the right mask to begin with, but it probably won't be easy to uplift.

Comment 26

[Matt Woodrow (:mattwoodrow)]

(In reply to Matt Woodrow (:mattwoodrow) from comment #25)
 
> I can try come up with a patch to get us using the right mask to begin with,
> but it probably won't be easy to uplift.

Actually, the relevant differences have made it to beta, so we should be fine for an uplift.

Comment 27

[Matt Woodrow (:mattwoodrow)]

We also had a gfxWarning() print for this, should we be trying to make those more noticeable somehow?

Comment 28

[Bas Schouten (:bas.schouten)]

(In reply to Matt Woodrow (:mattwoodrow) from comment #25)
> When we have Direct2D enabled, but are drawing to a BasicLayerManager then
> we end up using a gdi/cairo DrawTarget for layers drawing (which is what is
> happening for these popups).
> 
> We still create our Mask Layers in direct2d format though, since we only
> take the platform default into account.
> 
> We trying to draw the mask, we have to readback and
> SourceSurfaceD2D::GetAsImageSurface is failing here:
> http://mxr.mozilla.org/mozilla-central/source/gfx/2d/SourceSurfaceD2D.cpp#184
> 
> Not sure if this is actually intel/w520 specific or if the STR were
> misinterpreted by other people too.
> 
> Bas: Any idea why this would fail? I guess DrawBitmap with an A8 surface
> doesn't make much sense.
> 
> I can try come up with a patch to get us using the right mask to begin with,
> but it probably won't be easy to uplift.

I will make a patch for you to test that should at least fix the bug, this is awesome work, thanks!

Comment 29

[Bas Schouten (:bas.schouten)]

Attachment: Properly allow readback of A8 SourceSurfaces with D2D

Does this patch solve the issue?

Comment 30

[Matt Woodrow (:mattwoodrow)]

Unfortunately it doesn't, because on further Moz2D conversions we not get SourceSurfaceD2DTarget instead of SourceSurfaceD2D.

We're now running into a different problem where the DataSourceSurface is fine but cairo_image_surface_create_for_data fails because of an invalid stride.

Looks like we've hit this before: http://mxr.mozilla.org/mozilla-central/source/gfx/cairo/cairo/src/cairo-d2d-surface.cpp#2628

I'm doing a rebuild to see if your patch fixes the original bug at the time of the regression.

Comment 31

[Matt Woodrow (:mattwoodrow)]

Comment on attachment 8447199 [details] [diff] [review]
Properly allow readback of A8 SourceSurfaces with D2D

Review of attachment 8447199 [details] [diff] [review]:
-----------------------------------------------------------------

Patch basically works, but then we just run into the same stride issue.

::: gfx/2d/SourceSurfaceD2D.cpp
@@ +183,5 @@
> +                             Float(mSize.height)));
> +  } else {
> +    RefPtr<ID2D1SolidColorBrush> brush;
> +    renderTarget->CreateSolidColorBrush(D2D1::ColorF(D2D1::ColorF::White), byRef(brush));
> +    renderTarget->FillOpacityMask(aSourceSurface->mBitmap, brush, D2D1_OPACITY_MASK_CONTENT_GRAPHICS);

We need to add a call to renderTarget->SetAntialiasMode(D2D1_ANTIALIAS_MODE_ALIASED) for this to work.

Comment 32

[Bas Schouten (:bas.schouten)]

(In reply to Matt Woodrow (:mattwoodrow) from comment #30)
> Unfortunately it doesn't, because on further Moz2D conversions we not get
> SourceSurfaceD2DTarget instead of SourceSurfaceD2D.
> 
> We're now running into a different problem where the DataSourceSurface is
> fine but cairo_image_surface_create_for_data fails because of an invalid
> stride.
> 
> Looks like we've hit this before:
> http://mxr.mozilla.org/mozilla-central/source/gfx/cairo/cairo/src/cairo-d2d-
> surface.cpp#2628
> 
> I'm doing a rebuild to see if your patch fixes the original bug at the time
> of the regression.

Let's make the caller of cairo_image_surface_create_for_data copy into a surface with the right stride if (stride % 4).

Comment 33

[Matt Woodrow (:mattwoodrow)]

Attachment: Copy the data if it's an invalid stride

Comment 34

[Bas Schouten (:bas.schouten)]

Comment on attachment 8447812 [details] [diff] [review]
Copy the data if it's an invalid stride

Review of attachment 8447812 [details] [diff] [review]:
-----------------------------------------------------------------

::: gfx/2d/DrawTargetCairo.cpp
@@ +144,5 @@
> +                       int32_t aStride,
> +                       int32_t aPixelWidth)
> +{
> +  unsigned char* surfData = cairo_image_surface_get_data(aSurface);
> +  int surfStride = cairo_image_surface_get_stride(aSurface);

nit: Some vertical whitespace in this function would be nice. I'm also not sure we need to repeat this comment every time :-)

@@ +232,5 @@
> +      // a new surface with a stride that cairo chooses.
> +      return CopyToImageSurface(data->GetData(),
> +                                data->GetSize(),
> +                                data->Stride(),
> +                                data->GetFormat());

Document that in this case we can forgo on setting the user data because we no longer need to keep the data alive.

Comment 35

[Matt Woodrow (:mattwoodrow)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/f64447c5b17f
https://hg.mozilla.org/integration/mozilla-inbound/rev/c9c8c4ae8e0f

Comment 36

[Carsten Book [:Tomcat]]

sorry had to back out https://hg.mozilla.org/integration/mozilla-inbound/rev/c9c8c4ae8e0f for bustage like https://tbpl.mozilla.org/php/getParsedLog.php?id=42828585&tree=Mozilla-Inbound

Comment 37

[Matt Woodrow (:mattwoodrow)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/db4f6e215872

Comment 38

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/f64447c5b17f
https://hg.mozilla.org/mozilla-central/rev/db4f6e215872

Comment 39

[Mark Banner (:standard8)]

I've just tried this on my machine with one of the tinderbox builds and it works great. Thanks!

Comment 40

[Sylvestre Ledru [:Sylvestre]]

Bas, Matt: Do you think we should uplift that in 32 (currently aurora)? Thanks

Comment 41

[Bas Schouten (:bas.schouten)]

I think it'd be a pretty safe uplift.

Comment 42

[Matt Woodrow (:mattwoodrow)]

Comment on attachment 8447812 [details] [diff] [review]
Copy the data if it's an invalid stride

Approval Request Comment
[Feature/regressing bug #]: Bug 993784
[User impact if declined]: Broken panels (with rounded corners) on windows 7+ with D2D
[Describe test coverage new/current, TBPL]: No automated test coverage for popups, tested locally and verified by reporter.
[Risks and why]: Very low risk, just copying data to fix alignment.
[String/UUID change made/needed]: None.

Comment 43

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/6cb4f604213b
https://hg.mozilla.org/releases/mozilla-aurora/rev/002aee8237a3

Comment 44

[[SV Manager] Florin Mezei, QA (:FlorinMezei)]

This does not seem to have made it in Firefox 31 Beta (RC).
Marking as Verified on Firefox 33 based on comment 39.

Comment 45

[Sylvestre Ledru [:Sylvestre]]

Indeed. It was not. Too late in the cycle but we could take a fix for the ESR.

Comment 46

[Robert Kaiser]

The uplift of this to 32 caused bug 1035168 to appear there as well. :(

Comment 47

[Cornel Ionce [:noni] [Hubs QA]]

Mozilla/5.0 (Windows NT 6.1; WOW64; rv:32.0) Gecko/20100101 Firefox/32.0

Verified fixed on Firefox 32 beta 8, build ID: 20140818191513 using Talkilla.

Comment 48

[Lukas Blakk [:lsblakk] use ?needinfo]

Without being requested from the ESR community, this doesn't meet ESR landing criteria.