Closed
Bug 1111689
Opened 10 years ago
Closed 8 years ago
add support for ARB_shader_texture_lod to implement WebGL EXT_shader_texture_lod
Categories
(Core :: Graphics: CanvasWebGL, defect)
Core
Graphics: CanvasWebGL
Tracking
()
RESOLVED
FIXED
mozilla50
Tracking | Status | |
---|---|---|
firefox50 | --- | fixed |
People
(Reporter: vlad, Assigned: jgilbert)
References
Details
(Keywords: dev-doc-complete, Whiteboard: [webvr-noted])
Attachments
(3 files, 5 obsolete files)
ANGLE already includes the ability to translate EXT_shader_texture_lod to ARB_shader_texture_lod (e.g. changing EXT to ARB suffix) when using GLSL output. We don't enable WebGL EXT_shader_texture_lod unless the underlying implementation supports only EXT_shader_texture_lod though; we should fix this so that ARB_shader_texture_lod will also trigger support.
This affects exposing this WebGL extension on OSX and Linux.
Comment 1•10 years ago
|
||
Attachment #8536699 -
Flags: feedback?(vladimir)
Comment 2•10 years ago
|
||
ARB_shader_texture_lod is contained in the core profile of OpenGL 3.0 and OpenGL ES 3.0. Support should also trigger if a core or compatibility profile is used that's higher than 3.0, regardless of the presence or absence of EXT/ARB_shader_texture_lod.
Reporter | ||
Comment 3•10 years ago
|
||
Comment on attachment 8536699 [details] [diff] [review]
This is a start, I imagine there is more?
I actually am not sure -- that *may* be all that's needed. Do a build on OSX and see if the test passes? :)
Part of the problem here is that the extension tests pass if the extension is not available. Ideally we'd have a way to say "we expect this extension to be available" -- maybe we need a mozilla-specific test for that, to verify that we're actually testing what we expect to. (That's for a different bug, but it would just need to check that we have a set of extensions that we expect on our various test machines.)
Reporter | ||
Comment 4•10 years ago
|
||
(In reply to Florian Bösch from comment #2)
> ARB_shader_texture_lod is contained in the core profile of OpenGL 3.0 and
> OpenGL ES 3.0. Support should also trigger if a core or compatibility
> profile is used that's higher than 3.0, regardless of the presence or
> absence of EXT/ARB_shader_texture_lod.
Yeah, we should probably handle this via a meta feature in GLContextFeatures (shader_texture_lod) that's provided by EXT_, ARB_, or GL >= 3.0. We don't currently create a core profile on OSX, but would like to at some point in the future.
However, if we do provide it via core, I think ANGLE will need to be updated -- right now ANGLE's GLSL translator only ouputs the proper translation for the ARB extension. It will need to understand how to output the core symbols as well, if GL >= 3.0.
Comment 5•10 years ago
|
||
(In reply to Vladimir Vukicevic [:vlad] [:vladv] from comment #3)
> Comment on attachment 8536699 [details] [diff] [review]
> This is a start, I imagine there is more?
>
> I actually am not sure -- that *may* be all that's needed. Do a build on
> OSX and see if the test passes? :)
Which test?
This bug is preventing use of the extension on OSX (and Linux?). Ken Russell suggested that Chrome already supports ARB_shader_texture_lod, and exposes the extension on OSX. Can Firefox do the same? This is important for texture ops in loops/conditionals, since some shader optimizers unrolled/extracted texture ops that needed the derivatives.
Comment 7•10 years ago
|
||
Can somebody give me a (minimal?) test case that fails in Firefox and passes in Chrome because of this issue?
This is the khronos test case. Firefox reports that the extension isn't exposed on OSX 10.10.1.
https://www.khronos.org/registry/webgl/sdk/tests/conformance/extensions/ext-shader-texture-lod.html
Updated•10 years ago
|
Attachment #8536699 -
Flags: review?(jgilbert)
Comment on attachment 8536699 [details] [diff] [review]
This is a start, I imagine there is more?
That may be enough. Ken Russell said the Angle shader translator works with ARB_ or EXT_shader_texture_lod. #extension and the call itself texture2DLodExt vs. textureLod() in core 3.2. I'm not sure if texture2DLodARB() in 2.1, since I only use core 3.2+. If you run that test and it passes, then that should be enough.
Comment 10•10 years ago
|
||
Assignee | ||
Comment 11•10 years ago
|
||
Comment on attachment 8536699 [details] [diff] [review]
This is a start, I imagine there is more?
Review of attachment 8536699 [details] [diff] [review]:
-----------------------------------------------------------------
Yep. It should need something in the GLFeature configs, but we should double-check with the specs.
Attachment #8536699 -
Flags: review?(jgilbert) → feedback+
Comment 12•10 years ago
|
||
(In reply to Alecazam from comment #8)
> This is the khronos test case. Firefox reports that the extension isn't
> exposed on OSX 10.10.1.
>
> https://www.khronos.org/registry/webgl/sdk/tests/conformance/extensions/ext-
> shader-texture-lod.html
Passes and supports it with the patch.
Comment 13•10 years ago
|
||
Comment 14•10 years ago
|
||
Attachment #8536699 -
Attachment is obsolete: true
Attachment #8536699 -
Flags: feedback?(vladimir)
Attachment #8561303 -
Flags: review?(jgilbert)
Comment 15•10 years ago
|
||
Comment on attachment 8561303 [details] [diff] [review]
ARB_shader_texture_lod support to implement WebGL EXT_shader_texture_lod. r=jgilbert
Review of attachment 8561303 [details] [diff] [review]:
-----------------------------------------------------------------
::: dom/canvas/WebGLContextExtensions.cpp
@@ +115,5 @@
> case WebGLExtensionID::EXT_frag_depth:
> return WebGLExtensionFragDepth::IsSupported(this);
> case WebGLExtensionID::EXT_shader_texture_lod:
> + return gl->IsExtensionSupported(gl::GLContext::EXT_shader_texture_lod) ||
> + gl->IsExtensionSupported(gl::GLContext::ARB_shader_texture_lod);
This should be:
return gl->IsSupported(gl::GLFeature::shader_texture_lod);
::: gfx/gl/GLContext.h
@@ +122,1 @@
> sampler_objects,
Please keep these sorted:
...
sampler_objects,
shader_texture_lod,
...
(Yeah, sR... comes before sa... according to M-x sort-lines. Unix FTW)
::: gfx/gl/GLContextFeatures.cpp
@@ +446,5 @@
> + GLVersion::NONE,
> + GLESVersion::NONE,
> + GLContext::Extension_None,
> + {
> + GLContext::ARB_shader_texture_lod,
I believe this is wrong. Following the pattern it should be:
...
GLContext::ARB_shader_texture_lod,
{
GLContext::EXT_shader_texture_lod,
GLContext::Extensions_End
}
...
(But since the extension doesn't add functions to the C++ API, it's probably not an error, but I'd like it to keep the pattern of the other checks.)
Assignee | ||
Comment 16•10 years ago
|
||
Comment on attachment 8561303 [details] [diff] [review]
ARB_shader_texture_lod support to implement WebGL EXT_shader_texture_lod. r=jgilbert
Review of attachment 8561303 [details] [diff] [review]:
-----------------------------------------------------------------
::: gfx/gl/GLContext.h
@@ +122,1 @@
> sampler_objects,
TBH, I generally sort case-insensitively. Since sorting is generally about making is easy to find things, it's easier to just remember one criteria (as a human) rather than two (a-z order, uppercase before lowercase).
That said, "shader" goes after "sampler", but before "standard".
::: gfx/gl/GLContextFeatures.cpp
@@ +446,5 @@
> + GLVersion::NONE,
> + GLESVersion::NONE,
> + GLContext::Extension_None,
> + {
> + GLContext::ARB_shader_texture_lod,
So technically, it doesn't matter, since this exposes no C++ entrypoints. However, conceptually it should be in the extension list, as the GLSL functions it exposes are ARB-suffixed.
I would leave this in the extension list.
Attachment #8561303 -
Flags: review?(jgilbert) → review+
Assignee | ||
Comment 17•10 years ago
|
||
Comment on attachment 8561303 [details] [diff] [review]
ARB_shader_texture_lod support to implement WebGL EXT_shader_texture_lod. r=jgilbert
Review of attachment 8561303 [details] [diff] [review]:
-----------------------------------------------------------------
::: gfx/gl/GLContextFeatures.cpp
@@ +443,5 @@
> },
> {
> + "shader_texture_lod",
> + GLVersion::NONE,
> + GLESVersion::NONE,
I checked on when this went core, and this functionality is supported by GL3+ and GLES3+.
However, the function signatures are different. It looks like ANGLE's translating should cover us, though.
Comment 18•10 years ago
|
||
This extension is still missing from the Nightly builds. Is there any possibility of exposing it soon? I'm having to maintain two shaders one with and without the extension, but the semantics only translate for LOD 0. Firefox OSX is the only browser omitting this extension.
Comment 19•9 years ago
|
||
Any roadmap to ship this feature? Another example that still fails on OSX http://vorg.github.io/pex/examples/glu.TextureCube.lod/ - there is a slider to manually select cubemap mipmap level
Dan, can you just take this bug to the finish line? I have no idea what actually needs to be done before this lands, and what are stylistic comments.
Assignee: nobody → dglastonbury
Updated•9 years ago
|
Whiteboard: [webvr-noted]
Comment 21•9 years ago
|
||
Landing this would potentially help out ThreeJS and other libraries that are implementing a roughness factor for physically based lighting.
Please see this ThreeJS issue for details on how they are having to waste texture units to work around lack of the extension:
https://github.com/mrdoob/three.js/issues/5847
Comment 22•9 years ago
|
||
Updated•9 years ago
|
Assignee: dglastonbury → kgilbert
Comment 23•9 years ago
|
||
Comment 24•9 years ago
|
||
Bug 1111689 - Part 1: Let EXT_shader_texture_lod play when ARB_shader_texture_lod can play. r=jgilbert
Attachment #8658891 -
Flags: review?(jgilbert)
Comment 25•9 years ago
|
||
Comment on attachment 8658891 [details]
MozReview Request: Bug 1111689 - Part 1: Let EXT_shader_texture_lod play when ARB_shader_texture_lod can play. r=jgilbert
Bug 1111689 - Part 1: Let EXT_shader_texture_lod play when ARB_shader_texture_lod can play. r=jgilbert
Comment 26•9 years ago
|
||
Bug 1111689: Part 2 - Update ext-shader-texture-lod WebGL conformance test
- Cherry picked minimal changes to support updated ext-shader-texture-lod
webgl conformance test.
Attachment #8658903 -
Flags: review?(jgilbert)
Comment 27•9 years ago
|
||
The Part 1 patch hasn't changed since last r+'ed, except being rebased
Assignee | ||
Comment 28•9 years ago
|
||
Comment on attachment 8658891 [details]
MozReview Request: Bug 1111689 - Part 1: Let EXT_shader_texture_lod play when ARB_shader_texture_lod can play. r=jgilbert
https://reviewboard.mozilla.org/r/18713/#review17747
::: dom/canvas/WebGLContextExtensions.cpp:119
(Diff revision 2)
> - return gl->IsExtensionSupported(gl::GLContext::EXT_shader_texture_lod);
> + return gl->IsExtensionSupported(gl::GLContext::EXT_shader_texture_lod) ||
> + gl->IsExtensionSupported(gl::GLContext::ARB_shader_texture_lod);
Shouldn't you just use the GLFeature you added here?
Attachment #8658891 -
Flags: review?(jgilbert) → review+
Assignee | ||
Comment 29•9 years ago
|
||
Comment on attachment 8658903 [details]
MozReview Request: Bug 1111689 - part 3: Suppress ASAN leak reports for libglsl.so
https://reviewboard.mozilla.org/r/18721/#review17749
Attachment #8658903 -
Flags: review?(jgilbert) → review+
Comment 30•9 years ago
|
||
Comment on attachment 8658891 [details]
MozReview Request: Bug 1111689 - Part 1: Let EXT_shader_texture_lod play when ARB_shader_texture_lod can play. r=jgilbert
Bug 1111689 - Part 1: Let EXT_shader_texture_lod play when ARB_shader_texture_lod can play. r=jgilbert
Updated•9 years ago
|
Attachment #8658903 -
Attachment description: MozReview Request: Bug 1111689: Part 2 - Update ext-shader-texture-lod WebGL conformance test → MozReview Request: Bug 1111689: Part 2 - Update ext-shader-texture-lod WebGL conformance test,r=jgilbert
Comment 31•9 years ago
|
||
Comment on attachment 8658903 [details]
MozReview Request: Bug 1111689 - part 3: Suppress ASAN leak reports for libglsl.so
Bug 1111689: Part 2 - Update ext-shader-texture-lod WebGL conformance test,r=jgilbert
- Cherry picked minimal changes to support updated ext-shader-texture-lod
webgl conformance test.
Comment 32•9 years ago
|
||
Comment 33•9 years ago
|
||
Those ASan mochitest-gl leaks on your try push? They weren't a coincidence, they were from these patches. Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/c2577d136c72
Comment 34•9 years ago
|
||
Thanks Phil, I will address the leaks and do another try run to verify.
Comment 35•9 years ago
|
||
I have reproduced two leaks by running an ASAN build locally in Linux:
TEST-UNEXPECTED-FAIL | LeakSanitizer | leak at __interceptor_strdup, /usr/lib/x86_64-linux-gnu/dri/vmwgfx_dri.so, js::frontend::BytecodeEmitter::bindNameToSlotHelper
TEST-UNEXPECTED-FAIL | LeakSanitizer | leak at __interceptor_strdup, /usr/lib/x86_64-linux-gnu/dri/vmwgfx_dri.so, js::Invoke
These occurred with the "part 2" patch for this bug, but not with just the "part 1" patch for this bug applied.
Could you please confirm that these are the ones that were not just a co-incidence?
Flags: needinfo?(philringnalda)
Comment 36•9 years ago
|
||
Doesn't sound entirely and exactly like the leak in automation, https://treeherder.mozilla.org/logviewer.html#?job_id=14394183&repo=mozilla-inbound, but then, I don't know anything about ASan other than how to tell whether it is green or orange.
Flags: needinfo?(philringnalda)
Comment 37•9 years ago
|
||
Comment 38•9 years ago
|
||
Comment on attachment 8658891 [details]
MozReview Request: Bug 1111689 - Part 1: Let EXT_shader_texture_lod play when ARB_shader_texture_lod can play. r=jgilbert
Bug 1111689 - Part 1: Let EXT_shader_texture_lod play when ARB_shader_texture_lod can play. r=jgilbert
Comment 39•9 years ago
|
||
Comment on attachment 8658903 [details]
MozReview Request: Bug 1111689 - part 3: Suppress ASAN leak reports for libglsl.so
Bug 1111689: Part 2 - Update ext-shader-texture-lod WebGL conformance test,r=jgilbert
- Cherry picked minimal changes to support updated ext-shader-texture-lod
webgl conformance test.
Comment 40•9 years ago
|
||
Comment 41•9 years ago
|
||
(In reply to :kip (Kearwood Gilbert) from comment #40)
I have updated the mochitest and minified the related webgl-test-utils.js changes further. In my local linux-asan builds, this has eliminated at least two issues found by the Leak Sanitizer:
TEST-UNEXPECTED-FAIL | LeakSanitizer | leak at __interceptor_strdup, /usr/lib/x86_64-linux-gnu/dri/vmwgfx_dri.so, js::frontend::BytecodeEmitter::bindNameToSlotHelper
TEST-UNEXPECTED-FAIL | LeakSanitizer | leak at __interceptor_strdup, /usr/lib/x86_64-linux-gnu/dri/vmwgfx_dri.so, js::Invoke
I have pushed to try to run the mochitests on all platforms to ensure there are no regressions and that the try server sees the same result:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=ac7ee0b8ee4c
Comment 42•9 years ago
|
||
Comment on attachment 8658891 [details]
MozReview Request: Bug 1111689 - Part 1: Let EXT_shader_texture_lod play when ARB_shader_texture_lod can play. r=jgilbert
Bug 1111689 - Part 1: Let EXT_shader_texture_lod play when ARB_shader_texture_lod can play. r=jgilbert
Comment 43•9 years ago
|
||
Comment on attachment 8658903 [details]
MozReview Request: Bug 1111689 - part 3: Suppress ASAN leak reports for libglsl.so
Bug 1111689: Part 2 - Update ext-shader-texture-lod WebGL conformance test,r=jgilbert
- Cherry picked minimal changes to support updated ext-shader-texture-lod
webgl conformance test.
Comment 44•9 years ago
|
||
Comment 45•9 years ago
|
||
(In reply to :kip (Kearwood Gilbert) from comment #41)
> (In reply to :kip (Kearwood Gilbert) from comment #40)
>
> I have updated the mochitest and minified the related webgl-test-utils.js
> changes further. In my local linux-asan builds, this has eliminated at
> least two issues found by the Leak Sanitizer:
>
>
> TEST-UNEXPECTED-FAIL | LeakSanitizer | leak at __interceptor_strdup,
> /usr/lib/x86_64-linux-gnu/dri/vmwgfx_dri.so,
> js::frontend::BytecodeEmitter::bindNameToSlotHelper
> TEST-UNEXPECTED-FAIL | LeakSanitizer | leak at __interceptor_strdup,
> /usr/lib/x86_64-linux-gnu/dri/vmwgfx_dri.so, js::Invoke
>
> I have pushed to try to run the mochitests on all platforms to ensure there
> are no regressions and that the try server sees the same result:
>
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=ac7ee0b8ee4c
That try run was inadvertently running only reftests. Please see this one for the mochitests:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=be7814a807f8
Comment 46•9 years ago
|
||
Comment on attachment 8658891 [details]
MozReview Request: Bug 1111689 - Part 1: Let EXT_shader_texture_lod play when ARB_shader_texture_lod can play. r=jgilbert
Review request updated; see interdiff: https://reviewboard.mozilla.org/r/18713/diff/5-6/
Comment 47•9 years ago
|
||
Comment on attachment 8658903 [details]
MozReview Request: Bug 1111689 - part 3: Suppress ASAN leak reports for libglsl.so
Review request updated; see interdiff: https://reviewboard.mozilla.org/r/18721/diff/4-5/
Comment 48•9 years ago
|
||
Comment 49•9 years ago
|
||
I have un-bitrotted these patches and pushed to try to see what / if any leaks are still caused by the conformance tests.
Comment 50•9 years ago
|
||
Comment 51•9 years ago
|
||
Comment 52•9 years ago
|
||
Comment 53•9 years ago
|
||
Review request updated; see interdiff: https://reviewboard.mozilla.org/r/18713/diff/6-7/
Attachment #8698584 -
Flags: review?(jgilbert)
Comment 54•9 years ago
|
||
Review request updated; see interdiff: https://reviewboard.mozilla.org/r/18721/diff/5-6/
Attachment #8698585 -
Flags: review?(jgilbert)
Comment 55•9 years ago
|
||
Patches rebased
Updated•9 years ago
|
Attachment #8561303 -
Attachment is obsolete: true
Updated•9 years ago
|
Attachment #8658891 -
Attachment is obsolete: true
Updated•9 years ago
|
Attachment #8658903 -
Attachment is obsolete: true
Comment 56•9 years ago
|
||
Comment 57•9 years ago
|
||
Assignee | ||
Comment 58•9 years ago
|
||
Assignee | ||
Comment 59•9 years ago
|
||
Assignee | ||
Comment 60•9 years ago
|
||
Assignee | ||
Comment 61•9 years ago
|
||
Assignee | ||
Comment 62•9 years ago
|
||
Comment on attachment 8698584 [details]
MozReview Request: Bug 1111689 - Part 1: Let EXT_shader_texture_lod play when ARB_shader_texture_lod can play. r=jgilbert
Since MozReview can't seem to mark this r+, I'll do it myself.
Attachment #8698584 -
Flags: review?(jgilbert) → review+
Assignee | ||
Updated•9 years ago
|
Attachment #8698585 -
Flags: review?(jgilbert) → review+
Comment 63•9 years ago
|
||
Thanks Jeff. Sorry to generate extra bugmail for you. I wished to update the patches without re-submitting for review.
Comment 64•9 years ago
|
||
Looks like the patches are r+. Did this land, though?
Updated•9 years ago
|
Flags: needinfo?(kgilbert)
Comment 65•9 years ago
|
||
Thanks for bringing this to my attention, Milan, as it fell between the cracks over the holidays. The patches still apply -- I'll do a try run and if it still passes, then I can land this.
Flags: needinfo?(kgilbert)
Updated•9 years ago
|
Keywords: dev-doc-needed
Comment 66•9 years ago
|
||
Windows builds did not launch in try push. Pushing again..
Reporter | ||
Comment 68•9 years ago
|
||
Wait.. this patch can't work. If ARB_shader_texture_lod is supported, the new functions have an ARB suffix. So at a minimum, the ANGLE shader translator will need to be updated to either rename, e.g. textureCubeLodEXT -> textureCubeLodARB, or to implement wrapper functions with the EXT suffix in a preamble that will call the ARB functions.
Does the test actually pass on OSX, which is where we only have ARB_shader_texture_lod?
Comment 69•9 years ago
|
||
The test passes and I am able to use sites that except use the extension; however, there were potentially some leaks still occurring reported in the Linux Asan builds. My earlier update fixes some of the leaks but it is unclear if the others are still an issue: I could use help from someone familiar with the asan reports to look at the earlier try runs and see if it is something to be concerned with before landing
Reporter | ||
Comment 70•9 years ago
|
||
Yeah, it looks like it only accidentally works. We should make the feature depend on OpenGL >= 3.2, and not ARB_shader_texture_lod.
ANGLE does *not* understand ARB_shader_texture_lod at all. It will either translate things to EXT_shader_texture_lod (if the target is OpenGL ES), or to the core profile calls if the target output is GLSL >= 1.3. So, "textureCubeGradEXT" is translated to "textureGrad", and not to "textureCubeGradARB" as it should be if ARB_shader_texture_lod was actually being used.
It happens to work because the functions are core now, and things like textureGrad are available without explicitly enabling an extension in the shader.
For the Linux ASAN issue -- it looks like a leak inside the mesa/dri glsl compiler. I think you add a suppression and move on.
Reporter | ||
Comment 71•9 years ago
|
||
Hmm.. maybe I'm wrong about ANGLE not emitting the ARB names (and I wrote this code even, I think). So, ignore the above, other than the ASAN note.. I think you can just suppress that and ignore it.
Comment 72•9 years ago
|
||
Comment on attachment 8658891 [details]
MozReview Request: Bug 1111689 - Part 1: Let EXT_shader_texture_lod play when ARB_shader_texture_lod can play. r=jgilbert
Review request updated; see interdiff: https://reviewboard.mozilla.org/r/18713/diff/7-8/
Attachment #8658891 -
Attachment is obsolete: false
Comment 73•9 years ago
|
||
Comment on attachment 8658903 [details]
MozReview Request: Bug 1111689 - part 3: Suppress ASAN leak reports for libglsl.so
Review request updated; see interdiff: https://reviewboard.mozilla.org/r/18721/diff/6-7/
Attachment #8658903 -
Attachment is obsolete: false
Comment 74•9 years ago
|
||
- Suppress ASAN leak reports due to leaks in the mesa/dri glsl compiler.
Review commit: https://reviewboard.mozilla.org/r/41791/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/41791/
Updated•9 years ago
|
Attachment #8658891 -
Attachment is obsolete: true
Updated•9 years ago
|
Attachment #8658903 -
Attachment is obsolete: true
Updated•9 years ago
|
Attachment #8733476 -
Flags: review?(vladimir)
Comment 75•9 years ago
|
||
I have rebased the patches and added libglsl.so to the ASAN exclusions. If the try push shows that we have caught the last of the ASAN failures and Vlad's okay with this way of suppressing the glsl compiler leaks, then I can land this.
Comment 76•9 years ago
|
||
The last try run failed with:
"1090639 Only broken slaves running at too small a resolution hit test_conformance__attribs__gl-vertex-attrib-zero-issues.html | Unable to fetch WebGL rendering context for Canvas"
This seems to indicate that the failure was not related to the patch, so I've pushed to try again.
Comment 77•9 years ago
|
||
Got another bug report from Epic about this - on OS X, Safari and Chrome do support WebGL EXT_shader_texture_lod extension but Firefox is missing out. Wrote down bug 1270435 since didn't notice this at first.
This bug item relates to expanding the texture_lod support to cover also native GL systems that have GL_ARB_shader_texture_lod, which is case #1 from https://bugzilla.mozilla.org/show_bug.cgi?id=1270435#c0. After this lands, will this also cover case #2 from the same comment?
Looking at my OS X laptop, it no longer lists GL_ARB_shader_texture_lod extension, since it's running with GL 4.1 core, so it will fall under case #2 in there.
Kip, was the latest try green?
Flags: needinfo?(kgilbert)
Updated•9 years ago
|
Attachment #8733476 -
Flags: review?(vladimir) → review?(jgilbert)
Assignee | ||
Comment 78•9 years ago
|
||
Comment on attachment 8733476 [details]
MozReview Request: Bug 1111689 - part 3: Suppress ASAN leak reports for libglsl.so
https://reviewboard.mozilla.org/r/41791/#review47553
Attachment #8733476 -
Flags: review?(jgilbert) → review+
Comment 79•9 years ago
|
||
I was waiting on a review for a patch that suppresses ASAN leaks caused by the GPU driver. I have changed the reviewer, so hopefully will get seen sooner.
My last try push resulted in the errors in Comment 76, which seemed to indicate problems with the test nodes. I've pushed again hoping to see them cleared and to verify that this patch does not need rebasing.
Otherwise, this patch is ready to land.
I've left the NI on so I can check into your other question.
Flags: needinfo?(kgilbert)
Assignee | ||
Comment 80•9 years ago
|
||
Cool, it's all r+ now.
Comment 81•9 years ago
|
||
Bug 1193526 updated the WebGL conformance tests, so it should not longer be needed to apply the 2nd patch, which cherry picks the ext-shader-texture-lod conformance test.
I'm re-basing the other patches now.
Comment 82•9 years ago
|
||
Comment on attachment 8658891 [details]
MozReview Request: Bug 1111689 - Part 1: Let EXT_shader_texture_lod play when ARB_shader_texture_lod can play. r=jgilbert
Review request updated; see interdiff: https://reviewboard.mozilla.org/r/18713/diff/8-9/
Attachment #8658903 -
Attachment description: MozReview Request: Bug 1111689: Part 2 - Update ext-shader-texture-lod WebGL conformance test,r=jgilbert → MozReview Request: Bug 1111689 - part 3: Suppress ASAN leak reports for libglsl.so
Attachment #8658891 -
Attachment is obsolete: false
Attachment #8658903 -
Attachment is obsolete: false
Comment 83•9 years ago
|
||
Comment on attachment 8658903 [details]
MozReview Request: Bug 1111689 - part 3: Suppress ASAN leak reports for libglsl.so
Review request updated; see interdiff: https://reviewboard.mozilla.org/r/18721/diff/7-8/
Updated•9 years ago
|
Attachment #8733476 -
Attachment is obsolete: true
Updated•9 years ago
|
Attachment #8698585 -
Attachment is obsolete: true
Updated•9 years ago
|
Attachment #8698584 -
Attachment is obsolete: true
Comment 84•9 years ago
|
||
Comment on attachment 8658891 [details]
MozReview Request: Bug 1111689 - Part 1: Let EXT_shader_texture_lod play when ARB_shader_texture_lod can play. r=jgilbert
Review request updated; see interdiff: https://reviewboard.mozilla.org/r/18713/diff/9-10/
Comment 85•9 years ago
|
||
Comment on attachment 8658903 [details]
MozReview Request: Bug 1111689 - part 3: Suppress ASAN leak reports for libglsl.so
Review request updated; see interdiff: https://reviewboard.mozilla.org/r/18721/diff/8-9/
Comment 86•9 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/51403/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/51403/
Comment 87•9 years ago
|
||
Comment on attachment 8658891 [details]
MozReview Request: Bug 1111689 - Part 1: Let EXT_shader_texture_lod play when ARB_shader_texture_lod can play. r=jgilbert
Review request updated; see interdiff: https://reviewboard.mozilla.org/r/18713/diff/10-11/
Comment 88•9 years ago
|
||
Comment on attachment 8658903 [details]
MozReview Request: Bug 1111689 - part 3: Suppress ASAN leak reports for libglsl.so
Review request updated; see interdiff: https://reviewboard.mozilla.org/r/18721/diff/9-10/
Comment 89•9 years ago
|
||
The WebGL ensure-exts mochitests appear to have failed in the last try push due to expecting the extension to be disabled. I have updated the ensure-exts mochitest and have pushed again.
Comment 90•9 years ago
|
||
Comment 91•9 years ago
|
||
(In reply to :kip (Kearwood Gilbert) from comment #90)
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=ffba59e6be9a
Please disregard this try push, it is not related to this bug (wrong patch-set applied during try push)
Comment 92•9 years ago
|
||
The last try push in MozReview shows that this patch is working for OSX, but the extension is not being exposed on the other platforms. Perhaps this is due to bug 1270435 (See Comment 77).
@jgilbert - Would it make sense to update the ensure-exts mochitests to expect the extension only on OSX for now, then fix bug 1270435 after this lands?
Flags: needinfo?(jgilbert)
Assignee | ||
Comment 93•9 years ago
|
||
(In reply to :kip (Kearwood Gilbert) from comment #92)
> The last try push in MozReview shows that this patch is working for OSX, but
> the extension is not being exposed on the other platforms. Perhaps this is
> due to bug 1270435 (See Comment 77).
>
> @jgilbert - Would it make sense to update the ensure-exts mochitests to
> expect the extension only on OSX for now, then fix bug 1270435 after this
> lands?
This patch regresses EXT_shader_texture_lod on Windows, which is more important than adding support to OSX.
We don't care about having it ensured-exposed on all platforms per se, but we don't want to regress what we already have.
Flags: needinfo?(jgilbert)
Assignee | ||
Updated•8 years ago
|
Assignee: kgilbert → jgilbert
Assignee | ||
Updated•8 years ago
|
OS: Windows 8 → Unspecified
Hardware: x86_64 → Unspecified
Comment 94•8 years ago
|
||
Pushed by jgilbert@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/967d519f5d08
Part 1: Let EXT_shader_texture_lod play when ARB_shader_texture_lod can play. r=jgilbert
https://hg.mozilla.org/integration/mozilla-inbound/rev/cb93aa0fe394
Suppress ASAN leak reports for libglsl.so. - r=jgilbert
https://hg.mozilla.org/integration/mozilla-inbound/rev/e912f451cae4
Remark failures.
Assignee | ||
Comment 95•8 years ago
|
||
The problem was the GLFeature definition was added in the same order as the declarations.
The changes here are fine, just needed that fix and a unexpected-pass remark.
Comment 96•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/967d519f5d08
https://hg.mozilla.org/mozilla-central/rev/cb93aa0fe394
https://hg.mozilla.org/mozilla-central/rev/e912f451cae4
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox50:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
Comment 97•8 years ago
|
||
Developer release notes:
https://developer.mozilla.org/en-US/Firefox/Releases/50#WebGL
Reference docs:
https://developer.mozilla.org/en-US/docs/Web/API/EXT_shader_texture_lod
Updated support info:
https://developer.mozilla.org/en-US/docs/Web/API/WebGL_API/Using_Extensions
https://developer.mozilla.org/en-US/docs/Web/API/WebGLRenderingContext/getSupportedExtensions
https://developer.mozilla.org/en-US/docs/Web/API/WebGLRenderingContext/getExtension
Keywords: dev-doc-needed → dev-doc-complete
You need to log in
before you can comment on or make changes to this bug.
Description
•