Last Comment Bug 746296 - Check for antialiasing should be samples>1 not samples>0
: Check for antialiasing should be samples>1 not samples>0
Status: VERIFIED FIXED
webgl-conformance
:
Product: Core
Classification: Components
Component: Canvas: WebGL (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla14
Assigned To: Benoit Jacob [:bjacob] (mostly away)
:
Mentors:
Depends on:
Blocks: 746328
  Show dependency treegraph
 
Reported: 2012-04-17 13:04 PDT by Benoit Jacob [:bjacob] (mostly away)
Modified: 2012-04-24 16:03 PDT (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
fix check for antialiasing (1.43 KB, patch)
2012-04-17 13:04 PDT, Benoit Jacob [:bjacob] (mostly away)
jgilbert: review+
akeybl: approval‑mozilla‑central+
Details | Diff | Review

Description Benoit Jacob [:bjacob] (mostly away) 2012-04-17 13:04:56 PDT
Created attachment 615837 [details] [diff] [review]
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 Jeff Gilbert [:jgilbert] 2012-04-17 13:52:25 PDT
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) 2012-04-17 13:55:29 PDT
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) 2012-04-17 13:56:05 PDT
I mean, samples=1 seems useless overhead and this path is hit in the real world on linux, apparently.
Comment 4 Jeff Gilbert [:jgilbert] 2012-04-17 14:22:38 PDT
(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) 2012-04-19 19:21:26 PDT
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) 2012-04-19 20:37:25 PDT
http://hg.mozilla.org/integration/mozilla-inbound/rev/7b59d08bd69a
Comment 7 :Ehsan Akhgari (busy, don't ask for review please) 2012-04-20 10:40:41 PDT
https://hg.mozilla.org/mozilla-central/rev/7b59d08bd69a
Comment 8 alex_mayorga 2012-04-24 16:03:50 PDT
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

Note You need to log in before you can comment on or make changes to this bug.