On Fri, Aug 7, 2015 at 12:24 PM, Oded Gabbay <oded.gab...@gmail.com> wrote:
> This patch fixes a bug that is manifested in the read path of mesa when
> running on big-endian machines. The effects can be seen when running
> piglit sanity test and/or taking a screen capture.
piglit sanity isn't all that convincing. It's quite possible that
there are two byte-swapping bugs that just happen to cancel out. If
it fixes huge numbers of piglit tests, that's more convincing.
> The bug is caused when _mesa_format_convert receives src_format as
> mesa_format, which it thens changes to mesa_array_format. During this
> change, it checks for endianness and swaps the bytes accordingly.
> However, because the bytes are _already_ swapped in the memory itself
> (being written there by llvmpipe/softpipe/sw rast), and src_format
> value matches the _actual_ contents of the memory, the result of the
> read is wrong.
>
> Therefore, because other layers/functions, such as llvm or
> read_rgba_pixels() takes care whether we are dealing with big-endian or
> little-endian, _mesa_format_convert should be endian agnostic to avoid
> duplicate swapping of bytes.
As far as I know, the core mesa format conversion code has handled
byte-swapping for a very long time. it is *not* simply punted to the
driver. I'm 100% ready to believe that there is a problem but it's
probably far more subtle than just removing support for byte-swapping
in _mesa_format_to_array_format.
> btw, it is also mentioned in the comment of the function that it doesn't
> handle byte-swapping, so the original code contradicts the documentation.
>
> Signed-off-by: Oded Gabbay <oded.gab...@gmail.com>
> CC: "10.5 10.6" <mesa-sta...@lists.freedesktop.org>
> Signed-off-by: Oded Gabbay <oded.gab...@gmail.com>
> ---
> src/mesa/main/formats.c | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/src/mesa/main/formats.c b/src/mesa/main/formats.c
> index baeb1bf..7a41cb0 100644
> --- a/src/mesa/main/formats.c
> +++ b/src/mesa/main/formats.c
> @@ -372,10 +372,8 @@ uint32_t
> _mesa_format_to_array_format(mesa_format format)
> {
> const struct gl_format_info *info = _mesa_get_format_info(format);
> - if (_mesa_little_endian())
> - return info->ArrayFormat;
> - else
> - return _mesa_array_format_flip_channels(info->ArrayFormat);
> +
> + return info->ArrayFormat;
> }
>
> static struct hash_table *format_array_format_table;
> --
> 2.4.3
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
