Bug 1731 - Savage: Stray scissoring
Summary: Savage: Stray scissoring
Status: RESOLVED FIXED
Alias: None
Product: Mesa
Classification: Unclassified
Component: Drivers/DRI/Savage (show other bugs)
Version: git
Hardware: x86 (IA32) Linux (All)
: high normal
Assignee: Default DRI bug account
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2004-10-28 16:33 UTC by Owen Taylor
Modified: 2009-08-24 12:22 UTC (History)
0 users

See Also:
i915 platform:
i915 features:


Attachments
Fix scissoring, ignore window clipping (4.03 KB, patch)
2004-10-29 10:52 UTC, Owen Taylor
Details | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Owen Taylor 2004-10-28 16:33:07 UTC
savageDDRenderStart() contains:

        /* set scissor to the first clip box*/
        savageDDScissor(ctx,pbox->x1,pbox->y1,pbox->x2,pbox->y2);

If GL_SCISSOR_TEST is off, this has no effect. If GL_SCISSOR_TEST is on,
it overrides the applications scissor and also causes incorrect
clipping because savageDDScissor takes coordinates that are drawable
relative (like glScissor), not screen relative like the clip rects.

There seems to be quite a lot of rotted code around clipping in the 
savage driver; I'm still trying to figure out how things are supposed
to work and what the correct fix is.
Comment 1 Owen Taylor 2004-10-29 09:34:24 UTC
Here's some further results from my investigation:

 - In general, the savage driver *is not clipping* primitives.

   This means that drawing to the front buffer is not clipped properly,
   and multiple clients drawing to overlapping areas in the back buffer
   can also interfere.

 - Even if you ignore the above, just commenting out the stray scissoring call 
   doesn't completely get scissoring working - it doesn't respond properly
   to moving the window. This should be easy to fix.

 - I'm not sure if the savage hardware supports more than a single
   scissor rectangle. The algorithm used by the i810, unichrome,
   etc, drivers for clipping primitives by drawing them repeatedly in
   chunks of the number of supported clip rectangles would work with 
   only one clip rectangle, but could be really inefficient, if you
   say, overlapped 'xeyes' over the window

 - As noted above, there is a lot of rotted code around clipping. I think
   the right approach would either to be;

   A) Accept that we can't clip on the savage, remove all the 
      SAVAGE_UPLOAD_CLIPRECTS.

   B) Implement the standard draw-repeatedly algorithm referencing
      one of the other drivers using a single clip rectangle. If more
      powerful hardware clip facilities come to light, it should be
      easy to adapt.

  I tend to think B) is right, if moving xeyes over your GL window slows
  things to a crawl, move xeyes off.

Note that I'm not promising to do the above work, just writing down the
results of my investigation.
Comment 2 Felix Kühling 2004-10-29 09:51:13 UTC
(In reply to comment #0)
> There seems to be quite a lot of rotted code around clipping in the 
> savage driver; I'm still trying to figure out how things are supposed
> to work and what the correct fix is.

You're right, clipping has never worked correctly in the Savage driver. I didn't
bother fixing it so far since I was going to rewrite the code that sends
vertices to the hardware to use DMA buffers. In fact I'm starting to work on it
now. Eventually I'll get down to the clipping issues and fix them.
Comment 3 Owen Taylor 2004-10-29 10:52:29 UTC
Created attachment 1189 [details] [review]
Fix scissoring, ignore window clipping

Ignoring the clip-list issues, the attached patch seems to get scissoring
working right for me. It handles changing the hardware scissor rectangle
when the window is moved, and when scissoring is enabled/disabled.

It might be useful in the short term, though if you are working on redoing
vertex emission and clipping now, there may be no point.
Comment 4 Felix Kühling 2005-02-03 17:25:23 UTC
(In reply to comment #3)
> Created an attachment (id=1189) [edit]
> Fix scissoring, ignore window clipping

Oops, I never took a closer look at this patch. Anyway, I believe the clipping
and scissoring issues are fixed in latest CVS. Could you confirm this with
whatever application you were testing your patch?
Comment 5 Owen Taylor 2005-02-03 17:44:15 UTC
Unfortunately, the current code locks my machine solid as soon as it
tries to render anything.(glxinfo info works but no more.) I want to 
try and debug that, but it is going to involve getting together:

 - Me
 - Time
 - An environment with more than one machine

The code in CVS doesn't *look* right to me ... it's still saving
the scissor in screen coordinates, and doesn't seem to update it
on window move. But I could be missing something.
Comment 6 Alex Deucher 2005-02-03 20:13:04 UTC
(In reply to comment #5)
> Unfortunately, the current code locks my machine solid as soon as it
> tries to render anything.(glxinfo info works but no more.) I want to 
> try and debug that, but it is going to involve getting together:
> 

make sure you enable "shadowstatus" in your config.
Comment 7 Owen Taylor 2005-02-05 09:48:09 UTC
OK, tested with vdma off which works around the lockup and as expected there 
still are some residual problems with moving windows ... try adding 

   glEnable (GL_SCISSOR_TEST);
   glScissor (20, 20, 100, 100);

to init() in Mesa/progs/redbook/hello.c and you'll see that the scissor doesn't
move with the window. 

Seems the easiest fix would just to create a temporary rectangle to
pass to savageIntersectClipRects in savageFlushCmdBufLocked from
ctx->Scissor and the current window position. There also looks like there
is a lot of dead code in savage_state.c about scissoring that could be
cleaned up.


Looks like 
Comment 8 Felix Kühling 2005-02-05 14:00:34 UTC
I did some cleanup of the scissors code and removed some dead code. It passes
the glean scissor test now as well as your patched hello.c. I'm closing this bug
now. If you find something else scissor-related feel free to reopen it.
Comment 9 Adam Jackson 2009-08-24 12:22:49 UTC
Mass version move, cvs -> git


bug/show.html.tmpl processed on Dec 05, 2016 at 08:27:55.
(provided by the Example extension).