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

VERIFIED FIXED in mozilla14

Status

()

Core
Canvas: WebGL
VERIFIED FIXED
5 years ago
5 years ago

People

(Reporter: bjacob, Assigned: bjacob)

Tracking

unspecified
mozilla14
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: webgl-conformance)

Attachments

(1 attachment)

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.
Attachment #615837 - Flags: review?(jgilbert)
(Assignee)

Updated

5 years ago
Whiteboard: webgl-conformance
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.
Attachment #615837 - Flags: review?(jgilbert) → review+
(Assignee)

Comment 2

5 years ago
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;
(Assignee)

Comment 3

5 years ago
I mean, samples=1 seems useless overhead and this path is hit in the real world on linux, apparently.
Blocks: 746328
(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.
(Assignee)

Comment 5

5 years ago
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.
Attachment #615837 - Flags: approval-mozilla-central?

Updated

5 years ago
Attachment #615837 - Flags: approval-mozilla-central? → approval-mozilla-central+
(Assignee)

Comment 6

5 years ago
http://hg.mozilla.org/integration/mozilla-inbound/rev/7b59d08bd69a
Assignee: nobody → bjacob
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla14
https://hg.mozilla.org/mozilla-central/rev/7b59d08bd69a
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED

Comment 8

5 years ago
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
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.