On 20/06/18 20:49, Nanley Chery wrote:
On Wed, Jun 20, 2018 at 06:32:58PM +0100, Lionel Landwerlin wrote:
On 19/06/18 18:53, Nanley Chery wrote:
On Tue, Jun 19, 2018 at 11:03:17AM +0100, Lionel Landwerlin wrote:
On 18/06/18 19:57, Nanley Chery wrote:
Change the dimension of the texture to test how i965 handles having a
row pitch too large for the BLT engine.

v2: Query the maximum supported texture width (Ilia Mirkin)
---
    tests/texturing/getteximage-simple.c | 25 +++++++++++++++++--------
    1 file changed, 17 insertions(+), 8 deletions(-)

diff --git a/tests/texturing/getteximage-simple.c 
b/tests/texturing/getteximage-simple.c
index 525e6a392..5b87b7ccd 100644
--- a/tests/texturing/getteximage-simple.c
+++ b/tests/texturing/getteximage-simple.c
@@ -8,6 +8,10 @@
     * texture images is executed before the readback.
     *
     * This used to crash for R300+bufmgr.
+ *
+ * This also used to stress test the blit methods in i965. The BLT engine only
+ * supports pitch sizes up to but not including 32768 dwords. BLORP supports
+ * even larger sizes.
     */
    #include "piglit-util-gl.h"
@@ -21,10 +25,10 @@ PIGLIT_GL_TEST_CONFIG_BEGIN
    PIGLIT_GL_TEST_CONFIG_END
-#define MAX_TYPE_VAL UINT8_MAX
-#define PIX_TYPE GLubyte
-#define TEX_TYPE GL_UNSIGNED_BYTE
-#define TEX_INT_FMT GL_RGBA8
+#define MAX_TYPE_VAL 1.0
+#define PIX_TYPE GLfloat
+#define TEX_TYPE GL_FLOAT
+#define TEX_INT_FMT GL_RGBA32F
    #define TEX_FMT GL_RGBA
    #define CHANNELS_PER_PIXEL 4
This is changing the format of the texture we're testing with.
Yes, a higher resolution format is needed to hit the row pitch of
interest on the older gens which have a max width of 8K.

In patch 3 I thought you wanted to make this configurable (through arguments
to the test maybe?), but this is just switching the format.
In patch 3, I attempted to make the test configurable via a group of
#defines.

Why not add a command line parameter?

I didn't want to add a command line parameter because there didn't seem
to be any benefit to doing so. By changing the format and width, we're
able to continue testing the issue in R300 as well as the teximage
upload and download paths of interest in i965. Thoughts?
Not really, I was just wondering whether that format change would mean that
some driver wouldn't get tested anymore.
Sorry for not mentioning it in the cover letter, but we do lose testing
on i915 which doesn't support GL_ARB_texture_float. I didn't feel bad
about this because it's not actively developed and the test was written
to catch a regression in the R300 driver (which does support the
extension AFACIT). Is that okay with you?

Yeah, I just wasn't sure whether you thought about it :)


-Nanley

I guess it's a common enough format that everybody probably supports it.

Thanks for the explanation :


Reviewed-by: Lionel Landwerlin <lionel.g.landwer...@intel.com>

-Nanley

Thanks,

-
Lionel

@@ -42,8 +46,8 @@ static bool test_getteximage(PIX_TYPE *data, size_t 
data_size, GLint w, GLint h)
                        const unsigned pixel_channel = i % CHANNELS_PER_PIXEL;
                        printf("GetTexImage() returns incorrect data in element 
%i\n", i);
                        printf("    corresponding to (%i,%i) channel %i\n", 
pixel % w, pixel / w, pixel_channel);
-                       printf("    expected: %i\n", data[i]);
-                       printf("    got: %i\n", compare[i]);
+                       printf("    expected: %f\n", data[i]);
+                       printf("    got: %f\n", compare[i]);
                        match = false;
                        break;
                }
@@ -53,11 +57,13 @@ static bool test_getteximage(PIX_TYPE *data, size_t 
data_size, GLint w, GLint h)
        return match;
    }
+
    enum piglit_result
    piglit_display(void)
    {
-       GLsizei height = 16;
-       GLsizei width = 64;
+       GLsizei height = 2;
+       GLsizei width;
+       glGetIntegerv(GL_MAX_TEXTURE_SIZE, &width);
        /* Upload random data to a texture with the given dimensions */
        const unsigned data_channels = width * height * CHANNELS_PER_PIXEL;
@@ -94,6 +100,9 @@ piglit_display(void)
    void piglit_init(int argc, char **argv)
    {
+       if (TEX_TYPE == GL_FLOAT)
+               piglit_require_extension("GL_ARB_texture_float");
+
        GLuint tex;
        glGenTextures(1, &tex);


_______________________________________________
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit

Reply via email to