912551 - Antialiasing should be opt-in on mobile

Status: RESOLVED WONTFIX

(Core :: Graphics: CanvasWebGL, defect)

Description

[Marty Rosenberg [:mjrosenb]]

I tried to play http://hellorun.helloenjoy.com/ on my nexus 10 on nightly, and noticed that after the loading screen finished, we ran at about 1 fps.  My galaxy tab 2 seems to do much better, even with worse hardware.  Unfortunately, I cant test this under beta, since there seem to be rendering problems abound there.  I have not yet tried with aurora.  My app build id is 20130904030204, and it is running android 4.3.  Also, full disclosure, the game is incredibly crashy, so I frequently don't get more than a few seconds of bad performance.  I believe there is already a bug on file about the crashes.

Comment 1

[Kelsey Gilbert [:jgilbert]]

I'll take a look. I have both those pieces of hardware, so I'll see about reproducing this.

Comment 2

[James Willcox (:snorp) ([e-mail]) (he/him)]

I looked into WebGL slowness on Nexus 10 a bit today. Profile shows it spending a ton of time in GLScreenBuffer::AssureBlitted(). Not really sure why.

Comment 3

[Brad Grantham]

Has anyone tried this on a Note3?  It appears to run quite quickly for me on Note3, the same family of GPU.  I'll take a look next on Nexus 10; there could be a subtle hardware difference (different part number) or driver versioning issue.

Comment 4

[Brad Grantham]

FF 29.0 from Play Store, Android 4.3

Comment 5

[Brad Grantham]

On Nexus 10 verified the app is so slow it is unplayable.

Comment 6

[Kelsey Gilbert [:jgilbert]]

It's probably because we antialias for them on the Nexus 10.
After looking up the resolution of the Nexus 10, I'm not at all surprised: 2560×1600, with 2xAA (4-sample)

Disabling AA makes this playable again.

The Note3 could work well for a couple reasons:
* It has a lower resolution: 1920x1080 (half the pixels)
* It may only have a GLES2 driver, which means it won't get AA.
* ??? Dragons in the driver stack.

I think it's probably both of the first two that is making this more playable on the Note3.

The change we've been considering that could help here is making antialias:false the default for WebGL on mobile. (it defaults to true on desktop) Apps would still be able to specify antialias:true manually, but they would need to opt-in, instead of opt-out.

Comment 7

[James Willcox (:snorp) ([e-mail]) (he/him)]

Ah, I didn't know we were doing this. I agree that defaulting to antialias:false could be the way to go.

Comment 8

[Kelsey Gilbert [:jgilbert]]

Attachment: mobile-default-aa

Grabbing review from :kamidphish for the code, and :vlad for the concept w.r.t. games, etc.

Comment 9

[u480271]

Comment on attachment 8418398 [details] [diff] [review]
mobile-default-aa

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

LGTM if a little kludgey.

I recently learned that changed to WebIDL require review from DOM peers. Does this require that also?

Comment 10

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Comment on attachment 8418398 [details] [diff] [review]
mobile-default-aa

does ANDROID cover both android and FxOS?  Is it the right flag to use?  Should be fine, but I thought we had something that was moz specific.

Maybe #if defined(MOZ_WIDGET_GONK) || defined(MOZ_WIDGET_ANDROID) ?

Comment 11

[James Willcox (:snorp) ([e-mail]) (he/him)]

(In reply to Vladimir Vukicevic [:vlad] [:vladv] from comment #10)
> Comment on attachment 8418398 [details] [diff] [review]
> mobile-default-aa
> 
> does ANDROID cover both android and FxOS?  Is it the right flag to use? 
> Should be fine, but I thought we had something that was moz specific.
> 
> Maybe #if defined(MOZ_WIDGET_GONK) || defined(MOZ_WIDGET_ANDROID) ?

ANDROID does cover both, but I don't like it. I would much rather have something that clearly identifies Fennec || b2g.

Comment 12

[Kelsey Gilbert [:jgilbert]]

Attachment: for landing

r=kamidphish,vlad

Comment 13

[Kelsey Gilbert [:jgilbert]]

This touches IDL, so we need DOM approval. :\

Comment 14

[James Willcox (:snorp) ([e-mail]) (he/him)]

Comment on attachment 8419642 [details] [diff] [review]
for landing

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

::: dom/webidl/WebGLRenderingContext.webidl
@@ +37,5 @@
>      // If alpha isn't specified, we rely on a pref ("webgl.default-no-alpha")
>      boolean alpha;
>      boolean depth = true;
>      boolean stencil = false;
> +    boolean antialias;

should add a comment like we have for 'alpha' on how this gets set if there is no value specified.

Comment 15

[Kelsey Gilbert [:jgilbert]]

Attachment: for landing

Just needs single-line IDL review.
r=kamidphish,vlad so far

Comment 16

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8419674 [details] [diff] [review]
for landing

Could also put the ifdef in the webidl (and move this file to PREPROCESSED_WEBIDL_FILES), which would not require any C++ changes at all, but may require a clobber.

But more importantly, this is an explicit violation of the webgl spec as it's currently written.  Has a spec issue been raised already?

Comment 17

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8419674 [details] [diff] [review]
for landing

Please re-request review once the spec situation is clear...

Comment 18

[Kelsey Gilbert [:jgilbert]]

Alright, so the WG doesn't like my idea. Instead, we should probably just disable MSAA for these high-DPI devices, for the time being.

Comment 19

[u480271]

Can we make it opt-in on high-DPI?

Comment 20

[James Willcox (:snorp) ([e-mail]) (he/him)]

(In reply to Jeff Gilbert [:jgilbert] from comment #18)
> Alright, so the WG doesn't like my idea.

That's preposterous. Do they know who you are?

> Instead, we should probably just
> disable MSAA for these high-DPI devices, for the time being.

Sounds good to me. Let's do it. Where do you want to draw the line on "high DPI"? We could also tie it to GPU perhaps. Maybe there is some beast of a GPU out there that can do this with decent performance (K1?)

Comment 21

[Kelsey Gilbert [:jgilbert]]

(In reply to Dan Glastonbury :djg :kamidphish from comment #19)
> Can we make it opt-in on high-DPI?

I would love to do this. For now, disabling it is for the best.

Comment 22

[Kelsey Gilbert [:jgilbert]]

(In reply to Jeff Gilbert [:jgilbert] from comment #21)
> (In reply to Dan Glastonbury :djg :kamidphish from comment #19)
> > Can we make it opt-in on high-DPI?
> 
> I would love to do this. For now, disabling it is for the best.

Ok, got a little confused. This bug makes it opt-in for mobile. Bug 925530 disabled it for mobile.

I'm going to WONTFIX this until we bring it up for discussion again.