[v2] fbo-blit-check-limits: New test

Submitted by Sergii Romantsov on Feb. 5, 2019, 7:52 a.m.

Details

Message ID 1549353167-31774-1-git-send-email-sergii.romantsov@globallogic.com
State New
Headers show
Series "fbo-blit-check-limits: New test" ( rev: 2 ) in Piglit

Not browsing as part of any series.

Commit Message

Sergii Romantsov Feb. 5, 2019, 7:52 a.m.
From: Vadym Shovkoplias <vadim.shovkoplias@gmail.com>

This test checks max possible blit buffers sizes

v2: copyright updated

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108088
Signed-off-by: Vadym Shovkoplias <vadym.shovkoplias@globallogic.com>
---
 tests/fbo/CMakeLists.gl.txt       |  1 +
 tests/fbo/fbo-blit-check-limits.c | 85 +++++++++++++++++++++++++++++++++++++++
 tests/opengl.py                   |  1 +
 3 files changed, 87 insertions(+)
 create mode 100644 tests/fbo/fbo-blit-check-limits.c

Patch hide | download patch | download mbox

diff --git a/tests/fbo/CMakeLists.gl.txt b/tests/fbo/CMakeLists.gl.txt
index 1a1a607..e2c7b3a 100644
--- a/tests/fbo/CMakeLists.gl.txt
+++ b/tests/fbo/CMakeLists.gl.txt
@@ -91,6 +91,7 @@  piglit_add_executable (fbo-storage-formats fbo-storage-formats.c)
 piglit_add_executable (fbo-storage-completeness fbo-storage-completeness.c)
 piglit_add_executable (fbo-sys-blit fbo-sys-blit.c)
 piglit_add_executable (fbo-sys-sub-blit fbo-sys-sub-blit.c)
+piglit_add_executable (fbo-blit-check-limits fbo-blit-check-limits.c)
 piglit_add_executable (fbo-tex-rgbx fbo-tex-rgbx.c)
 piglit_add_executable (fbo-pbo-readpixels-small fbo-pbo-readpixels-small.c)
 piglit_add_executable (fbo-copyteximage fbo-copyteximage.c)
diff --git a/tests/fbo/fbo-blit-check-limits.c b/tests/fbo/fbo-blit-check-limits.c
new file mode 100644
index 0000000..92f54df
--- /dev/null
+++ b/tests/fbo/fbo-blit-check-limits.c
@@ -0,0 +1,85 @@ 
+/*
+ * Copyright (C) 2018 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ */
+
+/** @file fbo-blit-check-limits.c
+ *
+ * Test FBO blits with MAX possible buffer sizes
+ * Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108088
+ *
+ * \author Vadym Shovkoplias <vadym.shovkoplias@globallogic.com>
+ */
+
+#include "piglit-util-gl.h"
+
+PIGLIT_GL_TEST_CONFIG_BEGIN
+
+	config.supports_gl_compat_version = 10;
+
+	config.window_visual = PIGLIT_GL_VISUAL_DOUBLE | PIGLIT_GL_VISUAL_RGB;
+	config.requires_displayed_window = true;
+	config.khr_no_error_support = PIGLIT_NO_ERRORS;
+
+PIGLIT_GL_TEST_CONFIG_END
+
+enum piglit_result piglit_display(void)
+{
+	const float green[] = {0.0f, 1.0f, 0.0f};
+	int w = piglit_width;
+	int h = piglit_height;
+	bool success = 1;
+
+	glDrawBuffer(GL_BACK);
+	/* back buffer green */
+	glClearColor(0.0f, 1.0f, 0.0f, 1.0f);
+	glClear(GL_COLOR_BUFFER_BIT);
+
+        glDrawBuffer(GL_FRONT);
+	/* front buffer red */
+	glClearColor(1.0f, 0.0f, 0.0f, 1.0f);
+	glClear(GL_COLOR_BUFFER_BIT);
+
+	glReadBuffer(GL_BACK);
+
+	glBlitFramebufferEXT(INT_MIN, INT_MIN, INT_MAX, INT_MAX,
+			     INT_MIN, INT_MIN, INT_MAX, INT_MAX,
+			     GL_COLOR_BUFFER_BIT, GL_NEAREST);
+
+	glReadBuffer(GL_FRONT);
+
+	/* the corners should be green */
+	success &= piglit_probe_pixel_rgb(0, 0, green);
+	success &= piglit_probe_pixel_rgb(w - 1, 0, green);
+	success &= piglit_probe_pixel_rgb(0, h - 1, green);
+	success &= piglit_probe_pixel_rgb(w - 1, h - 1, green);
+
+	glFlush();
+
+	return success ? PIGLIT_PASS : PIGLIT_FAIL;
+}
+
+void piglit_init(int argc, char **argv)
+{
+	piglit_require_extension("GL_EXT_framebuffer_object");
+	piglit_require_extension("GL_EXT_framebuffer_blit");
+}
diff --git a/tests/opengl.py b/tests/opengl.py
index af68560..0be2980 100644
--- a/tests/opengl.py
+++ b/tests/opengl.py
@@ -2780,6 +2780,7 @@  with profile.test_list.group_manager(
     g(['fbo-readdrawpix'], run_concurrent=False)
     g(['fbo-sys-blit'], run_concurrent=False)
     g(['fbo-sys-sub-blit'], run_concurrent=False)
+    g(['fbo-blit-check-limits'], run_concurrent=False)
     g(['fbo-generatemipmap-versus-READ_FRAMEBUFFER'])
 
 with profile.test_list.group_manager(

Comments

On 05/02/2019 07:52, Sergii Romantsov wrote:
> From: Vadym Shovkoplias <vadim.shovkoplias@gmail.com>
>
> This test checks max possible blit buffers sizes
>
> v2: copyright updated
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108088
> Signed-off-by: Vadym Shovkoplias <vadym.shovkoplias@globallogic.com>

Hi Sergii,

The bug opened about does not give any description about what is 
incorrect in the current implementations or what is being tested in the 
spec.

The only paragraph I could find in the spec related to this is :
"
The actual region written to the draw framebuffer is limited to the 
intersection of
the destination buffers being written, which may include multiple draw 
buffers,
the depth buffer, and/or the stencil buffer depending on mask. Whether 
or not the
source or destination regions are altered due to these limits, the 
scaling and offset
applied to pixels being transferred is performed as though no such 
limits were
present.
"

I'm struggling to understand whether the "intersection" is related to 
dimensions or the attachments.
Could give more context?

Thanks,

-Lionel


> ---
>   tests/fbo/CMakeLists.gl.txt       |  1 +
>   tests/fbo/fbo-blit-check-limits.c | 85 +++++++++++++++++++++++++++++++++++++++
>   tests/opengl.py                   |  1 +
>   3 files changed, 87 insertions(+)
>   create mode 100644 tests/fbo/fbo-blit-check-limits.c
>
> diff --git a/tests/fbo/CMakeLists.gl.txt b/tests/fbo/CMakeLists.gl.txt
> index 1a1a607..e2c7b3a 100644
> --- a/tests/fbo/CMakeLists.gl.txt
> +++ b/tests/fbo/CMakeLists.gl.txt
> @@ -91,6 +91,7 @@ piglit_add_executable (fbo-storage-formats fbo-storage-formats.c)
>   piglit_add_executable (fbo-storage-completeness fbo-storage-completeness.c)
>   piglit_add_executable (fbo-sys-blit fbo-sys-blit.c)
>   piglit_add_executable (fbo-sys-sub-blit fbo-sys-sub-blit.c)
> +piglit_add_executable (fbo-blit-check-limits fbo-blit-check-limits.c)
>   piglit_add_executable (fbo-tex-rgbx fbo-tex-rgbx.c)
>   piglit_add_executable (fbo-pbo-readpixels-small fbo-pbo-readpixels-small.c)
>   piglit_add_executable (fbo-copyteximage fbo-copyteximage.c)
> diff --git a/tests/fbo/fbo-blit-check-limits.c b/tests/fbo/fbo-blit-check-limits.c
> new file mode 100644
> index 0000000..92f54df
> --- /dev/null
> +++ b/tests/fbo/fbo-blit-check-limits.c
> @@ -0,0 +1,85 @@
> +/*
> + * Copyright (C) 2018 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> + * IN THE SOFTWARE.
> + *
> + */
> +
> +/** @file fbo-blit-check-limits.c
> + *
> + * Test FBO blits with MAX possible buffer sizes
> + * Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108088
> + *
> + * \author Vadym Shovkoplias <vadym.shovkoplias@globallogic.com>
> + */
> +
> +#include "piglit-util-gl.h"
> +
> +PIGLIT_GL_TEST_CONFIG_BEGIN
> +
> +	config.supports_gl_compat_version = 10;
> +
> +	config.window_visual = PIGLIT_GL_VISUAL_DOUBLE | PIGLIT_GL_VISUAL_RGB;
> +	config.requires_displayed_window = true;
> +	config.khr_no_error_support = PIGLIT_NO_ERRORS;
> +
> +PIGLIT_GL_TEST_CONFIG_END
> +
> +enum piglit_result piglit_display(void)
> +{
> +	const float green[] = {0.0f, 1.0f, 0.0f};
> +	int w = piglit_width;
> +	int h = piglit_height;
> +	bool success = 1;
> +
> +	glDrawBuffer(GL_BACK);
> +	/* back buffer green */
> +	glClearColor(0.0f, 1.0f, 0.0f, 1.0f);
> +	glClear(GL_COLOR_BUFFER_BIT);
> +
> +        glDrawBuffer(GL_FRONT);
> +	/* front buffer red */
> +	glClearColor(1.0f, 0.0f, 0.0f, 1.0f);
> +	glClear(GL_COLOR_BUFFER_BIT);
> +
> +	glReadBuffer(GL_BACK);
> +
> +	glBlitFramebufferEXT(INT_MIN, INT_MIN, INT_MAX, INT_MAX,
> +			     INT_MIN, INT_MIN, INT_MAX, INT_MAX,
> +			     GL_COLOR_BUFFER_BIT, GL_NEAREST);
> +
> +	glReadBuffer(GL_FRONT);
> +
> +	/* the corners should be green */
> +	success &= piglit_probe_pixel_rgb(0, 0, green);
> +	success &= piglit_probe_pixel_rgb(w - 1, 0, green);
> +	success &= piglit_probe_pixel_rgb(0, h - 1, green);
> +	success &= piglit_probe_pixel_rgb(w - 1, h - 1, green);
> +
> +	glFlush();
> +
> +	return success ? PIGLIT_PASS : PIGLIT_FAIL;
> +}
> +
> +void piglit_init(int argc, char **argv)
> +{
> +	piglit_require_extension("GL_EXT_framebuffer_object");
> +	piglit_require_extension("GL_EXT_framebuffer_blit");
> +}
> diff --git a/tests/opengl.py b/tests/opengl.py
> index af68560..0be2980 100644
> --- a/tests/opengl.py
> +++ b/tests/opengl.py
> @@ -2780,6 +2780,7 @@ with profile.test_list.group_manager(
>       g(['fbo-readdrawpix'], run_concurrent=False)
>       g(['fbo-sys-blit'], run_concurrent=False)
>       g(['fbo-sys-sub-blit'], run_concurrent=False)
> +    g(['fbo-blit-check-limits'], run_concurrent=False)
>       g(['fbo-generatemipmap-versus-READ_FRAMEBUFFER'])
>   
>   with profile.test_list.group_manager(
Thanks Sergii,

That was the explanation I was looking for :)
I think that should be part of the commit message.

You have a patch for mesa as well?

Thanks,

-Lionel

On 06/02/2019 10:25, Sergii Romantsov wrote:
> Hello, Lionel. Thanks for looking on that.
>
> Will try to explain:
> Assume that a size of framebuffer is 160*160
> We have a read-buffer and write-buffer.
> In general: reading of pixels of any region-size and writing them into 
> region-size that greater or equal to 160 should give us the same 
> results (whole area of write-buffer should be filled by color of 
> read-buffer). That is satisfied except we are using some big 
> pixel-values (dstX1, dstY1) starting from 0x7ffffff.
> I haven't found any restrictions in minimum/maximum values for 
> parameters (dstX0, dstY0) and (dstX1, dstY1), they are integers so 
> looks like they could be any valid integer.
>
> Test shows that for big dest-region a draw performed incorrectly.
> Correct blit of 160*160 region to dest (0xffffff, 0xffffff): 
> read160_to0xffffff.png
> Incorrect blit of 160*160 region to dest (0x7fffffff, 0x7fffffff): 
> read160_to0x7fffffff.png
> Incorrect blit of INT_MAX*INT_MAX region to dest (INT_MAX, INT_MAX): 
> readIntMax_toIntMax.png
>
> On Tue, Feb 5, 2019 at 1:13 PM Lionel Landwerlin 
> <lionel.g.landwerlin@intel.com <mailto:lionel.g.landwerlin@intel.com>> 
> wrote:
>
>     On 05/02/2019 07:52, Sergii Romantsov wrote:
>     > From: Vadym Shovkoplias <vadim.shovkoplias@gmail.com
>     <mailto:vadim.shovkoplias@gmail.com>>
>     >
>     > This test checks max possible blit buffers sizes
>     >
>     > v2: copyright updated
>     >
>     > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108088
>     > Signed-off-by: Vadym Shovkoplias
>     <vadym.shovkoplias@globallogic.com
>     <mailto:vadym.shovkoplias@globallogic.com>>
>
>     Hi Sergii,
>
>     The bug opened about does not give any description about what is
>     incorrect in the current implementations or what is being tested
>     in the
>     spec.
>
>     The only paragraph I could find in the spec related to this is :
>     "
>     The actual region written to the draw framebuffer is limited to the
>     intersection of
>     the destination buffers being written, which may include multiple
>     draw
>     buffers,
>     the depth buffer, and/or the stencil buffer depending on mask.
>     Whether
>     or not the
>     source or destination regions are altered due to these limits, the
>     scaling and offset
>     applied to pixels being transferred is performed as though no such
>     limits were
>     present.
>     "
>
>     I'm struggling to understand whether the "intersection" is related to
>     dimensions or the attachments.
>     Could give more context?
>
>     Thanks,
>
>     -Lionel
>
>
>     > ---
>     >   tests/fbo/CMakeLists.gl.txt       |  1 +
>     >   tests/fbo/fbo-blit-check-limits.c | 85
>     +++++++++++++++++++++++++++++++++++++++
>     >   tests/opengl.py                   |  1 +
>     >   3 files changed, 87 insertions(+)
>     >   create mode 100644 tests/fbo/fbo-blit-check-limits.c
>     >
>     > diff --git a/tests/fbo/CMakeLists.gl.txt
>     b/tests/fbo/CMakeLists.gl.txt
>     > index 1a1a607..e2c7b3a 100644
>     > --- a/tests/fbo/CMakeLists.gl.txt
>     > +++ b/tests/fbo/CMakeLists.gl.txt
>     > @@ -91,6 +91,7 @@ piglit_add_executable (fbo-storage-formats
>     fbo-storage-formats.c)
>     >   piglit_add_executable (fbo-storage-completeness
>     fbo-storage-completeness.c)
>     >   piglit_add_executable (fbo-sys-blit fbo-sys-blit.c)
>     >   piglit_add_executable (fbo-sys-sub-blit fbo-sys-sub-blit.c)
>     > +piglit_add_executable (fbo-blit-check-limits
>     fbo-blit-check-limits.c)
>     >   piglit_add_executable (fbo-tex-rgbx fbo-tex-rgbx.c)
>     >   piglit_add_executable (fbo-pbo-readpixels-small
>     fbo-pbo-readpixels-small.c)
>     >   piglit_add_executable (fbo-copyteximage fbo-copyteximage.c)
>     > diff --git a/tests/fbo/fbo-blit-check-limits.c
>     b/tests/fbo/fbo-blit-check-limits.c
>     > new file mode 100644
>     > index 0000000..92f54df
>     > --- /dev/null
>     > +++ b/tests/fbo/fbo-blit-check-limits.c
>     > @@ -0,0 +1,85 @@
>     > +/*
>     > + * Copyright (C) 2018 Intel Corporation
>     > + *
>     > + * Permission is hereby granted, free of charge, to any person
>     obtaining a
>     > + * copy of this software and associated documentation files
>     (the "Software"),
>     > + * to deal in the Software without restriction, including
>     without limitation
>     > + * the rights to use, copy, modify, merge, publish, distribute,
>     sublicense,
>     > + * and/or sell copies of the Software, and to permit persons to
>     whom the
>     > + * Software is furnished to do so, subject to the following
>     conditions:
>     > + *
>     > + * The above copyright notice and this permission notice
>     (including the next
>     > + * paragraph) shall be included in all copies or substantial
>     portions of the
>     > + * Software.
>     > + *
>     > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY
>     KIND, EXPRESS OR
>     > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
>     MERCHANTABILITY,
>     > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO
>     EVENT SHALL
>     > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM,
>     DAMAGES OR OTHER
>     > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
>     OTHERWISE, ARISING
>     > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE
>     OR OTHER DEALINGS
>     > + * IN THE SOFTWARE.
>     > + *
>     > + */
>     > +
>     > +/** @file fbo-blit-check-limits.c
>     > + *
>     > + * Test FBO blits with MAX possible buffer sizes
>     > + * Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108088
>     > + *
>     > + * \author Vadym Shovkoplias <vadym.shovkoplias@globallogic.com
>     <mailto:vadym.shovkoplias@globallogic.com>>
>     > + */
>     > +
>     > +#include "piglit-util-gl.h"
>     > +
>     > +PIGLIT_GL_TEST_CONFIG_BEGIN
>     > +
>     > +     config.supports_gl_compat_version = 10;
>     > +
>     > +     config.window_visual = PIGLIT_GL_VISUAL_DOUBLE |
>     PIGLIT_GL_VISUAL_RGB;
>     > +     config.requires_displayed_window = true;
>     > +     config.khr_no_error_support = PIGLIT_NO_ERRORS;
>     > +
>     > +PIGLIT_GL_TEST_CONFIG_END
>     > +
>     > +enum piglit_result piglit_display(void)
>     > +{
>     > +     const float green[] = {0.0f, 1.0f, 0.0f};
>     > +     int w = piglit_width;
>     > +     int h = piglit_height;
>     > +     bool success = 1;
>     > +
>     > +     glDrawBuffer(GL_BACK);
>     > +     /* back buffer green */
>     > +     glClearColor(0.0f, 1.0f, 0.0f, 1.0f);
>     > +     glClear(GL_COLOR_BUFFER_BIT);
>     > +
>     > +        glDrawBuffer(GL_FRONT);
>     > +     /* front buffer red */
>     > +     glClearColor(1.0f, 0.0f, 0.0f, 1.0f);
>     > +     glClear(GL_COLOR_BUFFER_BIT);
>     > +
>     > +     glReadBuffer(GL_BACK);
>     > +
>     > +     glBlitFramebufferEXT(INT_MIN, INT_MIN, INT_MAX, INT_MAX,
>     > +                          INT_MIN, INT_MIN, INT_MAX, INT_MAX,
>     > +                          GL_COLOR_BUFFER_BIT, GL_NEAREST);
>     > +
>     > +     glReadBuffer(GL_FRONT);
>     > +
>     > +     /* the corners should be green */
>     > +     success &= piglit_probe_pixel_rgb(0, 0, green);
>     > +     success &= piglit_probe_pixel_rgb(w - 1, 0, green);
>     > +     success &= piglit_probe_pixel_rgb(0, h - 1, green);
>     > +     success &= piglit_probe_pixel_rgb(w - 1, h - 1, green);
>     > +
>     > +     glFlush();
>     > +
>     > +     return success ? PIGLIT_PASS : PIGLIT_FAIL;
>     > +}
>     > +
>     > +void piglit_init(int argc, char **argv)
>     > +{
>     > +  piglit_require_extension("GL_EXT_framebuffer_object");
>     > +  piglit_require_extension("GL_EXT_framebuffer_blit");
>     > +}
>     > diff --git a/tests/opengl.py b/tests/opengl.py
>     > index af68560..0be2980 100644
>     > --- a/tests/opengl.py
>     > +++ b/tests/opengl.py
>     > @@ -2780,6 +2780,7 @@ with profile.test_list.group_manager(
>     >       g(['fbo-readdrawpix'], run_concurrent=False)
>     >       g(['fbo-sys-blit'], run_concurrent=False)
>     >       g(['fbo-sys-sub-blit'], run_concurrent=False)
>     > +    g(['fbo-blit-check-limits'], run_concurrent=False)
>     >       g(['fbo-generatemipmap-versus-READ_FRAMEBUFFER'])
>     >
>     >   with profile.test_list.group_manager(
>
>
>
> I think that should be part of the commit message.

Ok, will add. Also in plans is to extend a test


> You have a patch for mesa as well?

We have initial version (but looks like it is incomplete):
https://patchwork.freedesktop.org/patch/253443/ and one more
partly-related: https://patchwork.freedesktop.org/patch/248561/ where Jason
said:

I think we likely want to
either do a full audit of all blorp_blit callers or somehow solve it in
blorp_blit.  The fact that we're using floats at all bothers me quite a bit
because it means we're likely loosing some precision somewhere.

So in plans also is to try to use integers... What is your opinion?

On Wed, Feb 6, 2019 at 12:46 PM Lionel Landwerlin <
lionel.g.landwerlin@intel.com> wrote:

> Thanks Sergii,
>
> That was the explanation I was looking for :)
> I think that should be part of the commit message.
>
> You have a patch for mesa as well?
>
> Thanks,
>
> -Lionel
>
> On 06/02/2019 10:25, Sergii Romantsov wrote:
>
> Hello, Lionel. Thanks for looking on that.
>
> Will try to explain:
> Assume that a size of framebuffer is 160*160
> We have a read-buffer and write-buffer.
> In general: reading of pixels of any region-size and writing them into
> region-size that greater or equal to 160 should give us the same results
> (whole area of write-buffer should be filled by color of read-buffer). That
> is satisfied except we are using some big pixel-values (dstX1, dstY1)
> starting from 0x7ffffff.
> I haven't found any restrictions in minimum/maximum values for parameters
> (dstX0, dstY0) and (dstX1, dstY1), they are integers so looks like they
> could be any valid integer.
>
> Test shows that for big dest-region a draw performed incorrectly.
> Correct blit of 160*160 region to dest (0xffffff, 0xffffff):
> read160_to0xffffff.png
> Incorrect blit of 160*160 region to dest (0x7fffffff, 0x7fffffff):
> read160_to0x7fffffff.png
> Incorrect blit of INT_MAX*INT_MAX region to dest (INT_MAX, INT_MAX):
> readIntMax_toIntMax.png
>
> On Tue, Feb 5, 2019 at 1:13 PM Lionel Landwerlin <
> lionel.g.landwerlin@intel.com> wrote:
>
>> On 05/02/2019 07:52, Sergii Romantsov wrote:
>> > From: Vadym Shovkoplias <vadim.shovkoplias@gmail.com>
>> >
>> > This test checks max possible blit buffers sizes
>> >
>> > v2: copyright updated
>> >
>> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108088
>> > Signed-off-by: Vadym Shovkoplias <vadym.shovkoplias@globallogic.com>
>>
>> Hi Sergii,
>>
>> The bug opened about does not give any description about what is
>> incorrect in the current implementations or what is being tested in the
>> spec.
>>
>> The only paragraph I could find in the spec related to this is :
>> "
>> The actual region written to the draw framebuffer is limited to the
>> intersection of
>> the destination buffers being written, which may include multiple draw
>> buffers,
>> the depth buffer, and/or the stencil buffer depending on mask. Whether
>> or not the
>> source or destination regions are altered due to these limits, the
>> scaling and offset
>> applied to pixels being transferred is performed as though no such
>> limits were
>> present.
>> "
>>
>> I'm struggling to understand whether the "intersection" is related to
>> dimensions or the attachments.
>> Could give more context?
>>
>> Thanks,
>>
>> -Lionel
>>
>>
>> > ---
>> >   tests/fbo/CMakeLists.gl.txt       |  1 +
>> >   tests/fbo/fbo-blit-check-limits.c | 85
>> +++++++++++++++++++++++++++++++++++++++
>> >   tests/opengl.py                   |  1 +
>> >   3 files changed, 87 insertions(+)
>> >   create mode 100644 tests/fbo/fbo-blit-check-limits.c
>> >
>> > diff --git a/tests/fbo/CMakeLists.gl.txt b/tests/fbo/CMakeLists.gl.txt
>> > index 1a1a607..e2c7b3a 100644
>> > --- a/tests/fbo/CMakeLists.gl.txt
>> > +++ b/tests/fbo/CMakeLists.gl.txt
>> > @@ -91,6 +91,7 @@ piglit_add_executable (fbo-storage-formats
>> fbo-storage-formats.c)
>> >   piglit_add_executable (fbo-storage-completeness
>> fbo-storage-completeness.c)
>> >   piglit_add_executable (fbo-sys-blit fbo-sys-blit.c)
>> >   piglit_add_executable (fbo-sys-sub-blit fbo-sys-sub-blit.c)
>> > +piglit_add_executable (fbo-blit-check-limits fbo-blit-check-limits.c)
>> >   piglit_add_executable (fbo-tex-rgbx fbo-tex-rgbx.c)
>> >   piglit_add_executable (fbo-pbo-readpixels-small
>> fbo-pbo-readpixels-small.c)
>> >   piglit_add_executable (fbo-copyteximage fbo-copyteximage.c)
>> > diff --git a/tests/fbo/fbo-blit-check-limits.c
>> b/tests/fbo/fbo-blit-check-limits.c
>> > new file mode 100644
>> > index 0000000..92f54df
>> > --- /dev/null
>> > +++ b/tests/fbo/fbo-blit-check-limits.c
>> > @@ -0,0 +1,85 @@
>> > +/*
>> > + * Copyright (C) 2018 Intel Corporation
>> > + *
>> > + * Permission is hereby granted, free of charge, to any person
>> obtaining a
>> > + * copy of this software and associated documentation files (the
>> "Software"),
>> > + * to deal in the Software without restriction, including without
>> limitation
>> > + * the rights to use, copy, modify, merge, publish, distribute,
>> sublicense,
>> > + * and/or sell copies of the Software, and to permit persons to whom
>> the
>> > + * Software is furnished to do so, subject to the following conditions:
>> > + *
>> > + * The above copyright notice and this permission notice (including
>> the next
>> > + * paragraph) shall be included in all copies or substantial portions
>> of the
>> > + * Software.
>> > + *
>> > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
>> EXPRESS OR
>> > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
>> MERCHANTABILITY,
>> > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT
>> SHALL
>> > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES
>> OR OTHER
>> > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
>> ARISING
>> > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
>> DEALINGS
>> > + * IN THE SOFTWARE.
>> > + *
>> > + */
>> > +
>> > +/** @file fbo-blit-check-limits.c
>> > + *
>> > + * Test FBO blits with MAX possible buffer sizes
>> > + * Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108088
>> > + *
>> > + * \author Vadym Shovkoplias <vadym.shovkoplias@globallogic.com>
>> > + */
>> > +
>> > +#include "piglit-util-gl.h"
>> > +
>> > +PIGLIT_GL_TEST_CONFIG_BEGIN
>> > +
>> > +     config.supports_gl_compat_version = 10;
>> > +
>> > +     config.window_visual = PIGLIT_GL_VISUAL_DOUBLE |
>> PIGLIT_GL_VISUAL_RGB;
>> > +     config.requires_displayed_window = true;
>> > +     config.khr_no_error_support = PIGLIT_NO_ERRORS;
>> > +
>> > +PIGLIT_GL_TEST_CONFIG_END
>> > +
>> > +enum piglit_result piglit_display(void)
>> > +{
>> > +     const float green[] = {0.0f, 1.0f, 0.0f};
>> > +     int w = piglit_width;
>> > +     int h = piglit_height;
>> > +     bool success = 1;
>> > +
>> > +     glDrawBuffer(GL_BACK);
>> > +     /* back buffer green */
>> > +     glClearColor(0.0f, 1.0f, 0.0f, 1.0f);
>> > +     glClear(GL_COLOR_BUFFER_BIT);
>> > +
>> > +        glDrawBuffer(GL_FRONT);
>> > +     /* front buffer red */
>> > +     glClearColor(1.0f, 0.0f, 0.0f, 1.0f);
>> > +     glClear(GL_COLOR_BUFFER_BIT);
>> > +
>> > +     glReadBuffer(GL_BACK);
>> > +
>> > +     glBlitFramebufferEXT(INT_MIN, INT_MIN, INT_MAX, INT_MAX,
>> > +                          INT_MIN, INT_MIN, INT_MAX, INT_MAX,
>> > +                          GL_COLOR_BUFFER_BIT, GL_NEAREST);
>> > +
>> > +     glReadBuffer(GL_FRONT);
>> > +
>> > +     /* the corners should be green */
>> > +     success &= piglit_probe_pixel_rgb(0, 0, green);
>> > +     success &= piglit_probe_pixel_rgb(w - 1, 0, green);
>> > +     success &= piglit_probe_pixel_rgb(0, h - 1, green);
>> > +     success &= piglit_probe_pixel_rgb(w - 1, h - 1, green);
>> > +
>> > +     glFlush();
>> > +
>> > +     return success ? PIGLIT_PASS : PIGLIT_FAIL;
>> > +}
>> > +
>> > +void piglit_init(int argc, char **argv)
>> > +{
>> > +     piglit_require_extension("GL_EXT_framebuffer_object");
>> > +     piglit_require_extension("GL_EXT_framebuffer_blit");
>> > +}
>> > diff --git a/tests/opengl.py b/tests/opengl.py
>> > index af68560..0be2980 100644
>> > --- a/tests/opengl.py
>> > +++ b/tests/opengl.py
>> > @@ -2780,6 +2780,7 @@ with profile.test_list.group_manager(
>> >       g(['fbo-readdrawpix'], run_concurrent=False)
>> >       g(['fbo-sys-blit'], run_concurrent=False)
>> >       g(['fbo-sys-sub-blit'], run_concurrent=False)
>> > +    g(['fbo-blit-check-limits'], run_concurrent=False)
>> >       g(['fbo-generatemipmap-versus-READ_FRAMEBUFFER'])
>> >
>> >   with profile.test_list.group_manager(
>>
>>
>>
>
On 06/02/2019 11:29, Sergii Romantsov wrote:
>
>     I think that should be part of the commit message.
>
> Ok, will add. Also in plans is to extend a test
>
>     You have a patch for mesa as well?
>
> We have initial version (but looks like it is incomplete): 
> https://patchwork.freedesktop.org/patch/253443/ and one more 
> partly-related: https://patchwork.freedesktop.org/patch/248561/ where 
> Jason said:
> I think we likely want to
> either do a full audit of all blorp_blit callers or somehow solve it in
> blorp_blit.  The fact that we're using floats at all bothers me quite a bit
> because it means we're likely loosing some precision somewhere.


I agree with Jason. As far as I can tell, all the call site of 
blorp_blit() are using integers.

It's only brw_blorp_blit_miptrees() that starts taking floats, leading 
to precision issues.

I think we should try to make brw_blorp_blit_miptrees() & blorp 
functions take integers.

That sounds like a cleaner fix, rather than trying to bump the precision 
(there might still be cases where that won't workout well).


-Lionel


> So in plans also is to try to use integers... What is your opinion?
>
> On Wed, Feb 6, 2019 at 12:46 PM Lionel Landwerlin 
> <lionel.g.landwerlin@intel.com <mailto:lionel.g.landwerlin@intel.com>> 
> wrote:
>
>     Thanks Sergii,
>
>     That was the explanation I was looking for :)
>     I think that should be part of the commit message.
>
>     You have a patch for mesa as well?
>
>     Thanks,
>
>     -Lionel
>
>     On 06/02/2019 10:25, Sergii Romantsov wrote:
>>     Hello, Lionel. Thanks for looking on that.
>>
>>     Will try to explain:
>>     Assume that a size of framebuffer is 160*160
>>     We have a read-buffer and write-buffer.
>>     In general: reading of pixels of any region-size and writing them
>>     into region-size that greater or equal to 160 should give us the
>>     same results (whole area of write-buffer should be filled by
>>     color of read-buffer). That is satisfied except we are using some
>>     big pixel-values (dstX1, dstY1) starting from 0x7ffffff.
>>     I haven't found any restrictions in minimum/maximum values for
>>     parameters (dstX0, dstY0) and (dstX1, dstY1), they are integers
>>     so looks like they could be any valid integer.
>>
>>     Test shows that for big dest-region a draw performed incorrectly.
>>     Correct blit of 160*160 region to dest (0xffffff, 0xffffff):
>>     read160_to0xffffff.png
>>     Incorrect blit of 160*160 region to dest (0x7fffffff,
>>     0x7fffffff): read160_to0x7fffffff.png
>>     Incorrect blit of INT_MAX*INT_MAX region to dest (INT_MAX,
>>     INT_MAX): readIntMax_toIntMax.png
>>
>>     On Tue, Feb 5, 2019 at 1:13 PM Lionel Landwerlin
>>     <lionel.g.landwerlin@intel.com
>>     <mailto:lionel.g.landwerlin@intel.com>> wrote:
>>
>>         On 05/02/2019 07:52, Sergii Romantsov wrote:
>>         > From: Vadym Shovkoplias <vadim.shovkoplias@gmail.com
>>         <mailto:vadim.shovkoplias@gmail.com>>
>>         >
>>         > This test checks max possible blit buffers sizes
>>         >
>>         > v2: copyright updated
>>         >
>>         > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108088
>>         > Signed-off-by: Vadym Shovkoplias
>>         <vadym.shovkoplias@globallogic.com
>>         <mailto:vadym.shovkoplias@globallogic.com>>
>>
>>         Hi Sergii,
>>
>>         The bug opened about does not give any description about what is
>>         incorrect in the current implementations or what is being
>>         tested in the
>>         spec.
>>
>>         The only paragraph I could find in the spec related to this is :
>>         "
>>         The actual region written to the draw framebuffer is limited
>>         to the
>>         intersection of
>>         the destination buffers being written, which may include
>>         multiple draw
>>         buffers,
>>         the depth buffer, and/or the stencil buffer depending on
>>         mask. Whether
>>         or not the
>>         source or destination regions are altered due to these
>>         limits, the
>>         scaling and offset
>>         applied to pixels being transferred is performed as though no
>>         such
>>         limits were
>>         present.
>>         "
>>
>>         I'm struggling to understand whether the "intersection" is
>>         related to
>>         dimensions or the attachments.
>>         Could give more context?
>>
>>         Thanks,
>>
>>         -Lionel
>>
>>
>>         > ---
>>         >   tests/fbo/CMakeLists.gl.txt       |  1 +
>>         >   tests/fbo/fbo-blit-check-limits.c | 85
>>         +++++++++++++++++++++++++++++++++++++++
>>         >   tests/opengl.py                   |  1 +
>>         >   3 files changed, 87 insertions(+)
>>         >   create mode 100644 tests/fbo/fbo-blit-check-limits.c
>>         >
>>         > diff --git a/tests/fbo/CMakeLists.gl.txt
>>         b/tests/fbo/CMakeLists.gl.txt
>>         > index 1a1a607..e2c7b3a 100644
>>         > --- a/tests/fbo/CMakeLists.gl.txt
>>         > +++ b/tests/fbo/CMakeLists.gl.txt
>>         > @@ -91,6 +91,7 @@ piglit_add_executable
>>         (fbo-storage-formats fbo-storage-formats.c)
>>         >   piglit_add_executable (fbo-storage-completeness
>>         fbo-storage-completeness.c)
>>         >   piglit_add_executable (fbo-sys-blit fbo-sys-blit.c)
>>         >   piglit_add_executable (fbo-sys-sub-blit fbo-sys-sub-blit.c)
>>         > +piglit_add_executable (fbo-blit-check-limits
>>         fbo-blit-check-limits.c)
>>         >   piglit_add_executable (fbo-tex-rgbx fbo-tex-rgbx.c)
>>         >   piglit_add_executable (fbo-pbo-readpixels-small
>>         fbo-pbo-readpixels-small.c)
>>         >   piglit_add_executable (fbo-copyteximage fbo-copyteximage.c)
>>         > diff --git a/tests/fbo/fbo-blit-check-limits.c
>>         b/tests/fbo/fbo-blit-check-limits.c
>>         > new file mode 100644
>>         > index 0000000..92f54df
>>         > --- /dev/null
>>         > +++ b/tests/fbo/fbo-blit-check-limits.c
>>         > @@ -0,0 +1,85 @@
>>         > +/*
>>         > + * Copyright (C) 2018 Intel Corporation
>>         > + *
>>         > + * Permission is hereby granted, free of charge, to any
>>         person obtaining a
>>         > + * copy of this software and associated documentation
>>         files (the "Software"),
>>         > + * to deal in the Software without restriction, including
>>         without limitation
>>         > + * the rights to use, copy, modify, merge, publish,
>>         distribute, sublicense,
>>         > + * and/or sell copies of the Software, and to permit
>>         persons to whom the
>>         > + * Software is furnished to do so, subject to the
>>         following conditions:
>>         > + *
>>         > + * The above copyright notice and this permission notice
>>         (including the next
>>         > + * paragraph) shall be included in all copies or
>>         substantial portions of the
>>         > + * Software.
>>         > + *
>>         > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF
>>         ANY KIND, EXPRESS OR
>>         > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
>>         MERCHANTABILITY,
>>         > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. 
>>         IN NO EVENT SHALL
>>         > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY
>>         CLAIM, DAMAGES OR OTHER
>>         > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
>>         OTHERWISE, ARISING
>>         > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
>>         USE OR OTHER DEALINGS
>>         > + * IN THE SOFTWARE.
>>         > + *
>>         > + */
>>         > +
>>         > +/** @file fbo-blit-check-limits.c
>>         > + *
>>         > + * Test FBO blits with MAX possible buffer sizes
>>         > + * Bugzilla:
>>         https://bugs.freedesktop.org/show_bug.cgi?id=108088
>>         > + *
>>         > + * \author Vadym Shovkoplias
>>         <vadym.shovkoplias@globallogic.com
>>         <mailto:vadym.shovkoplias@globallogic.com>>
>>         > + */
>>         > +
>>         > +#include "piglit-util-gl.h"
>>         > +
>>         > +PIGLIT_GL_TEST_CONFIG_BEGIN
>>         > +
>>         > +     config.supports_gl_compat_version = 10;
>>         > +
>>         > +     config.window_visual = PIGLIT_GL_VISUAL_DOUBLE |
>>         PIGLIT_GL_VISUAL_RGB;
>>         > +     config.requires_displayed_window = true;
>>         > +     config.khr_no_error_support = PIGLIT_NO_ERRORS;
>>         > +
>>         > +PIGLIT_GL_TEST_CONFIG_END
>>         > +
>>         > +enum piglit_result piglit_display(void)
>>         > +{
>>         > +     const float green[] = {0.0f, 1.0f, 0.0f};
>>         > +     int w = piglit_width;
>>         > +     int h = piglit_height;
>>         > +     bool success = 1;
>>         > +
>>         > +     glDrawBuffer(GL_BACK);
>>         > +     /* back buffer green */
>>         > +     glClearColor(0.0f, 1.0f, 0.0f, 1.0f);
>>         > +     glClear(GL_COLOR_BUFFER_BIT);
>>         > +
>>         > +        glDrawBuffer(GL_FRONT);
>>         > +     /* front buffer red */
>>         > +     glClearColor(1.0f, 0.0f, 0.0f, 1.0f);
>>         > +     glClear(GL_COLOR_BUFFER_BIT);
>>         > +
>>         > +     glReadBuffer(GL_BACK);
>>         > +
>>         > +     glBlitFramebufferEXT(INT_MIN, INT_MIN, INT_MAX, INT_MAX,
>>         > +                          INT_MIN, INT_MIN, INT_MAX, INT_MAX,
>>         > +                          GL_COLOR_BUFFER_BIT, GL_NEAREST);
>>         > +
>>         > +     glReadBuffer(GL_FRONT);
>>         > +
>>         > +     /* the corners should be green */
>>         > +     success &= piglit_probe_pixel_rgb(0, 0, green);
>>         > +     success &= piglit_probe_pixel_rgb(w - 1, 0, green);
>>         > +     success &= piglit_probe_pixel_rgb(0, h - 1, green);
>>         > +     success &= piglit_probe_pixel_rgb(w - 1, h - 1, green);
>>         > +
>>         > +     glFlush();
>>         > +
>>         > +     return success ? PIGLIT_PASS : PIGLIT_FAIL;
>>         > +}
>>         > +
>>         > +void piglit_init(int argc, char **argv)
>>         > +{
>>         > +  piglit_require_extension("GL_EXT_framebuffer_object");
>>         > +  piglit_require_extension("GL_EXT_framebuffer_blit");
>>         > +}
>>         > diff --git a/tests/opengl.py b/tests/opengl.py
>>         > index af68560..0be2980 100644
>>         > --- a/tests/opengl.py
>>         > +++ b/tests/opengl.py
>>         > @@ -2780,6 +2780,7 @@ with profile.test_list.group_manager(
>>         >       g(['fbo-readdrawpix'], run_concurrent=False)
>>         >       g(['fbo-sys-blit'], run_concurrent=False)
>>         >       g(['fbo-sys-sub-blit'], run_concurrent=False)
>>         > +    g(['fbo-blit-check-limits'], run_concurrent=False)
>>         >  g(['fbo-generatemipmap-versus-READ_FRAMEBUFFER'])
>>         >
>>         >   with profile.test_list.group_manager(
>>
>>
>