Closed Bug 1006198 Opened 6 years ago Closed 6 years ago

[e10s] Google Maps is upside down on OS X

Categories

(Core :: Canvas: WebGL, defect)

All
macOS
defect
Not set

Tracking

()

VERIFIED FIXED
mozilla32
Tracking Status
e10s + ---
firefox31 --- unaffected
firefox32 + verified

People

(Reporter: cpeterson, Assigned: mattwoodrow)

References

Details

(Keywords: regression, reproducible)

Attachments

(7 files)

STR:
1. Load the New Google Maps: https://www.google.com/maps/preview
2. The map loads as expected.
3. Now open a new e10s window and load Google Maps there.

RESULT:
The map image is upside down. See the attached screenshot.

This is a recent regression in the past couple days.
I bisected this regression to the following pushlog:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=b227a707080f&tochange=e2e1b19fcffc

This bug is probably a regression from WebGL bug 995745.
roc: does this upside down WebGL bug look like a direct regression from your fix for bug 995745 or did the bug 995745 just reveal an earlier regression that happened during period Google Maps was black on OMTC?
Component: Graphics: Layers → Canvas: WebGL
Flags: needinfo?(roc)
Keywords: reproducible
I suspect the latter.
Flags: needinfo?(roc)
(In reply to Chris Peterson (:cpeterson) from comment #0)
> RESULT:
> The map image is upside down. See the attached screenshot.
I can't reproduce, 32.0a1 (2014-05-06), Win 7 x64
Paul: Did you test Windows in an e10s window ("File > Open e10s Window") and/or with OMTC enabled (set "layers.offmainthreadcomposition.enabled" pref to true)? I can still reproduce with Nightly 32 (2014-05-06) on OS X.
Flags: needinfo?(paul.silaghi)
(In reply to Chris Peterson (:cpeterson) from comment #5)
> Paul: Did you test Windows in an e10s window ("File > Open e10s Window")
> and with OMTC enabled (set "layers.offmainthreadcomposition.enabled" pref
> to true)?
Exactly, but on Win 7 x64
Flags: needinfo?(paul.silaghi)
Also not repro on Ubuntu, but repro on OS X 10.9.2
OS: All → Mac OS X
Summary: [e10s] Google Maps tiles are upside down → [e10s] Google Maps is upside down on OS X
Assignee: nobody → matt.woodrow
Attachment #8419957 - Flags: review?(nical.bugzilla)
Attachment #8419959 - Flags: review?(nical.bugzilla)
Attachment #8419962 - Flags: review?(jgilbert)
Basically the same as the last time we did this, but for the moz2d version.
Attachment #8419963 - Flags: review?(jgilbert)
Attachment #8419964 - Flags: review?(jgilbert)
Attachment #8419957 - Flags: review?(nical.bugzilla) → review+
Comment on attachment 8419959 [details] [diff] [review]
Add PremultiplyDataSurface

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

::: gfx/thebes/gfxUtils.cpp
@@ +89,5 @@
>      }
>  }
>  
>  void
> +gfxUtils::PremultiplyDataSurface(DataSourceSurface *aSurface)

What's the difference between this PremultiplyDataSurface, and the original PremultiplySurface with srcSurface == destSurface ?

Ah, on my laptop, PremultiplySurface takes DataSourceSurfaces so it must have been Moz2dified while you were writing this. So if the motivation was to have a moz2d version then I think you can remove this function now.

@@ +92,5 @@
>  void
> +gfxUtils::PremultiplyDataSurface(DataSourceSurface *aSurface)
> +{
> +    // Only premultiply ARGB32
> +    if (aSurface->GetFormat() != SurfaceFormat::B8G8R8A8) {

nit: We should assert here or print a warning if the format is not ARGB32.
(In reply to Nicolas Silva [:nical] from comment #15)
> Comment on attachment 8419959 [details] [diff] [review]
> Add PremultiplyDataSurface
> 
> Review of attachment 8419959 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: gfx/thebes/gfxUtils.cpp
> @@ +89,5 @@
> >      }
> >  }
> >  
> >  void
> > +gfxUtils::PremultiplyDataSurface(DataSourceSurface *aSurface)
> 
> What's the difference between this PremultiplyDataSurface, and the original
> PremultiplySurface with srcSurface == destSurface ?
> 
> Ah, on my laptop, PremultiplySurface takes DataSourceSurfaces so it must
> have been Moz2dified while you were writing this. So if the motivation was
> to have a moz2d version then I think you can remove this function now.

Nothing really, this is basically just moving it to gfxUtils instead of LayerUtils.

A later patch in the series gets rid of the duplication.
Attachment #8419962 - Flags: review?(jgilbert) → review+
Attachment #8419966 - Flags: review?(nical.bugzilla) → review+
Comment on attachment 8419959 [details] [diff] [review]
Add PremultiplyDataSurface

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

Canceling the review because if I understand correctly you don't need this anymore, don't hesitate to put the "r?" back if I was wrong.
Attachment #8419959 - Flags: review?(nical.bugzilla)
Comment on attachment 8419959 [details] [diff] [review]
Add PremultiplyDataSurface

Please look at the 'remove all the dead code' patch, since it removes all the duplicate code you're worried about.
Attachment #8419959 - Flags: review?(nical.bugzilla)
Attachment #8419959 - Flags: review?(nical.bugzilla) → review+
Attachment #8419963 - Flags: review?(jgilbert) → review+
Attachment #8419964 - Flags: review?(jgilbert) → review+
Looks ok in 32.0a1 (2014-05-15), OS X 10.8.5
Status: RESOLVED → VERIFIED
Meh. Some of the patches here duplicated the patch in bug 1001687.
You need to log in before you can comment on or make changes to this bug.