Closed Bug 1273752 Opened 4 years ago Closed 3 years ago

gl_PointSize not working in WebGL2 context

Categories

(Core :: Canvas: WebGL, defect)

46 Branch
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla52
Tracking Status
firefox51 --- fixed
firefox52 --- fixed

People

(Reporter: ralabate, Assigned: mtseng)

Details

(Keywords: correctness, testcase, Whiteboard: [gfx-noted])

Attachments

(2 files)

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.11; rv:46.0) Gecko/20100101 Firefox/46.0
Build ID: 20160502172042

Steps to reproduce:

First, create a WebGL2 context.  Next, write a vertex shader that sets gl_PointSize to a value besides one.


Actual results:

The point size will be set to one, no matter what.


Expected results:

In the same browser, with a WebGL 1.0 context, setting gl_PointSize in the vertex shader alters the rasterization of the point.
Perhaps it is missing the implementation of gl.enable(gl.PROGRAM_POINT_SIZE)?
Summary: gl_PointSize not working in WebGL2 context (missing gl.PROGRAM_POINT_SIZE) → gl_PointSize not working in WebGL2 context
Component: Untriaged → Canvas: WebGL
Product: Firefox → Core
Could you attach a testcase, please.
Flags: needinfo?(ralabate)
Keywords: testcase-wanted
This bug still lacks a testcase. Edwin, are you able to move this bug forward without one? If not then I suspect this bug will eventually get closed as incomplete.
Testcase that draws three points of a triangle using the webgl2 context.  Changing gl_PointSize in the vertex shader has no effect.
Also, this only occurs for me on OSX.  On Windows, I am able to successfully alter gl_PointSize in the vertex shader when using a "webgl2" context.
Bump. I also see this on OSX, not only for WebGL2 context but also WebGL one. 

Take a look at the following demo in FF and Chrome, which sets gl_PointSize in the vertex shader: in FF (on OSX 10.12, haven't tested Windows), the point size seems to be hardwired to 1.0, in Chrome the gl_PointSize value is honored (you can increase/decrease point size with the left/right key):

http://floooh.github.io/oryol/asmjs/PrimitiveTypes.html

For WebGL2:

https://floooh.github.io/oryol-webgl2/asmjs/PrimitiveTypes.html

As far as I can gather from the WebGL spec, neither WebGL nor WebGL2 have the GL_PROGRAM_POINT_SIZE render state, so I think writing to gl_PointSize in the vertex shader should be enough.
Assignee: nobody → mtseng
Looks like we didn't enable GL_PROGRAM_POINT_SIZE in gecko. If I forced enable it, the test case would works.
Comment on attachment 8794718 [details]
Bug 1273752 - Replace GL_VERTEX_PROGRAM_POINT_SIZE with GL_PROGRAM_POINT_SIZE.

https://reviewboard.mozilla.org/r/81056/#review80454

::: dom/canvas/WebGLContextValidate.cpp:870
(Diff revision 1)
>      if (gl->IsCompatibilityProfile()) {
>          // gl_PointSize is always available in ES2 GLSL, but has to be
>          // specifically enabled on desktop GLSL.
>          gl->fEnable(LOCAL_GL_VERTEX_PROGRAM_POINT_SIZE);
>  
>          /* gl_PointCoord is always available in ES2 GLSL and in newer desktop
>           * GLSL versions, but apparently not in OpenGL 2 and apparently not (due
>           * to a driver bug) on certain NVIDIA setups. See:
>           *   http://www.opengl.org/discussion_boards/ubbthreads.php?ubb=showflat&Number=261472
>           *
>           * Note that this used to cause crashes on old ATI drivers... Hopefully
>           * not a significant anymore. See bug 602183.
>           */
>          gl->fEnable(LOCAL_GL_POINT_SPRITE);
> +    } else if (gl->IsAtLeast(gl::ContextProfile::OpenGLCore, 330)) {
> +        gl->fEnable(LOCAL_GL_PROGRAM_POINT_SIZE);
>      }

Change this whole section out for:

if (gl->IsCompatibilityProfile()) {
  gl->fEnable(LOCAL_GL_POINT_SPRITE);
}

if (!gl->IsGLES()) {
  gl->fEnable(LOCAL_GL_PROGRAM_POINT_SIZE);
}

The values for VERTEX_PROGRAM_POINT_SIZE and PROGRAM_POINT_SIZE are the same. It's just a naming convention change, so let's prefer the more modern one.

We can drop the extensive comments, as IsCompat/!IsGLES are pretty self-explanitory.
Comment on attachment 8794718 [details]
Bug 1273752 - Replace GL_VERTEX_PROGRAM_POINT_SIZE with GL_PROGRAM_POINT_SIZE.

https://reviewboard.mozilla.org/r/81056/#review80504
Attachment #8794718 - Flags: review?(jgilbert) → review+
https://treeherder.mozilla.org/#/jobs?repo=try&revision=6038dc344c9c&selectedJob=28200560

Try looks good. There are some unexpected pass tests. I'll remove the fail-if flag.
Pushed by mtseng@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/020813458563
Replace GL_VERTEX_PROGRAM_POINT_SIZE with GL_PROGRAM_POINT_SIZE. r=jgilbert
https://hg.mozilla.org/integration/mozilla-inbound/rev/8502d0e88cf5
Fix unexpected pass tests. r=me
https://hg.mozilla.org/mozilla-central/rev/020813458563
https://hg.mozilla.org/mozilla-central/rev/8502d0e88cf5
Status: UNCONFIRMED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
Fix has arrived in Nightly, thanks! :)
Comment on attachment 8794718 [details]
Bug 1273752 - Replace GL_VERTEX_PROGRAM_POINT_SIZE with GL_PROGRAM_POINT_SIZE.

Approval Request Comment
[Feature/regressing bug #]: webgl2
[User impact if declined]:
[Describe test coverage new/current, TreeHerder]:
[Risks and why]: 
[String/UUID change made/needed]:
Attachment #8794718 - Flags: approval-mozilla-aurora?
Flags: needinfo?(ralabate)
Comment on attachment 8794718 [details]
Bug 1273752 - Replace GL_VERTEX_PROGRAM_POINT_SIZE with GL_PROGRAM_POINT_SIZE.

WebGL2 point size fix, let's uplift to 51 before the merge to beta.
Attachment #8794718 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.