746296 - Check for antialiasing should be samples>1 not samples>0

Status: VERIFIED FIXED

(Core :: Graphics: CanvasWebGL, defect)

Description

[Benoit Jacob [:bjacob] (mostly away)]

Attachment: fix check for antialiasing

We're failing on

  https://cvs.khronos.org/svn/repos/registry/trunk/public/webgl/conformance-suites/1.0.1/conformance/context/context-attributes-alpha-depth-stencil-antialias.html

on mesa 8 + intel driver, while we are passing on mesa 7.11 + r600g driver. Neither support MSAA.

The failure is when getContextAttributes returns antialias=true despite antialiasing not being effectively performed.

Our test is samples>0. Maybe samples>1 is the better test to perform? Indeed, for practical purposes, samples=1 seems undistinguishable from samples=0.

Comment 1

[Kelsey Gilbert [:jgilbert]]

Comment on attachment 615837 [details] [diff] [review]
fix check for antialiasing

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

Please add a comment that while samples=1 uses our multisampling render path, it does not qualify as antialiasing.

Comment 2

[Benoit Jacob [:bjacob] (mostly away)]

I wonder then. Shouldn't we just kill the samples=1 path and just tweak samples in the GL context creation:

  if (samples==1)
    samples=0;

Comment 3

[Benoit Jacob [:bjacob] (mostly away)]

I mean, samples=1 seems useless overhead and this path is hit in the real world on linux, apparently.

Comment 4

[Kelsey Gilbert [:jgilbert]]

(In reply to Benoit Jacob [:bjacob] from comment #2)
> I wonder then. Shouldn't we just kill the samples=1 path and just tweak
> samples in the GL context creation:
> 
>   if (samples==1)
>     samples=0;

Filed bug 746328 for this.

Comment 5

[Benoit Jacob [:bjacob] (mostly away)]

Comment on attachment 615837 [details] [diff] [review]
fix check for antialiasing

Please approve for mozilla-central. Needed for WebGL conformance on real-world drivers. Very safe. Only affects WebGL, which is not a priority for the upcoming Android release.

Comment 6

[Benoit Jacob [:bjacob] (mostly away)]

http://hg.mozilla.org/integration/mozilla-inbound/rev/7b59d08bd69a

Comment 7

[(no longer active)]

https://hg.mozilla.org/mozilla-central/rev/7b59d08bd69a

Comment 8

[alex_mayorga]

https://cvs.khronos.org/svn/repos/registry/trunk/public/webgl/conformance-suites/1.0.1/conformance/context/context-attributes-alpha-depth-stencil-antialias.html now passes on Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:14.0) Gecko/20120424 Firefox/14.0a1 ID:20120424034314

Adapter Description nouveau -- Gallium 0.4 on NVA5
Vendor ID nouveau
Device ID Gallium 0.4 on NVA5
Driver Version 2.1 Mesa 8.0.2
WebGL Renderer nouveau -- Gallium 0.4 on NVA5 -- 2.1 Mesa 8.0.2
GPU Accelerated Windows 0
AzureBackends kia