After giving it another look, I concluded that no additional checks are needed, but...
Can I have a ruling on these, so that I know what to put into the docs for Gallium? "The scissors may be set to include area outside of the viewport." "The driver may clamp the scissors to the viewport when scissoring is enabled." I know that this is wrong for GL, but is it wrong for Gallium? I'll have to double-check some sanity code in this case, because Radeon scissors can't ever be turned off and so they have some fun side effects. "The scissors are ignored when the scissor flag (linkref to pipe_rasterizer_state::scissor) is disabled, so the scissors never need to be set if they will not be used." On Sun, Jan 24, 2010 at 4: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. > > -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 > -- Only fools are easily impressed by what is only barely beyond their reach. ~ Unknown Corbin Simpson <mostawesomed...@gmail.com> ------------------------------------------------------------------------------ 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
