| Summary: | No test for Image Load/Store on texture buffer sized greater than MAX_TEXTURE_BUFFER_SIZE_ARB | ||
|---|---|---|---|
| Product: | Mesa | Reporter: | Nanley Chery <nanleychery> |
| Component: | Drivers/DRI/i965 | Assignee: | Intel 3D Bugs Mailing List <intel-3d-bugs> |
| Status: | RESOLVED MOVED | QA Contact: | Intel 3D Bugs Mailing List <intel-3d-bugs> |
| Severity: | normal | ||
| Priority: | medium | CC: | agomez, currojerez, illia.iorin |
| Version: | git | ||
| Hardware: | Other | ||
| OS: | All | ||
| Whiteboard: | |||
| i915 platform: | i915 features: | ||
| Attachments: | piglit test workaround | ||
Created attachment 139707 [details]
piglit test workaround
Hello, i try to edit piglit test to cover that situation, and your mesa patch doesn't fix the issue. My output is
MAX_TEXTURE_BUFFER_SIZE: 134217728
Error code: 0
MAX_TEXTURE_BUFFER_SIZE: 134218728
Probe color at (0,0)
Expected: 0 255 0 0
Observed: 0 0 0 0
Probe color at (0,0)
Expected: 0 255 0 0
Observed: 0 0 0 0
Probe color at (0,0)
Expected: 0 255 0 0
Observed: 0 0 0 0
Probe color at (0,0)
Expected: 0 255 0 0
Observed: 0 0 0 0
Is need to make additional test witch cover that behavior, or add assert to that test?
(In reply to Illia from comment #1) > Created attachment 139707 [details] > piglit test workaround > > Hello, i try to edit piglit test to cover that situation, and your mesa > patch doesn't fix the issue. Hi, That's expected. The bug fix clamps the buffer size in the surface state. It doesn't modify glBufferData's behavior. > My output is > > MAX_TEXTURE_BUFFER_SIZE: 134217728 > Error code: 0 > MAX_TEXTURE_BUFFER_SIZE: 134218728 > Probe color at (0,0) > Expected: 0 255 0 0 > Observed: 0 0 0 0 > Probe color at (0,0) > Expected: 0 255 0 0 > Observed: 0 0 0 0 > Probe color at (0,0) > Expected: 0 255 0 0 > Observed: 0 0 0 0 > Probe color at (0,0) > Expected: 0 255 0 0 > Observed: 0 0 0 0 > > Is need to make additional test witch cover that behavior, or add assert to > that test? I'm not sure what you'd assert in the max-size test. A new test can be created or an existing one modified. I'm guessing adding a test for GL_ARB_shader_image_size would be the easiest way to test the issue. (In reply to Nanley Chery from comment #2) > I'm not sure what you'd assert in the max-size test. A new test can be > created or an existing one modified. I'm guessing adding a test for > GL_ARB_shader_image_size would be the easiest way to test the issue. I have wrote test to piglit https://patchwork.freedesktop.org/patch/227961/ that is the latest version. That test passes on radeon and fails on i965. For some reason a new mail thread was created for that patch. Here is a link to original mail thread. https://patchwork.freedesktop.org/patch/227613/ (In reply to Illia from comment #3) > (In reply to Nanley Chery from comment #2) > > > I'm not sure what you'd assert in the max-size test. A new test can be > > created or an existing one modified. I'm guessing adding a test for > > GL_ARB_shader_image_size would be the easiest way to test the issue. > > I have wrote test to piglit https://patchwork.freedesktop.org/patch/227961/ > that is the latest version. That test passes on radeon and fails on i965. > For some reason a new mail thread was created for that patch. Here is a link > to original mail thread. https://patchwork.freedesktop.org/patch/227613/ Sorry, but this test doesn't cover the situation mentioned here. It's completely valid for a texture buffer to have a buffer object larger than 2^27B bound to it. The issue mentioned here was that the size field in the surface state object for buffers wasn't clamped to 2^27B as mentioned in the texture_buffer spec. More information about these surface state objects can be found in the function, brw_emit_buffer_surface_state(). Curro is this fixed by? commit 156d2c6e621d836c4d45c636b87669e1de3d4464 Author: Francisco Jerez <currojerez@riseup.net> Date: Fri Mar 16 13:06:26 2018 -0700 i965: Move buffer texture size calculation into a common helper function. The buffer texture size calculations (should be easy enough, right?) are repeated in three different places, each of them subtly broken in a different way. E.g. the image load/store path was never fixed to clamp to MaxTextureBufferSize, and none of them are taking into account the buffer offset correctly. It's easier to fix it all in one place. Cc: mesa-stable@lists.freedesktop.org Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106481 Reviewed-by: Nanley Chery <nanley.g.chery@intel.com> (In reply to Andrés Gómez García from comment #5) > 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? -- GitLab Migration Automatic Message -- This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/mesa/mesa/issues/1723. |
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_texture_buffer_object extension spec: While buffer textures can be substantially larger than equivalent one-dimensional textures; the maximum texture size supported for buffer textures in the initial implementation of this extension is 2^27 texels, versus 2^13 (8192) texels for otherwise equivalent one-dimensional textures. [...] There's a bug in i965, that allows the creation of buffer surface states with sizes greater than 2^27 texels when doing image load/store operations on texture buffers. This patch fixes the issue: https://patchwork.freedesktop.org/patch/211341/ There doesn't seem to be any piglit tests which exercise this path. We should create one to prevent this bug from showing up again.