On Sun, Jan 24, 2010 at 5:53 PM, Brian Paul <brian.e.p...@gmail.com> wrote: > On Sun, Jan 24, 2010 at 5:40 PM, Xavier Chantry > <chantry.xav...@gmail.com> wrote: >> 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. > > MostAwesomeDude is incorrect. The viewport transformation and scissor > test are different things. Clamping the scissor box to the viewport > would be wrong. > > >> 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 ! >> > > What exactly is the issue with teeworlds? I haven't seen a bug report. > > I can apply the first part of your bounds-check patch which adds some > _mesa_debug() calls, but the rest is invalid.
Forgot: the second patch to prevent bogus int->uint values is the right idea. I'll just clean it up a little. Thanks. -Brian ------------------------------------------------------------------------------ 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
