Insufficiently strict validation of copyTex{Sub}Image when DrawingBuffer emulates RGB |
|
Issue description
CopyTexImage2D specifies that
The components required for internalformat must be a subset
of those present in the framebuffer's format. For example,
a GL_RGBA framebuffer can be used to supply components for
any internalformat. However, a GL_RGB framebuffer can only
be used to supply components for GL_RGB or GL_LUMINANCE base
internal format textures, not GL_ALPHA, GL_LUMINANCE_ALPHA,
or GL_RGBA textures.
https://www.khronos.org/registry/OpenGL-Refpages/es2.0/xhtml/glCopyTexImage2D.xml
DrawingBuffer has a mode wherein it creates an RGBA allocation even when an alpha channel was not requested.
We rely on the command buffer to detect and validate the above rule in ValidateCopyTexFormat, using the format we get from GetBoundReadFramebufferInternalFormat.
https://cs.chromium.org/chromium/src/gpu/command_buffer/service/gles2_cmd_decoder.cc?rcl=51416696f2df3e8a46fb93b19db485f0eaa3a3e7&l=14585
In the case where we created an RGBA allocation, we will allow a CopyTexImage from stated-RGB (but RGBA internally) to RGBA textures.
Probably the easiest fix is to create a CHROMIUM extension that says "use this other internal format for validation", which will get GetBoundReadFramebufferInternalFormat to return RGB.
This will allow us to use a uniform set of logic for the 3 different RGB emulation paths: DISABLE_GL_RGB_FORMAT, DISABLE_WEBGL_RGB_MULTISAMPLING_USAGE, and chromium_image_rgb_emulation.
,
Oct 19 2017
It came up while I was writing this patch (not ready yet, still un-tangling the paths), which was in anticipation of adding fp16 IOSurface support to DrawingBuffer https://chromium-review.googlesource.com/c/chromium/src/+/726955 There are conformance tests that test this path, they just don't run on affected hardware (of the 3 workarounds, only the Mac one works around this issue). By merging the 3 paths into 1, we'll have conformance coverage for it.
,
Oct 20 2017
Ah, I see. Thanks very much for picking this up!
,
Oct 25 2017
Of the 3 paths: - chromium_image_rgb_emulation is solid because of the GL_RGB binding tricks - DISABLE_GL_RGB_FORMAT has the below issues I just tested - DISABLE_WEBGL_RGB_MULTISAMPLING_USAGE I think is safe - can we CopyTexImage2D from a multisample backbuffer? - can we FramebufferBlit into a multisample backbuffer? - some GL conformance tests indicate that the answer is no, but I can't seem to find it explicitly in the spec -- I went and verified DISABLE_GL_RGB_FORMAT locally, and * CopyTexSubImage2D tests fail [1] because of insufficient vaildation -- this is not a big deal * FramebufferBlit tests [2] from RGB renderbuffers to the implicitly-RGBA framebuffer fail [1] https://www.khronos.org/registry/webgl/sdk/tests/conformance/textures/misc/copy-tex-image-2d-formats.html?webglVersion=1&quiet=0 [2] https://www.khronos.org/registry/webgl/sdk/tests/conformance2/rendering/blitframebuffer-resolve-to-back-buffer.html?webglVersion=2&quiet=0
,
Oct 25 2017
To answer my questions about DISABLE_WEBGL_RGB_MULTISAMPLING_USAGE, from the GLES 3 spec (no idea what GLES2 says and under what combination of extensions): - can we CopyTexImage2D from a multisample backbuffer? - no - fails if the effective value of SAMPLE_BUFFERS for the read framebuffer is one. - the effective value of SAMPLE_BUFFERS is one if SAMPLES is non-zero, and zero otherwise. - can we FramebufferBlit into a multisample backbuffer? - no, as per OpenGL ES 3 documentation of FramebufferBlit So DISABLE_WEBGL_RGB_MULTISAMPLING_USAGE is safe here. |
|
►
Sign in to add a comment |
|
Comment 1 by kbr@chromium.org
, Oct 19 2017