[Mesa-dev] Bugs in swr (OpenSWR) driver
Victor Moya del Barrio
victor.moya.del.barrio at gmail.com
Sat Aug 13 11:20:06 UTC 2016
Hi,
I made an attempt to run Enemy Territories Quake Wars using the OpenSWR
driver and I found a number of bugs in the implementation
The final result of the 'hacks' below is the first one or two frames of the
actual game rendering but with most of the rendered textures painted in red
(perhaps related with the miss definition of LATC2 as BC5?). After that
Mesa triggers a GL_OUT_OF_MEMORY on glCompressedTexSubImage2D (perhaps
another variant of DXTn that I should map).
As I'm new and I'm not sure how to proceed I will just report them.
1) INT64 not defined for 32-bit build of Mesa3D (ETQW is a 32-bit binary),
I used uint64_t instead.
diff --git a/src/gallium/drivers/swr/rasterizer/core/utils.h
b/src/gallium/drivers/swr/rasterizer/core/utils.h
index 79f45eb..039db2c 100644
--- a/src/gallium/drivers/swr/rasterizer/core/utils.h
+++ b/src/gallium/drivers/swr/rasterizer/core/utils.h
@@ -37,7 +37,7 @@
#define _MM_INSERT_EPI64 _mm_insert_epi64
#define _MM_EXTRACT_EPI64 _mm_extract_epi64
#else
-INLINE INT64 _MM_EXTRACT_EPI64(__m128i a, const int32_t ndx)
+INLINE uint64_t _MM_EXTRACT_EPI64(__m128i a, const int32_t ndx)
{
OSALIGNLINE(uint32_t) elems[4];
_mm_store_si128((__m128i*)elems, a);
@@ -55,7 +55,7 @@ INLINE INT64 _MM_EXTRACT_EPI64(__m128i a, const int32_t
ndx)
}
}
-INLINE __m128i _MM_INSERT_EPI64(__m128i a, INT64 b, const int32_t ndx)
+INLINE __m128i _MM_INSERT_EPI64(__m128i a, uint64_t b, const int32_t ndx)
{
OSALIGNLINE(int64_t) elems[2];
_mm_store_si128((__m128i*)elems, a);
2) total_size is defined just as 'unsigned' and in a 32-bit build that's a
32-bit value which is smaller than the largest texture size defined, so it
should be defined as 64-bit value. Not sure if the 1048 values in the
definition of SWR_MAX_TEXTURE_SIZE are intended or not (or an error
changing from 2048 x 2048)-
3) the DXTn texture formats are not properly mapped to the internal formats
(BCn_UNORM). Ignore LATC2 below as that is just a hack to get something
running, from what I remember BC5 doesn't matches LATC2 definition. The
lack of support for DXTn formats triggers GL_OUT_OF_MEMORY errors before
the actual game starts. Note that I'm running a Virtual Box Ubuntu VM with
8 GBs of RAM so perhaps these GL_OUT_OF_MEMORY are masking another kind of
error on the texture format conversion logic.
diff --git a/src/gallium/drivers/swr/swr_screen.cpp
b/src/gallium/drivers/swr/swr_screen.cpp
index 3790fea..60e289f 100644
--- a/src/gallium/drivers/swr/swr_screen.cpp
+++ b/src/gallium/drivers/swr/swr_screen.cpp
@@ -55,7 +55,7 @@ extern "C" {
* Max texture sizes
* XXX Check max texture size values against core and sampler.
*/
-#define SWR_MAX_TEXTURE_SIZE (4 * 1048 * 1048 * 1024ULL) /* 4GB */
+#define SWR_MAX_TEXTURE_SIZE (4 * 1024 * 1024 * 1024ULL) /* 4GB */
#define SWR_MAX_TEXTURE_2D_LEVELS 14 /* 8K x 8K for now */
#define SWR_MAX_TEXTURE_3D_LEVELS 12 /* 2K x 2K x 2K for now */
#define SWR_MAX_TEXTURE_CUBE_LEVELS 14 /* 8K x 8K for now */
@@ -447,6 +447,12 @@ mesa_to_swr_format(enum pipe_format format)
return R32_FLOAT_X8X24_TYPELESS;
case PIPE_FORMAT_L8A8_UNORM:
return R8G8_UNORM;
+ case PIPE_FORMAT_DXT1_RGB:
+ return BC1_UNORM;
+ case PIPE_FORMAT_DXT5_RGBA:
+ return BC3_UNORM;
+ case PIPE_FORMAT_LATC2_UNORM:
+ return BC5_UNORM;
default:
break;
}
@@ -514,11 +520,11 @@ swr_texture_layout(struct swr_screen *screen,
SWR_FORMAT_INFO finfo = GetFormatInfo(res->swr.format);
- unsigned total_size = 0;
- unsigned width = pt->width0;
- unsigned height = pt->height0;
- unsigned depth = pt->depth0;
- unsigned layers = pt->array_size;
+ unsigned long long total_size = 0;
+ unsigned int width = pt->width0;
+ unsigned int height = pt->height0;
+ unsigned int depth = pt->depth0;
+ unsigned int layers = pt->array_size;
for (int level = 0; level <= pt->last_level; level++) {
unsigned alignedWidth, alignedHeight;
4) An error in the interaction between the Gallium Blitter (source seems to
be a texture, a few previous blit operations worked) and OpenSWR handling
of vertex buffers. That's a bit more difficult to explain ... It seems
swr has some expectation on the state of the vertex buffers defined in
ctx->vertex_buffers that the Blitter, or the functions used by the Blitter,
are not fulfilling. From what I understand the Blitter sets the state to a
single vertex buffer, but swr_update_derived() finds
ctx->num_vertex_buffers set to 4, probably because previous draw calls use
4 vertex buffers. My guess is that when combining the current vertex
buffer state with the new one in util_set_vertex_buffers_count()
(src/gallium/auxiliary/util/u_helpers.c) the old 4 vertex buffers 'survive'
and only the new one defined by the Blitter is updated. The swr driver may
expect these old vertex buffers to be 'disabled' previously. When
swr_update_derived() is called the incorrectly active vertex buffers don't
have pitch/stream_pitch defined, the size becomes 0 and
swr_copy_to_scratch_space() triggers an assert on size. The code below is
just a hack to make it work, I suppose the correct solution is in a
different place related with the assumptions the swr driver makes on the
state of the vertex buffers.
diff --git a/src/gallium/drivers/swr/swr_state.cpp
b/src/gallium/drivers/swr/swr_state.cpp
index de41ddc..70ebb16 100644
--- a/src/gallium/drivers/swr/swr_state.cpp
+++ b/src/gallium/drivers/swr/swr_state.cpp
@@ -1018,13 +1020,18 @@ swr_update_derived(struct pipe_context *pipe,
max_vertex = info.max_index + 1;
partial_inbounds = 0;
- /* Copy only needed vertices to scratch space */
- size = AlignUp(size, 4);
- const void *ptr = (const uint8_t *) vb->user_buffer
- + info.min_index * pitch;
- ptr = swr_copy_to_scratch_space(
- ctx, &ctx->scratch->vertex_buffer, ptr, size);
- p_data = (const uint8_t *)ptr - info.min_index * pitch;
+ if (size > 0)
+ {
+ /* Copy only needed vertices to scratch space */
+ size = AlignUp(size, 4);
+ const void *ptr = (const uint8_t *) vb->user_buffer
+ + info.min_index * pitch;
+ ptr = swr_copy_to_scratch_space(
+ ctx, &ctx->scratch->vertex_buffer, ptr, size);
+ p_data = (const uint8_t *)ptr - info.min_index * pitch;
+ }
+ else
+ p_data = NULL;
}
swrVertexBuffers[i] = {0};
Victor
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160813/e2d5f0ab/attachment.html>
More information about the mesa-dev
mailing list