On Mon, Jan 25, 2010 at 12:56 AM, Brian Paul <brian.e.p...@gmail.com> wrote:
> On Sat, Jan 23, 2010 at 9:27 AM, Xavier Chantry
> <chantry.xav...@gmail.com> wrote:
>> commit 53174afeeb introduced a portability change that converted GLint x,y
>> to GLuint. That breaks when x and y are negative, which seems to be allowed,
>> and which at least one game uses : teeworlds.
>>
>> Rather than simply reverting the change, it seems possible to convert the
>> 16bit unsigned to GLint so that comparisons are made between signed integers
>> instead. This hopefully does not break anything while keeping MSVC happy.
>>
>> Signed-off-by: Xavier Chantry <chantry.xav...@gmail.com>
>
> Committed to the 7.7 branch. Thanks.
>
Oh great, thanks !
I kept investigating / testing and found another related issue. Sorry
for not spotting it earlier, patch attached :
0001-st-mesa-another-signed-unsigned-fix-in-scissor.patch
I also considered your last mail proposing to do bound/sanity checks
earlier. I discussed a bit with MostAwesomeDude on IRC who made this
proposal :
22:44 < MostAwesomeDude> Hm. Personally, I think that you should clamp
against the current viewport.
22:44 < MostAwesomeDude> Since the scissor test operates on pixels.
22:48 < MostAwesomeDude> The scissor test discards pixels that would
otherwise be in the viewport.
22:49 < MostAwesomeDude> So it makes sense that having your scissor
box be bigger than the viewport is useless.
So I went ahead and did that, see attached patch
0001-mesa-bounds-check-and-sanitize-glScissor-arguments.patch
This patch would also fix teeworlds on its own, without any of my st/mesa fixes.
However jbauman spotted serious shortcomings :
00:13 < jbauman> shining^, what if they set a small viewport, a large
scissor, a big viewport, and then draw?
00:14 < jbauman> you also have to keep track of the input scissor and
the actual used scissor
00:30 < jbauman> also, there may be interactions between switching
scissor/viewport/fbos
So it's more complex than I expected, and not sure what to do now.
Even if we find and implement a reliable way to clamp scissor against
viewport, I don't know if this will ensure that x+width and y+height
are never negative. So I would be happy with my straightforward fix to
st/mesa which deals with that and fix teeworlds, I hope completely
this time !
From b7f47dc3a74dcc521c6efa1fd6caa92833a44ad1 Mon Sep 17 00:00:00 2001
From: Xavier Chantry <chantry.xav...@gmail.com>
Date: Mon, 25 Jan 2010 01:18:49 +0100
Subject: [PATCH] st/mesa: another signed/unsigned fix in scissor
If x+width or y+height is negative, then maxx or maxy will get a bogus value
when converting that to unsigned. Fix this by setting 0 as minimal value.
This was also triggered by teeworlds, but only with some combination of
resolution and map section. For example upper part of dm2 at 1280x1024.
Signed-off-by: Xavier Chantry <chantry.xav...@gmail.com>
---
src/mesa/state_tracker/st_atom_scissor.c | 5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/src/mesa/state_tracker/st_atom_scissor.c b/src/mesa/state_tracker/st_atom_scissor.c
index fb666b8..57c456c 100644
--- a/src/mesa/state_tracker/st_atom_scissor.c
+++ b/src/mesa/state_tracker/st_atom_scissor.c
@@ -34,6 +34,7 @@
#include "st_context.h"
#include "pipe/p_context.h"
#include "st_atom.h"
+#include "main/macros.h"
/**
@@ -58,9 +59,9 @@ update_scissor( struct st_context *st )
scissor.miny = st->ctx->Scissor.Y;
if (st->ctx->Scissor.X + st->ctx->Scissor.Width < (GLint)scissor.maxx)
- scissor.maxx = st->ctx->Scissor.X + st->ctx->Scissor.Width;
+ scissor.maxx = MAX2(0, st->ctx->Scissor.X + st->ctx->Scissor.Width);
if (st->ctx->Scissor.Y + st->ctx->Scissor.Height < (GLint)scissor.maxy)
- scissor.maxy = st->ctx->Scissor.Y + st->ctx->Scissor.Height;
+ scissor.maxy = MAX2(0, st->ctx->Scissor.Y + st->ctx->Scissor.Height);
/* check for null space */
if (scissor.minx >= scissor.maxx || scissor.miny >= scissor.maxy)
--
1.6.6.1
From aa03ab430967a550e2b3b3313a5b5b2d757bfe83 Mon Sep 17 00:00:00 2001
From: Xavier Chantry <chantry.xav...@gmail.com>
Date: Mon, 25 Jan 2010 00:00:12 +0100
Subject: [PATCH] mesa: bounds-check and sanitize glScissor arguments
This is done by clamping the scissor rectangle against the viewport.
Signed-off-by: Xavier Chantry <chantry.xav...@gmail.com>
---
src/mesa/main/scissor.c | 18 ++++++++++++++----
1 files changed, 14 insertions(+), 4 deletions(-)
diff --git a/src/mesa/main/scissor.c b/src/mesa/main/scissor.c
index b5f4cde..b50f73c 100644
--- a/src/mesa/main/scissor.c
+++ b/src/mesa/main/scissor.c
@@ -25,6 +25,7 @@
#include "main/glheader.h"
#include "main/context.h"
+#include "main/macros.h"
#include "main/scissor.h"
@@ -37,14 +38,14 @@ _mesa_Scissor( GLint x, GLint y, GLsizei width, GLsizei height )
GET_CURRENT_CONTEXT(ctx);
ASSERT_OUTSIDE_BEGIN_END(ctx);
+ if (MESA_VERBOSE & VERBOSE_API)
+ _mesa_debug(ctx, "glScissor %d %d %d %d\n", x, y, width, height);
+
if (width < 0 || height < 0) {
_mesa_error( ctx, GL_INVALID_VALUE, "glScissor" );
return;
}
- if (MESA_VERBOSE & VERBOSE_API)
- _mesa_debug(ctx, "glScissor %d %d %d %d\n", x, y, width, height);
-
_mesa_set_scissor(ctx, x, y, width, height);
}
@@ -64,8 +65,17 @@ _mesa_Scissor( GLint x, GLint y, GLsizei width, GLsizei height )
*/
void
_mesa_set_scissor(GLcontext *ctx,
- GLint x, GLint y, GLsizei width, GLsizei height)
+ GLint x1, GLint y1, GLsizei width1, GLsizei height1)
{
+ GLint x,y;
+ GLsizei width,height;
+
+ /* sanitize the values by clamping them against the viewport */
+ x = CLAMP(x1, ctx->Viewport.X, ctx->Viewport.X + ctx->Viewport.Width);
+ y = CLAMP(y1, ctx->Viewport.Y, ctx->Viewport.Y + ctx->Viewport.Height);
+ width = CLAMP(x1 + width1 - x, 0, ctx->Viewport.X + ctx->Viewport.Width - x);
+ height = CLAMP(y1 + height1 - y, 0, ctx->Viewport.Y + ctx->Viewport.Height - y);
+
if (x == ctx->Scissor.X &&
y == ctx->Scissor.Y &&
width == ctx->Scissor.Width &&
--
1.6.6.1
------------------------------------------------------------------------------
Throughout its 18-year history, RSA Conference consistently attracts the
world's best and brightest in the field, creating opportunities for Conference
attendees to learn about information security's most important issues through
interactions with peers, luminaries and emerging and established companies.
http://p.sf.net/sfu/rsaconf-dev2dev
_______________________________________________
Mesa3d-dev mailing list
Mesa3d-dev@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/mesa3d-dev
