Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1691853003:
Do not try to use GL_texture_rectangle on GL ES (Closed)
|
Created:
4 years, 10 months ago by Kimmo Kinnunen Modified:
4 years, 10 months ago CC:
reviews_skia.org Base URL:
https://skia.googlesource.com/skia.git@master Target Ref:
refs/heads/master Project:
skia Visibility:
Public. |
DescriptionDo not try to use GL_texture_rectangle on GL ES
Do not try to use ARB_GL_texture_rectangle on GL ES.
Command buffer exposes it, but it's not recommended. It also
does not work with TexImage2D, and ANGLE does not
have support for it on with GL ES 3.0 source context.
Bug triggered only when trying to use command buffer in
GL ES 3.0 mode.
BUG=angleproject:1313
GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1691853003
Committed: https://skia.googlesource.com/skia/+/e06ed2554064781237430cfbd3abddd6c85126eb
Patch Set 1 #
Total comments: 1
Dependent Patchsets: Messages
Total messages: 21 (8 generated)
Description was changed from ========== Do not try to use GL_texture_rectangle on GL ES Do not try to use ARB_GL_texture_rectangle on GL ES. Command buffer exposes it, but it's not recommended. It also does not work with TexImage2D, and ANGLE does not have support for it on with GL ES 3.0 source context. Bug triggered only when trying to use command buffer in GL ES 3.0 mode. BUG=angleproject:1313 ========== to ========== Do not try to use GL_texture_rectangle on GL ES Do not try to use ARB_GL_texture_rectangle on GL ES. Command buffer exposes it, but it's not recommended. It also does not work with TexImage2D, and ANGLE does not have support for it on with GL ES 3.0 source context. Bug triggered only when trying to use command buffer in GL ES 3.0 mode. BUG=angleproject:1313 GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ==========
kkinnunen@nvidia.com changed reviewers: + bsalomon@google.com
Description was changed from ========== Do not try to use GL_texture_rectangle on GL ES Do not try to use ARB_GL_texture_rectangle on GL ES. Command buffer exposes it, but it's not recommended. It also does not work with TexImage2D, and ANGLE does not have support for it on with GL ES 3.0 source context. Bug triggered only when trying to use command buffer in GL ES 3.0 mode. BUG=angleproject:1313 GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ========== to ========== Do not try to use GL_texture_rectangle on GL ES Do not try to use ARB_GL_texture_rectangle on GL ES. Command buffer exposes it, but it's not recommended. It also does not work with TexImage2D, and ANGLE does not have support for it on with GL ES 3.0 source context. Bug triggered only when trying to use command buffer in GL ES 3.0 mode. BUG=angle:1313 GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ==========
Description was changed from ========== Do not try to use GL_texture_rectangle on GL ES Do not try to use ARB_GL_texture_rectangle on GL ES. Command buffer exposes it, but it's not recommended. It also does not work with TexImage2D, and ANGLE does not have support for it on with GL ES 3.0 source context. Bug triggered only when trying to use command buffer in GL ES 3.0 mode. BUG=angle:1313 GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ========== to ========== Do not try to use GL_texture_rectangle on GL ES Do not try to use ARB_GL_texture_rectangle on GL ES. Command buffer exposes it, but it's not recommended. It also does not work with TexImage2D, and ANGLE does not have support for it on with GL ES 3.0 source context. Bug triggered only when trying to use command buffer in GL ES 3.0 mode. BUG=angleproject:1313 GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ==========
bsalomon@google.com changed reviewers: + ericrk@chromium.org
bsalomon@google.com changed reviewers: + erikchen@chromium.org
Kimmo does this cause our tests to fail on the cmd buffer? Eric and Erik (coincidence??) have been looking at using RECTANGLE textures in Chromium with Skia. I agree that the command buffer support is incomplete (and not really spec'ed since the extension doesn't mention GLES). We should figure out how to proceed here. I don't want this to block getting our testing running over the command buffer on Skia bots. Seems like we may need a CHROMIUM extension that adapts ARB_RECTANGLE to the ES command buffer and some code changes to add support for texture functions (and textureSize() GLSL support of rectangle2d samplers).
On 2016/02/12 13:51:36, bsalomon wrote: > Kimmo does this cause our tests to fail on the cmd buffer? Causes all code to fail when run with cmd buffer, when cmd buffer is modified to give ES3. (copy shader uses rect) On ES2 command buffer, there's no problem since the glsl lang generation check is there. > Eric and Erik > (coincidence??) have been looking at using RECTANGLE textures in Chromium with > Skia. I agree that the command buffer support is incomplete (and not really > spec'ed since the extension doesn't mention GLES). We should figure out how to > proceed here. I don't want this to block getting our testing running over the > command buffer on Skia bots. Yep. I don't see any problem disabling it on ES though, since it's not really working on ES anyway? > Seems like we may need a CHROMIUM extension that > adapts ARB_RECTANGLE to the ES command buffer and some code changes to add > support for texture functions (and textureSize() GLSL support of rectangle2d > samplers). Yeah, I don't know if you saw kbr commenting at: https://bugs.chromium.org/p/angleproject/issues/detail?id=1313 CHROMIUM extension sounds like a solution. I suppose there was texelFetch or some such function, which might do some but not everything that texture_rectangle does. This is sort of needed to trigger ANGLE, if then ANGLE is fixed (not saying you didn't know it or couldn't do it :) ): https://codereview.chromium.org/1688233003
And to clarify: I dont know whether kbr was opposed of tex rect in general or just the lack pf chromium extension. I trust you guys can hash it over. Just that since feature needs these discussions, i would rather not fix it in angle+chromium+skia. Thus from my perspective it would be good to disabke it until it works, to unblock my es3 work
On 2016/02/12 14:52:26, Kimmo Kinnunen wrote: > And to clarify: > I dont know whether kbr was opposed of tex rect in general or just the lack pf > chromium extension. I trust you guys can hash it over. Just that since feature > needs these discussions, i would rather not fix it in angle+chromium+skia. Thus > from my perspective it would be good to disabke it until it works, to unblock my > es3 work Understood... if disabling until this is worked out is ok with ericrk@ then it's ok with me.
https://codereview.chromium.org/1691853003/diff/1/src/gpu/gl/GrGLCaps.cpp File src/gpu/gl/GrGLCaps.cpp (left): https://codereview.chromium.org/1691853003/diff/1/src/gpu/gl/GrGLCaps.cpp#old... src/gpu/gl/GrGLCaps.cpp:233: if (ctxInfo.glslGeneration() >= k140_GrGLSLGeneration) { Soo.. doesn't this kind of mean that it is disabled today, too? So what would be the harm in disabling it the other way?
On 2016/02/12 at 15:02:53, bsalomon wrote: > On 2016/02/12 14:52:26, Kimmo Kinnunen wrote: > > And to clarify: > > I dont know whether kbr was opposed of tex rect in general or just the lack pf > > chromium extension. I trust you guys can hash it over. Just that since feature > > needs these discussions, i would rather not fix it in angle+chromium+skia. Thus > > from my perspective it would be good to disabke it until it works, to unblock my > > es3 work > > Understood... if disabling until this is worked out is ok with ericrk@ then it's ok with me. Lgtm - I think coand buffer on desktop should be standard gl, so this shouldn't impact the work I'm doing.
On 2016/02/15 at 07:00:41, kkinnunen wrote: > https://codereview.chromium.org/1691853003/diff/1/src/gpu/gl/GrGLCaps.cpp > File src/gpu/gl/GrGLCaps.cpp (left): > > https://codereview.chromium.org/1691853003/diff/1/src/gpu/gl/GrGLCaps.cpp#old... > src/gpu/gl/GrGLCaps.cpp:233: if (ctxInfo.glslGeneration() >= k140_GrGLSLGeneration) { > Soo.. doesn't this kind of mean that it is disabled today, too? So what would be the harm in disabling it the other way? Depending on the settings/platform I believe (we pass this if I enable core profile on Mac for example). But yes, I think for ES this is currently disabled anyway.
On 2016/02/15 17:59:08, ericrk wrote: > On 2016/02/15 at 07:00:41, kkinnunen wrote: > https://codereview.chromium.org/1691853003/diff/1/src/gpu/gl/GrGLCaps.cpp#old... > > src/gpu/gl/GrGLCaps.cpp:233: if (ctxInfo.glslGeneration() >= > k140_GrGLSLGeneration) { > > Soo.. doesn't this kind of mean that it is disabled today, too? So what would > be the harm in disabling it the other way? > > Depending on the settings/platform I believe (we pass this if I enable core > profile on Mac for example). But yes, I think for ES this is currently disabled > anyway. There must be some sort of misunderstanding now. "Core profile on Mac" does not make sense. Neither does "I think coand buffer on desktop should be standard gl" Command buffer is GL ES 2.0. The flag is always false on GL ES 2.0.
On 2016/02/15 at 19:08:01, kkinnunen wrote: > On 2016/02/15 17:59:08, ericrk wrote: > > On 2016/02/15 at 07:00:41, kkinnunen wrote: > > https://codereview.chromium.org/1691853003/diff/1/src/gpu/gl/GrGLCaps.cpp#old... > > > src/gpu/gl/GrGLCaps.cpp:233: if (ctxInfo.glslGeneration() >= > > k140_GrGLSLGeneration) { > > > Soo.. doesn't this kind of mean that it is disabled today, too? So what would > > be the harm in disabling it the other way? > > > > Depending on the settings/platform I believe (we pass this if I enable core > > profile on Mac for example). But yes, I think for ES this is currently disabled > > anyway. > > There must be some sort of misunderstanding now. > "Core profile on Mac" does not make sense. > Neither does "I think coand buffer on desktop should be standard gl" > > Command buffer is GL ES 2.0. > > The flag is always false on GL ES 2.0. Ok - I'll need to look more closely tomorrow (out of office today) - but I agree that this generally doesn't work right now so your change doesn't hurt anything. Please don't block submission on me. I'll likely need to update this logic a bit later, as I need to be able to render to texture rectangle in es2 mode... Maybe exposed via a custom chrome gl extension. Rendering to texture rectangle currently works fine on Mac if I comment out the shader level check in this code (shader level is only needed to texture from a texture rectangle).
lgtm. Eric I think the confusion is that even on Mac core profile Skia is running on the client side of the command buffer which presents an ES interface even though the underlying implementation on the service side is regular GL. All the GL strings and such indicate an ES interface so from Skia's perspective it is ES.
The CQ bit was checked by kkinnunen@nvidia.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1691853003/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1691853003/1
Message was sent while issue was closed.
Description was changed from ========== Do not try to use GL_texture_rectangle on GL ES Do not try to use ARB_GL_texture_rectangle on GL ES. Command buffer exposes it, but it's not recommended. It also does not work with TexImage2D, and ANGLE does not have support for it on with GL ES 3.0 source context. Bug triggered only when trying to use command buffer in GL ES 3.0 mode. BUG=angleproject:1313 GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ========== to ========== Do not try to use GL_texture_rectangle on GL ES Do not try to use ARB_GL_texture_rectangle on GL ES. Command buffer exposes it, but it's not recommended. It also does not work with TexImage2D, and ANGLE does not have support for it on with GL ES 3.0 source context. Bug triggered only when trying to use command buffer in GL ES 3.0 mode. BUG=angleproject:1313 GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... Committed: https://skia.googlesource.com/skia/+/e06ed2554064781237430cfbd3abddd6c85126eb ==========
Message was sent while issue was closed.
Committed patchset #1 (id:1) as https://skia.googlesource.com/skia/+/e06ed2554064781237430cfbd3abddd6c85126eb |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
