| Summary: | No test for Image Load/Store on format-incompatible texture buffer | ||
|---|---|---|---|
| Product: | Mesa | Reporter: | Nanley Chery <nanleychery> |
| Component: | Mesa core | Assignee: | mesa-dev |
| Status: | RESOLVED FIXED | QA Contact: | mesa-dev |
| Severity: | normal | ||
| Priority: | medium | CC: | agomez, currojerez, danylo.piliaiev |
| Version: | git | ||
| Hardware: | Other | ||
| OS: | All | ||
| Whiteboard: | |||
| i915 platform: | i915 features: | ||
It should be trivial to extend the arb_shader_image_load_store-invalid piglit test (which already checks a similar condition for other texture targets) to catch this problem. Sent the test catch the problem - https://patchwork.freedesktop.org/patch/239424/ Curro, is this fixed by? commit 5a6814780322988a7adee525899bca8a83907ab7 Author: Francisco Jerez <currojerez@riseup.net> Date: Fri Mar 16 13:43:27 2018 -0700 Revert "mesa: simplify _mesa_is_image_unit_valid for buffers" This reverts commit c0ed52f6146c7e24e1275451773bd47c1eda3145. It was preventing the image format validation from being done on buffer textures, which is required to ensure that the application doesn't attempt to bind a buffer texture with an internal format incompatible with the image unit format (e.g. of different texel size), which is not allowed by the spec (it's not allowed for *any* texture target, whether or not there is spec wording restricting this behavior specifically for buffer textures) and will cause the driver to calculate texel bounds incorrectly and potentially crash instead of the expected behavior. Cc: mesa-stable@lists.freedesktop.org Reviewed-by: Marek Olšák <marek.olsak@amd.com> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106465 Reviewed-by: Nanley Chery <nanley.g.chery@intel.com> (In reply to Andrés Gómez García from comment #3) > Curro, is this fixed by? [...] My bad, I just realized that the report is about a missing piglit test. In that case, shouldn't we move this report to the "piglit" product? (In reply to Andrés Gómez García from comment #4) > (In reply to Andrés Gómez García from comment #3) > > Curro, is this fixed by? > > [...] > > My bad, I just realized that the report is about a missing piglit test. > > In that case, shouldn't we move this report to the "piglit" product? I'm not sure. I could see an argument for either product. Sometimes when fixing bugs in Mesa, we'd open reports against Mesa and require a fix and (sometimes) a test to close the issue. In this case the fix was already created when the bug was filed, so I just filed the issue about the missing test. This issue has been fixed by the following commit: commit b7f493e3a60baddfcf8003a326a31f6b8be2d4ca Author: Danylo Piliaiev <danylo.piliaiev@gmail.com> Date: Mon Jul 23 15:13:34 2018 +0300 arb_shader_image_load_store: Test format incompatible texture buffer Test for the regression which happened when GL_TEXTURE_BUFFER was allowed to have incompatible format. v2: Removed unnecessary code duplication - use upload_image instead of init_level. (Francisco Jerez) v3: Removed upload_image call because image is already called by init_image. (Francisco Jerez) Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106465 Signed-off-by: Danylo Piliaiev <danylo.piliaiev@globallogic.com> Reviewed-by: Francisco Jerez <currojerez@riseup.net> |
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.
From the ARB_shader_image_load_store spec: If the individual texel identified for an image load, store, or atomic operation doesn't exist, the access is treated as invalid. Invalid image loads will return zero. Invalid image stores will have no effect. Invalid image atomics will not update any texture bound to the image unit and will return zero. An access is considered invalid if: [...] * the internal format of the texture bound to the image unit is incompatible with the specified <format> according to Table X.3; Mesa does not validate the format of texture buffers to ensure this behavior. This bug is fixed with this patch: https://patchwork.freedesktop.org/patch/211340/ There currently isn't a test in the CTS or in piglit which catches this error. One should be written to prevent this issue from appearing in the future.