<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<div class="moz-cite-prefix">On 06/02/2019 11:29, Sergii Romantsov
wrote:<br>
</div>
<blockquote type="cite"
cite="mid:CALWg4ioiAjW0XuD1fB7wQeu0DEQnm9Jqs5js_GzAri8JQfKjhQ@mail.gmail.com">
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<blockquote class="gmail_quote" style="margin:0px 0px 0px
0.8ex;border-left:1px solid
rgb(204,204,204);padding-left:1ex">I think that should be
part of the commit message.</blockquote>
<div>Ok, will add. Also in plans is to extend a test</div>
<div> </div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px
0.8ex;border-left:1px solid
rgb(204,204,204);padding-left:1ex">You have a patch for
mesa as well?</blockquote>
<div>We have initial version (but looks like it is
incomplete): <a
href="https://patchwork.freedesktop.org/patch/253443/"
moz-do-not-send="true">https://patchwork.freedesktop.org/patch/253443/</a>
and one more partly-related: <a
href="https://patchwork.freedesktop.org/patch/248561/"
moz-do-not-send="true">https://patchwork.freedesktop.org/patch/248561/</a>
where Jason said:<br>
</div>
<div>
<pre class="gmail-content" style="font-size:13px;box-sizing:border-box;overflow:auto;font-family:Menlo,Monaco,Consolas,"Courier New",monospace;padding:9.5px;margin-top:0px;margin-bottom:10px;line-height:14.3px;color:rgb(51,51,51);word-break:break-all;word-wrap:break-word;border:0px;border-radius:0px">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.</pre>
</div>
</div>
</div>
</div>
</blockquote>
<p><br>
</p>
<p>I agree with Jason. As far as I can tell, all the call site of
blorp_blit() are using integers.</p>
<p>It's only brw_blorp_blit_miptrees() that starts taking floats,
leading to precision issues.</p>
<p>I think we should try to make brw_blorp_blit_miptrees() &
blorp functions take integers.</p>
<p>That sounds like a cleaner fix, rather than trying to bump the
precision (there might still be cases where that won't workout
well).</p>
<p><br>
</p>
<p>-Lionel</p>
<p><br>
</p>
<blockquote type="cite"
cite="mid:CALWg4ioiAjW0XuD1fB7wQeu0DEQnm9Jqs5js_GzAri8JQfKjhQ@mail.gmail.com">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div>So in plans also is to try to use integers... What is
your opinion?</div>
</div>
</div>
</div>
<br>
<div class="gmail_quote">
<div dir="ltr" class="gmail_attr">On Wed, Feb 6, 2019 at 12:46
PM Lionel Landwerlin <<a
href="mailto:lionel.g.landwerlin@intel.com"
moz-do-not-send="true">lionel.g.landwerlin@intel.com</a>>
wrote:<br>
</div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px
0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div bgcolor="#FFFFFF">
<div class="gmail-m_5073674045632076502moz-cite-prefix">Thanks
Sergii,</div>
<div class="gmail-m_5073674045632076502moz-cite-prefix"><br>
</div>
<div class="gmail-m_5073674045632076502moz-cite-prefix">That
was the explanation I was looking for :)</div>
<div class="gmail-m_5073674045632076502moz-cite-prefix">I
think that should be part of the commit message.</div>
<div class="gmail-m_5073674045632076502moz-cite-prefix"><br>
</div>
<div class="gmail-m_5073674045632076502moz-cite-prefix">You
have a patch for mesa as well?</div>
<div class="gmail-m_5073674045632076502moz-cite-prefix"><br>
</div>
<div class="gmail-m_5073674045632076502moz-cite-prefix">Thanks,</div>
<div class="gmail-m_5073674045632076502moz-cite-prefix"><br>
</div>
<div class="gmail-m_5073674045632076502moz-cite-prefix">-Lionel<br>
</div>
<div class="gmail-m_5073674045632076502moz-cite-prefix"><br>
</div>
<div class="gmail-m_5073674045632076502moz-cite-prefix">On
06/02/2019 10:25, Sergii Romantsov wrote:<br>
</div>
<blockquote type="cite">
<div dir="ltr">
<div dir="ltr">
<div>Hello, Lionel. Thanks for looking on that.</div>
<div><br>
</div>
<div>Will try to explain:</div>
<div>Assume that a size of framebuffer is 160*160</div>
<div>We have a read-buffer and write-buffer.</div>
<div>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.</div>
<div>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.</div>
<div><br>
</div>
<div>Test shows that for big dest-region a draw
performed incorrectly.</div>
<div>Correct blit of 160*160 region to dest (0xffffff,
0xffffff): read160_to0xffffff.png</div>
<div>Incorrect blit of 160*160 region to dest
(0x7fffffff, 0x7fffffff): read160_to0x7fffffff.png</div>
<div>Incorrect blit of INT_MAX*INT_MAX region to dest
(INT_MAX, INT_MAX): readIntMax_toIntMax.png</div>
</div>
</div>
<br>
<div class="gmail_quote">
<div dir="ltr" class="gmail_attr">On Tue, Feb 5, 2019 at
1:13 PM Lionel Landwerlin <<a
href="mailto:lionel.g.landwerlin@intel.com"
target="_blank" moz-do-not-send="true">lionel.g.landwerlin@intel.com</a>>
wrote:<br>
</div>
<blockquote class="gmail_quote" style="margin:0px 0px
0px 0.8ex;border-left:1px solid
rgb(204,204,204);padding-left:1ex">On 05/02/2019
07:52, Sergii Romantsov wrote:<br>
> From: Vadym Shovkoplias <<a
href="mailto:vadim.shovkoplias@gmail.com"
target="_blank" moz-do-not-send="true">vadim.shovkoplias@gmail.com</a>><br>
><br>
> This test checks max possible blit buffers sizes<br>
><br>
> v2: copyright updated<br>
><br>
> Bugzilla: <a
href="https://bugs.freedesktop.org/show_bug.cgi?id=108088"
rel="noreferrer" target="_blank"
moz-do-not-send="true">https://bugs.freedesktop.org/show_bug.cgi?id=108088</a><br>
> Signed-off-by: Vadym Shovkoplias <<a
href="mailto:vadym.shovkoplias@globallogic.com"
target="_blank" moz-do-not-send="true">vadym.shovkoplias@globallogic.com</a>><br>
<br>
Hi Sergii,<br>
<br>
The bug opened about does not give any description
about what is <br>
incorrect in the current implementations or what is
being tested in the <br>
spec.<br>
<br>
The only paragraph I could find in the spec related to
this is :<br>
"<br>
The actual region written to the draw framebuffer is
limited to the <br>
intersection of<br>
the destination buffers being written, which may
include multiple draw <br>
buffers,<br>
the depth buffer, and/or the stencil buffer depending
on mask. Whether <br>
or not the<br>
source or destination regions are altered due to these
limits, the <br>
scaling and offset<br>
applied to pixels being transferred is performed as
though no such <br>
limits were<br>
present.<br>
"<br>
<br>
I'm struggling to understand whether the
"intersection" is related to <br>
dimensions or the attachments.<br>
Could give more context?<br>
<br>
Thanks,<br>
<br>
-Lionel<br>
<br>
<br>
> ---<br>
> tests/fbo/CMakeLists.gl.txt | 1 +<br>
> tests/fbo/fbo-blit-check-limits.c | 85
+++++++++++++++++++++++++++++++++++++++<br>
> tests/opengl.py | 1 +<br>
> 3 files changed, 87 insertions(+)<br>
> create mode 100644
tests/fbo/fbo-blit-check-limits.c<br>
><br>
> diff --git a/tests/fbo/CMakeLists.gl.txt
b/tests/fbo/CMakeLists.gl.txt<br>
> index 1a1a607..e2c7b3a 100644<br>
> --- a/tests/fbo/CMakeLists.gl.txt<br>
> +++ b/tests/fbo/CMakeLists.gl.txt<br>
> @@ -91,6 +91,7 @@ piglit_add_executable
(fbo-storage-formats fbo-storage-formats.c)<br>
> piglit_add_executable (fbo-storage-completeness
fbo-storage-completeness.c)<br>
> piglit_add_executable (fbo-sys-blit
fbo-sys-blit.c)<br>
> piglit_add_executable (fbo-sys-sub-blit
fbo-sys-sub-blit.c)<br>
> +piglit_add_executable (fbo-blit-check-limits
fbo-blit-check-limits.c)<br>
> piglit_add_executable (fbo-tex-rgbx
fbo-tex-rgbx.c)<br>
> piglit_add_executable (fbo-pbo-readpixels-small
fbo-pbo-readpixels-small.c)<br>
> piglit_add_executable (fbo-copyteximage
fbo-copyteximage.c)<br>
> diff --git a/tests/fbo/fbo-blit-check-limits.c
b/tests/fbo/fbo-blit-check-limits.c<br>
> new file mode 100644<br>
> index 0000000..92f54df<br>
> --- /dev/null<br>
> +++ b/tests/fbo/fbo-blit-check-limits.c<br>
> @@ -0,0 +1,85 @@<br>
> +/*<br>
> + * Copyright (C) 2018 Intel Corporation<br>
> + *<br>
> + * Permission is hereby granted, free of charge,
to any person obtaining a<br>
> + * copy of this software and associated
documentation files (the "Software"),<br>
> + * to deal in the Software without restriction,
including without limitation<br>
> + * the rights to use, copy, modify, merge,
publish, distribute, sublicense,<br>
> + * and/or sell copies of the Software, and to
permit persons to whom the<br>
> + * Software is furnished to do so, subject to
the following conditions:<br>
> + *<br>
> + * The above copyright notice and this
permission notice (including the next<br>
> + * paragraph) shall be included in all copies or
substantial portions of the<br>
> + * Software.<br>
> + *<br>
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT
WARRANTY OF ANY KIND, EXPRESS OR<br>
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE
WARRANTIES OF MERCHANTABILITY,<br>
> + * FITNESS FOR A PARTICULAR PURPOSE AND
NONINFRINGEMENT. IN NO EVENT SHALL<br>
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE
FOR ANY CLAIM, DAMAGES OR OTHER<br>
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT,
TORT OR OTHERWISE, ARISING<br>
> + * FROM, OUT OF OR IN CONNECTION WITH THE
SOFTWARE OR THE USE OR OTHER DEALINGS<br>
> + * IN THE SOFTWARE.<br>
> + *<br>
> + */<br>
> +<br>
> +/** @file fbo-blit-check-limits.c<br>
> + *<br>
> + * Test FBO blits with MAX possible buffer sizes<br>
> + * Bugzilla: <a
href="https://bugs.freedesktop.org/show_bug.cgi?id=108088"
rel="noreferrer" target="_blank"
moz-do-not-send="true">https://bugs.freedesktop.org/show_bug.cgi?id=108088</a><br>
> + *<br>
> + * \author Vadym Shovkoplias <<a
href="mailto:vadym.shovkoplias@globallogic.com"
target="_blank" moz-do-not-send="true">vadym.shovkoplias@globallogic.com</a>><br>
> + */<br>
> +<br>
> +#include "piglit-util-gl.h"<br>
> +<br>
> +PIGLIT_GL_TEST_CONFIG_BEGIN<br>
> +<br>
> + config.supports_gl_compat_version = 10;<br>
> +<br>
> + config.window_visual =
PIGLIT_GL_VISUAL_DOUBLE | PIGLIT_GL_VISUAL_RGB;<br>
> + config.requires_displayed_window = true;<br>
> + config.khr_no_error_support =
PIGLIT_NO_ERRORS;<br>
> +<br>
> +PIGLIT_GL_TEST_CONFIG_END<br>
> +<br>
> +enum piglit_result piglit_display(void)<br>
> +{<br>
> + const float green[] = {0.0f, 1.0f, 0.0f};<br>
> + int w = piglit_width;<br>
> + int h = piglit_height;<br>
> + bool success = 1;<br>
> +<br>
> + glDrawBuffer(GL_BACK);<br>
> + /* back buffer green */<br>
> + glClearColor(0.0f, 1.0f, 0.0f, 1.0f);<br>
> + glClear(GL_COLOR_BUFFER_BIT);<br>
> +<br>
> + glDrawBuffer(GL_FRONT);<br>
> + /* front buffer red */<br>
> + glClearColor(1.0f, 0.0f, 0.0f, 1.0f);<br>
> + glClear(GL_COLOR_BUFFER_BIT);<br>
> +<br>
> + glReadBuffer(GL_BACK);<br>
> +<br>
> + glBlitFramebufferEXT(INT_MIN, INT_MIN,
INT_MAX, INT_MAX,<br>
> + INT_MIN, INT_MIN,
INT_MAX, INT_MAX,<br>
> + GL_COLOR_BUFFER_BIT,
GL_NEAREST);<br>
> +<br>
> + glReadBuffer(GL_FRONT);<br>
> +<br>
> + /* the corners should be green */<br>
> + success &= piglit_probe_pixel_rgb(0, 0,
green);<br>
> + success &= piglit_probe_pixel_rgb(w -
1, 0, green);<br>
> + success &= piglit_probe_pixel_rgb(0, h
- 1, green);<br>
> + success &= piglit_probe_pixel_rgb(w -
1, h - 1, green);<br>
> +<br>
> + glFlush();<br>
> +<br>
> + return success ? PIGLIT_PASS : PIGLIT_FAIL;<br>
> +}<br>
> +<br>
> +void piglit_init(int argc, char **argv)<br>
> +{<br>
> +
piglit_require_extension("GL_EXT_framebuffer_object");<br>
> +
piglit_require_extension("GL_EXT_framebuffer_blit");<br>
> +}<br>
> diff --git a/tests/opengl.py b/tests/opengl.py<br>
> index af68560..0be2980 100644<br>
> --- a/tests/opengl.py<br>
> +++ b/tests/opengl.py<br>
> @@ -2780,6 +2780,7 @@ with
profile.test_list.group_manager(<br>
> g(['fbo-readdrawpix'],
run_concurrent=False)<br>
> g(['fbo-sys-blit'], run_concurrent=False)<br>
> g(['fbo-sys-sub-blit'],
run_concurrent=False)<br>
> + g(['fbo-blit-check-limits'],
run_concurrent=False)<br>
>
g(['fbo-generatemipmap-versus-READ_FRAMEBUFFER'])<br>
> <br>
> with profile.test_list.group_manager(<br>
<br>
<br>
</blockquote>
</div>
</blockquote>
<p><br>
</p>
</div>
</blockquote>
</div>
</blockquote>
<p><br>
</p>
</body>
</html>