Mesa (master): draw: drop unnecessary index overflow handling from vsplit code
Roland Scheidegger
sroland at kemper.freedesktop.org
Mon Nov 21 19:04:18 UTC 2016
Module: Mesa
Branch: master
Commit: 78a997f72841310620d18daa9015633343d04db1
URL: http://cgit.freedesktop.org/mesa/mesa/commit/?id=78a997f72841310620d18daa9015633343d04db1
Author: Roland Scheidegger <sroland at vmware.com>
Date: Sun Nov 13 17:17:25 2016 +0100
draw: drop unnecessary index overflow handling from vsplit code
This was kind of strange, since it replaced indices which were only
overflowing due to bias with MAX_UINT. This would cause an overflow later
in the shader, except if stride was 0, however the vertex id would be
essentially random then (-1 + eltBias). No test cared about it, though.
So, drop this and just use ordinary int arithmetic wraparound as usual.
This is much simpler to understand and the results are "more correct" or
at least more consistent (vertex id as well as actual fetch results just
correspond to wrapped around arithmetic).
There's only one catch, it is now possible to hit the cache initialization
value also with ushort and ubyte elts path (this wouldn't be an issue if
we'd simply handle the eltBias itself later in the shader). Hence, we need
to make sure the cache logic doesn't think this element has already been
emitted when it has not (I believe some seriously bad things could happen
otherwise). So, borrow the logic which handled this from the uint case, but
not before fixing it up...
Reviewed-by: Jose Fonseca <jfonseca at vmware.com>
---
src/gallium/auxiliary/draw/draw_pt_vsplit.c | 69 ++++++++++---------------
src/gallium/auxiliary/draw/draw_pt_vsplit_tmp.h | 15 +-----
2 files changed, 28 insertions(+), 56 deletions(-)
diff --git a/src/gallium/auxiliary/draw/draw_pt_vsplit.c b/src/gallium/auxiliary/draw/draw_pt_vsplit.c
index fb131c3..a68d5bf 100644
--- a/src/gallium/auxiliary/draw/draw_pt_vsplit.c
+++ b/src/gallium/auxiliary/draw/draw_pt_vsplit.c
@@ -85,7 +85,7 @@ vsplit_flush_cache(struct vsplit_frontend *vsplit, unsigned flags)
* Add a fetch element and add it to the draw elements.
*/
static inline void
-vsplit_add_cache(struct vsplit_frontend *vsplit, unsigned fetch, unsigned ofbias)
+vsplit_add_cache(struct vsplit_frontend *vsplit, unsigned fetch)
{
unsigned hash;
@@ -93,7 +93,7 @@ vsplit_add_cache(struct vsplit_frontend *vsplit, unsigned fetch, unsigned ofbias
/* If the value isn't in the cache or it's an overflow due to the
* element bias */
- if (vsplit->cache.fetches[hash] != fetch || ofbias) {
+ if (vsplit->cache.fetches[hash] != fetch) {
/* update cache */
vsplit->cache.fetches[hash] = fetch;
vsplit->cache.draws[hash] = vsplit->cache.num_fetch_elts;
@@ -108,7 +108,7 @@ vsplit_add_cache(struct vsplit_frontend *vsplit, unsigned fetch, unsigned ofbias
/**
* Returns the base index to the elements array.
- * The value is checked for integer overflow.
+ * The value is checked for integer overflow (not sure it can happen?).
*/
static inline unsigned
vsplit_get_base_idx(unsigned start, unsigned fetch)
@@ -116,39 +116,14 @@ vsplit_get_base_idx(unsigned start, unsigned fetch)
return draw_overflow_uadd(start, fetch, MAX_ELT_IDX);
}
-/**
- * Returns the element index adjust for the element bias.
- * The final element index is created from the actual element
- * index, plus the element bias, clamped to maximum element
- * index if that addition overflows.
+/*
+ * The final element index is just element index plus element bias.
*/
-static inline unsigned
-vsplit_get_bias_idx(int idx, int bias, unsigned *ofbias)
-{
- int res = idx + bias;
-
- *ofbias = 0;
-
- if (idx > 0 && bias > 0) {
- if (res < idx) {
- res = DRAW_MAX_FETCH_IDX;
- *ofbias = 1;
- }
- } else if (idx < 0 && bias < 0) {
- if (res > idx) {
- res = DRAW_MAX_FETCH_IDX;
- *ofbias = 1;
- }
- }
-
- return res;
-}
-
#define VSPLIT_CREATE_IDX(elts, start, fetch, elt_bias) \
unsigned elt_idx; \
- unsigned ofbias; \
- elt_idx = vsplit_get_base_idx(start, fetch); \
- elt_idx = vsplit_get_bias_idx(DRAW_GET_IDX(elts, elt_idx), elt_bias, &ofbias)
+ elt_idx = vsplit_get_base_idx(start, fetch); \
+ elt_idx = (unsigned)((int)(DRAW_GET_IDX(elts, elt_idx)) + (int)elt_bias);
+
static inline void
vsplit_add_cache_ubyte(struct vsplit_frontend *vsplit, const ubyte *elts,
@@ -156,7 +131,13 @@ vsplit_add_cache_ubyte(struct vsplit_frontend *vsplit, const ubyte *elts,
{
struct draw_context *draw = vsplit->draw;
VSPLIT_CREATE_IDX(elts, start, fetch, elt_bias);
- vsplit_add_cache(vsplit, elt_idx, ofbias);
+ /* unlike the uint case this can only happen with elt_bias */
+ if (elt_bias && elt_idx == DRAW_MAX_FETCH_IDX && !vsplit->cache.has_max_fetch) {
+ unsigned hash = fetch % MAP_SIZE;
+ vsplit->cache.fetches[hash] = 0;
+ vsplit->cache.has_max_fetch = TRUE;
+ }
+ vsplit_add_cache(vsplit, elt_idx);
}
static inline void
@@ -165,7 +146,13 @@ vsplit_add_cache_ushort(struct vsplit_frontend *vsplit, const ushort *elts,
{
struct draw_context *draw = vsplit->draw;
VSPLIT_CREATE_IDX(elts, start, fetch, elt_bias);
- vsplit_add_cache(vsplit, elt_idx, ofbias);
+ /* unlike the uint case this can only happen with elt_bias */
+ if (elt_bias && elt_idx == DRAW_MAX_FETCH_IDX && !vsplit->cache.has_max_fetch) {
+ unsigned hash = fetch % MAP_SIZE;
+ vsplit->cache.fetches[hash] = 0;
+ vsplit->cache.has_max_fetch = TRUE;
+ }
+ vsplit_add_cache(vsplit, elt_idx);
}
@@ -178,17 +165,15 @@ vsplit_add_cache_uint(struct vsplit_frontend *vsplit, const uint *elts,
unsigned start, unsigned fetch, int elt_bias)
{
struct draw_context *draw = vsplit->draw;
- unsigned raw_elem_idx = start + fetch + elt_bias;
VSPLIT_CREATE_IDX(elts, start, fetch, elt_bias);
-
- /* special care for DRAW_MAX_FETCH_IDX */
- if (raw_elem_idx == DRAW_MAX_FETCH_IDX && !vsplit->cache.has_max_fetch) {
+ /* Take care for DRAW_MAX_FETCH_IDX (since cache is initialized to -1). */
+ if (elt_idx == DRAW_MAX_FETCH_IDX && !vsplit->cache.has_max_fetch) {
unsigned hash = fetch % MAP_SIZE;
- vsplit->cache.fetches[hash] = raw_elem_idx - 1; /* force update */
+ /* force update - any value will do except DRAW_MAX_FETCH_IDX */
+ vsplit->cache.fetches[hash] = 0;
vsplit->cache.has_max_fetch = TRUE;
}
-
- vsplit_add_cache(vsplit, elt_idx, ofbias);
+ vsplit_add_cache(vsplit, elt_idx);
}
diff --git a/src/gallium/auxiliary/draw/draw_pt_vsplit_tmp.h b/src/gallium/auxiliary/draw/draw_pt_vsplit_tmp.h
index 7b08970..be353c4 100644
--- a/src/gallium/auxiliary/draw/draw_pt_vsplit_tmp.h
+++ b/src/gallium/auxiliary/draw/draw_pt_vsplit_tmp.h
@@ -156,7 +156,7 @@ CONCAT(vsplit_segment_cache_, ELT_TYPE)(struct vsplit_frontend *vsplit,
if (close)
ADD_CACHE(vsplit, ib, 0, iclose, 0);
}
- else if (ibias > 0) {
+ else {
if (spoken)
ADD_CACHE(vsplit, ib, 0, ispoken, ibias);
@@ -166,19 +166,6 @@ CONCAT(vsplit_segment_cache_, ELT_TYPE)(struct vsplit_frontend *vsplit,
if (close)
ADD_CACHE(vsplit, ib, 0, iclose, ibias);
}
- else {
- if (spoken) {
- ADD_CACHE(vsplit, ib, 0, ispoken, ibias);
- }
-
- for (i = spoken; i < icount; i++) {
- ADD_CACHE(vsplit, ib, istart, i, ibias);
- }
-
- if (close) {
- ADD_CACHE(vsplit, ib, 0, iclose, ibias);
- }
- }
vsplit_flush_cache(vsplit, flags);
}
More information about the mesa-commit
mailing list