Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove gl.lineWidth #5541

Merged
merged 1 commit into from
Jun 5, 2018
Merged

Remove gl.lineWidth #5541

merged 1 commit into from
Jun 5, 2018

Conversation

lbud
Copy link
Contributor

@lbud lbud commented Oct 27, 2017

This PR removes the use of gl.lineWidth in the few places where we use it: the MDN doc for WebGLRenderingContext.lineWidth() states

gl.lineWidth() has no effect in common modern browsers. The corresponding Chromium bug and Firefox bug are closed as Won't Fix and the WebGL specification now defines that gl.lineWidth() does not change the line width anymore. If you need lines of any width other than 1px, consider drawing a narrow strip of triangles.

@jfirebaugh
Copy link
Contributor

Refs #2080. The only reason I'm hesitant to approve this is http://webglstats.com/webgl/parameter/ALIASED_LINE_WIDTH_RANGE:

image

I.e., something happened recently that appears to have increased >1 line width support to 70%+ (although declining since then).

@lbud
Copy link
Contributor Author

lbud commented Jun 5, 2018

It sounds like WebGL Stats gets its data by embedding a script in websites that have chosen to participate (specifically these). Given that MDN's lineWidth compatibility table shows only IE12 as supporting this, I have to wonder if that chart only indicates that in mid-2017 a new site with a lot of IE users opted in to sharing stats. Given the caveats —

The webgl spec, based on the OpenGL ES 2.0/3.0 specs points out that the minimum and maximum width for a line is implementation defined. The maximum minimum width is allowed to be 1.0. The minimum maximum width is also allowed to be 1.0. Because of these implementation defined limits it is not recommended to use line widths other than 1.0 since there is no guarantee any user's browser will display any other width.

As of January 2017 most implementations of WebGL only support a minimum of 1 and a maximum of 1 as the technology they are based on has these same limits.

(source: MDN)

Professional applications should not use gl.lineWidth. First of all, there is zero guarantee that wide lines are supported on all WebGL implementations. So you must have a fallback anyway. Secondly, wide lines can be implemented by the driver in a variety of ways that are spec compliant (or not). To make sure they look the same across devices, polygons should be used instead. Thirdly, it is likely that a driver implements wide lines using polygons anyway. So there's no performance advantage. It has been deprecated from core desktop OpenGL for these reasons.

(source: a Swiftshader maintainer's comment on the Chromium issue, marked Wontfix)

Given what we know (and the fact that rendering antialiased polygons with different line widths based on whether or not they're on IE12 seems like a suboptimal experience) I think we may as well merge this — @jfirebaugh what do you think? I'll rebase + merge if you agree.

@lbud lbud merged commit b9e32bf into master Jun 5, 2018
@lbud lbud deleted the rm-linewidth branch June 5, 2018 23:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants